go: Roll Goopfile, remove two no longer used packages.

go/clean_goop.py

+#!/usr/bin/env python
+# Copyright 2015 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Suggests what Goop file entries can be removed.
+
+Horribly slow, but simple. Evaluates recursive dependencies of all packages
+in infra/go/... (including github ones) and compares them against Goopfile.lock
+entries.
+
+Supposed to be called under activated go environment.
+"""
+
+import os
+import subprocess
+import sys
+
+# For VENDORED_TOOLS.
+import bootstrap
+
+
+GO_DIR = os.path.dirname(os.path.abspath(__file__))

[nodir, 2015/09/29 16:46:05]

You can check that $GOROOT equals this and suggest the user to run this in the
activated go env

[Vadim Sh., 2015/09/29 19:49:34]

GOROOT points to goland SDK. GOPATH have multiple entries. It is even more
complicated in infra_internal. I prefer not to bother. I don't present this
script to be overly user friendly.

+
+
+# List of Goopfile.lock entries we want to keep because they are imported only
+# when building on Windows (and thus detected as unused when script is used
+# on Linux).
+EXCEPTIONS = [
+  'github.com/mattn/go-isatty',
+  'github.com/olekukonko/ts',
+]
+
+
+def get_deps(pkg_glob):

[tandrii(chromium), 2015/09/29 10:17:27]

Add caching of the result, and it'll be so much faster :)

[Vadim Sh., 2015/09/29 19:49:34]

Added cache, it doesn't make much difference.

+  """Returns a set of imported dependencies."""
+  cmd = ['go', 'list', '-f', '{{ join .Deps "\\n" }}', pkg_glob]

[nodir, 2015/09/29 16:46:05]

this must be ran with GO_DIR as current dir, because you pass a rel path in
pkg_glob

alternatively, just run

GOPATH=$GOROOT go list all

when want to ignore vendored packages. (I think it is simpler)

[Vadim Sh., 2015/09/29 19:49:34]

relpath(...) below builds correct path regardless of current directory. It can
become incorrect only if roll_goop.py scrip itself changed cwd, and it doesn't.
GOROOT is goland SDK, not src/...

[nodir, 2015/09/29 20:39:13]

I just ran go/clean_goop.py from root and it errors out:

Traceback (most recent call last):
  File "go/clean_goop.py", line 116, in <module>
    sys.exit(main())
  File "go/clean_goop.py", line 82, in main
    assert go_dir_rel.startswith('.')
AssertionError

because go_dir_rel == `go`.

I mentioned cwd because go.relpath with one parameter ultimately depends on cwd

+  print 'Running ', cmd
+  deps = subprocess.check_output(cmd).strip()
+  return sorted(set(deps.splitlines()))
+
+
+def get_all_goop():

[tandrii(chromium), 2015/09/29 10:17:27]

and maybe cache this too?

[Vadim Sh., 2015/09/29 19:49:33]

Done.

+  """Returns a list of packages specified in Goopfile.lock."""
+  with open(os.path.join(GO_DIR, 'Goopfile.lock'), 'rt') as f:

[nodir, 2015/09/29 16:46:05]

't' mode is unnecessary to specify since it is default

[Vadim Sh., 2015/09/29 19:49:34]

r is too for that matter

+    goop = f.read()

[tandrii(chromium), 2015/09/29 10:17:27]

nit: return [l.split()[0] for l in f]

[Vadim Sh., 2015/09/29 19:49:34]

Done.

+  return [line.split()[0] for line in goop.splitlines()]
+
+
+def get_used_goop(deps):
+  """Returns set of packages in Goopfile.lock that cover a dependency list."""
+  used = set()
+  for pkg in get_all_goop():
+    for dep in deps:
+      # If goop pkg covers at least one dependency, it is used.
+      if dep.startswith(pkg):

[nodir, 2015/09/29 16:46:05]

if dep == pkg or dep.startswith(pkg+'/'):

[Vadim Sh., 2015/09/29 19:49:34]

Done.

+        used.add(pkg)
+        break
+  return used
+
+
+def main():
+  all_deps = set(EXCEPTIONS)
+
+  # We want to enumerate packages in infra/go/src/... and only them (not all
+  # packages in GOPAHT, since we don't want to enumerate vendored packages).

[nodir, 2015/09/29 16:46:05]

GOPATH

+  # A package glob needs to start with '.' or '..' to be interpreted as file
+  # system path. See 'go help packages'.
+  go_dir_rel = os.path.relpath(os.path.join(GO_DIR))

[nodir, 2015/09/29 16:46:05]

os.path.join(GO_DIR) == GO_DIR ? Am I missing something?

[Vadim Sh., 2015/09/29 19:49:34]

There used to be something else there.

+  assert go_dir_rel.startswith('.')
+  all_deps.update(get_deps(os.path.join(go_dir_rel, 'src', '...')))
+
+  # Tools build by bootstrap script may not be directly referenced by src/ code.
+  for tool in bootstrap.VENDORED_TOOLS:
+    all_deps.add(tool)
+    all_deps.update(get_deps(tool))
+
+  # all_deps is all direct dependencies of infra/go/src code. Find what part of
+  # Goopfile.lock covers them. Then recurse into them. Note that Goopfile
+  # may specify a parent package of a package used in actual code. So recursing
+  # into deps of that parent package may reveal more dependencies.
+  goop_deps = set()
+  while True:
+    for pkg in goop_deps:

[nodir, 2015/09/29 16:46:05]

goop_deps is set() in the first iteration. Then it is modified only in line 89.
Maybe move this loop to line 90 to make more readable?

[Vadim Sh., 2015/09/29 19:49:33]

Done.

+      all_deps.update(get_deps(pkg + '/...'))
+    new_goop_deps = get_used_goop(all_deps)
+    if new_goop_deps == goop_deps:
+      break
+    goop_deps = new_goop_deps
+
+  # Find what is not used.
+  unused = [p for p in get_all_goop() if p not in goop_deps]
+  if unused:
+    print '-' * 80
+    print 'Consider removing from Goopfile following packages:'

[tandrii(chromium), 2015/09/29 10:17:27]

nit: s/following/the following

[Vadim Sh., 2015/09/29 19:49:34]

Done.

+    print '\n'.join(unused)
+  else:
+    print 'Hooray! All Goopfile packages seems to be in use.'

[tandrii(chromium), 2015/09/29 10:17:28]

s/seems/seem

[Vadim Sh., 2015/09/29 19:49:34]

Done.

+
+  return 0
+
+
+if __name__ == '__main__':
+  sys.exit(main())