Integrate GoDistribution with GoTask

- Go tasks no longer use `go` command directly
- Removed `go` availability guards on integration tests

Testing Done:
CI went green here: https://travis-ci.org/pantsbuild/pants/builds/75183173

Bugs closed: 1956

Reviewed at https://rbcommons.com/s/twitter/r/2600/

contrib/go/src/python/pants/contrib/go/subsystems/go_distribution.py

@@ -96,6 +96,19 @@ def spawn(self, env=None, **kwargs):
       env.update(self.env)
       return subprocess.Popen(self.cmdline, env=env, **kwargs)
 
+    def check_output(self, env=None, **kwargs):
+      """Returns the output of the executed Go command.
+
+      :param dict env: A custom environment to launch the Go command in.  If `None` the current
+                       environment is used.
+      :param **kwargs: Keyword arguments to pass through to `subprocess.check_output`.
+      :return str: Output of Go command.
+      :raises subprocess.CalledProcessError: Raises if Go command fails.
+      """
+      env = (env or os.environ).copy()
+      env.update(self.env)
+      return subprocess.check_output(self.cmdline, env=env, **kwargs)
+
     def __str__(self):
       return (' '.join('{}={}'.format(k, v) for k, v in self.env.items()) +
               ' ' +

contrib/go/src/python/pants/contrib/go/tasks/BUILD

@@ -2,6 +2,7 @@ python_library(
   name='go_compile',
   sources=['go_compile.py'],
   dependencies=[
+    'src/python/pants/base:workunit',
     'src/python/pants/util:dirutil',
     'contrib/go/src/python/pants/contrib/go/tasks:go_workspace_task',
   ]
@@ -35,6 +36,7 @@ python_library(
   dependencies=[
     'src/python/pants/backend/core/tasks:task',
     'src/python/pants/base:exceptions',
+    'src/python/pants/util:memo',
     'contrib/go/src/python/pants/contrib/go/subsystems:go_distribution',
     'contrib/go/src/python/pants/contrib/go/targets:go_binary',
     'contrib/go/src/python/pants/contrib/go/targets:go_library',
@@ -47,6 +49,7 @@ python_library(
   name='go_test',
   sources=['go_test.py'],
   dependencies=[
+    'src/python/pants/base:workunit',
     'contrib/go/src/python/pants/contrib/go/tasks:go_workspace_task',
   ]
 )

contrib/go/src/python/pants/contrib/go/tasks/go_compile.py

@@ -8,6 +8,7 @@
 import os
 from collections import defaultdict
 
+from pants.base.workunit import WorkUnit
 from pants.util.dirutil import safe_mkdir
 
 from pants.contrib.go.tasks.go_workspace_task import GoWorkspaceTask
@@ -42,8 +43,7 @@ def execute(self):
         if not vt.valid:
           self.ensure_workspace(vt.target)
           self._sync_binary_dep_links(vt.target, gopath, lib_binary_map)
-          self.run_go_cmd('install', gopath, vt.target,
-                          cmd_flags=self.get_options().build_flags.split())
+          self._go_install(vt.target, gopath)
         if self.is_binary(vt.target):
           binary_path = os.path.join(gopath, 'bin', os.path.basename(vt.target.address.spec_path))
           self.context.products.get_data('exec_binary')[vt.target] = binary_path
@@ -54,6 +54,14 @@ def execute(self):
                                                    self.goos_goarch,
                                                    lib_binary_path) + '.a'
 
+  def _go_install(self, target, gopath):
+    pkg_path = (self.global_import_id(target) if self.is_remote_lib(target)
+                else target.address.spec_path)
+    args = self.get_options().build_flags.split() + [pkg_path]
+    self.go_dist.execute_go_cmd('install', gopath=gopath, args=args,
+                                workunit_factory=self.context.new_workunit,
+                                workunit_labels=[WorkUnit.COMPILER])
+
   def _sync_binary_dep_links(self, target, gopath, lib_binary_map):
     """Syncs symlinks under gopath to the library binaries of target's transitive dependencies.
 

contrib/go/src/python/pants/contrib/go/tasks/go_fetch.py

@@ -15,7 +15,7 @@
 from pants.util.dirutil import get_basedir, safe_mkdir, safe_open
 
 from pants.contrib.go.targets.go_remote_library import GoRemoteLibrary
-from pants.contrib.go.tasks.go_task import GoTask, get_cmd_output
+from pants.contrib.go.tasks.go_task import GoTask
 
 
 class GoFetch(GoTask):

contrib/go/src/python/pants/contrib/go/tasks/go_task.py

@@ -11,6 +11,7 @@
 
 from pants.backend.core.tasks.task import Task
 from pants.base.exceptions import TaskError
+from pants.util.memo import memoized_property
 
 from pants.contrib.go.subsystems.go_distribution import GoDistribution
 from pants.contrib.go.targets.go_binary import GoBinary
@@ -19,20 +20,10 @@
 from pants.contrib.go.targets.go_remote_library import GoRemoteLibrary
 
 
-# TODO(cgibb): Find a better home for this.
-def get_cmd_output(args, shell=False):
-  if shell:
-    args = ' '.join(args)
-  p = subprocess.Popen(args, shell=shell, stdout=subprocess.PIPE)
-  out, _ = p.communicate()
-  return out.strip()
-
-
 class GoTask(Task):
 
   @classmethod
   def global_subsystems(cls):
-    # TODO(John Sirois): Actualy use the GoDistribution.Factory to create/run go commands.
     return super(GoTask, cls).global_subsystems() + (GoDistribution.Factory,)
 
   @staticmethod
@@ -55,25 +46,21 @@ def is_local_src(target):
   def is_go(target):
     return isinstance(target, (GoLocalSource, GoRemoteLibrary))
 
-  @staticmethod
-  def lookup_goos_goarch():
-    goos = get_cmd_output(['go', 'env', 'GOOS'])
-    goarch = get_cmd_output(['go', 'env', 'GOARCH'])
-    return goos + '_' + goarch
-
-  def __init__(self, *args, **kwargs):
-    super(GoTask, self).__init__(*args, **kwargs)
-    self._goos_goarch = None
+  @memoized_property
+  def go_dist(self):
+    return GoDistribution.Factory.global_instance().create()
 
-  @property
+  @memoized_property
   def goos_goarch(self):
     """Returns concatenated $GOOS and $GOARCH environment variables, separated by an underscore.
 
     Useful for locating where the Go compiler is placing binaries ("$GOPATH/pkg/$GOOS_$GOARCH").
     """
-    if self._goos_goarch is None:
-      self._goos_goarch = self.lookup_goos_goarch()
-    return self._goos_goarch
+    return '{goos}_{goarch}'.format(goos=self._lookup_go_env_var('GOOS'),
+                                    goarch=self._lookup_go_env_var('GOARCH'))
+
+  def _lookup_go_env_var(self, var):
+    return self.go_dist.create_go_cmd('env', args=[var]).check_output().strip()
 
   def global_import_id(self, go_remote_lib):
     """Returns the global import identifier of the given GoRemoteLibrary.
@@ -86,26 +73,3 @@ def global_import_id(self, go_remote_lib):
     """
     return os.path.relpath(go_remote_lib.address.spec_path,
                            go_remote_lib.target_base)
-
-  def run_go_cmd(self, cmd, gopath, target, cmd_flags=None, pkg_flags=None):
-    """Runs a Go command on a target from within a Go workspace.
-
-    :param str cmd: Go command to execute, e.g. 'test' for `go test`
-    :param str gopath: $GOPATH which points to a valid Go workspace from which
-                       to run the command.
-    :param Target target: A Go package whose source the command will execute on.
-    :param list<str> cmd_flags: Command line flags to pass to command.
-    :param list<str> pkg_flags: Command line flags to pass to target package.
-    """
-    cmd_flags = cmd_flags or []
-    pkg_flags = pkg_flags or []
-    pkg_path = (self.global_import_id(target) if self.is_remote_lib(target)
-                else target.address.spec_path)
-    envcopy = os.environ.copy()
-    envcopy['GOPATH'] = gopath
-    args = ['go', cmd] + cmd_flags + [pkg_path] + pkg_flags
-    p = subprocess.Popen(args, env=envcopy, stdout=sys.stdout, stderr=sys.stderr)
-    retcode = p.wait()
-    if retcode != 0:
-      raise TaskError('`{}` exited non-zero ({})'
-                      .format(' '.join(args), retcode))

contrib/go/src/python/pants/contrib/go/tasks/go_test.py

@@ -7,6 +7,8 @@
 
 import os
 
+from pants.base.workunit import WorkUnit
+
 from pants.contrib.go.tasks.go_workspace_task import GoWorkspaceTask
 
 
@@ -36,6 +38,12 @@ def execute(self):
     # we don't run the tests for _all_ dependencies of said package.
     for target in filter(self.is_local_src, self.context.target_roots):
       self.ensure_workspace(target)
-      self.run_go_cmd('test', self.get_gopath(target), target,
-                      cmd_flags=self.get_options().build_and_test_flags.split(),
-                      pkg_flags=self.get_passthru_args())
+      self._go_test(target)
+
+  def _go_test(self, target):
+    args = (self.get_options().build_and_test_flags.split()
+            + [target.address.spec_path]
+            + self.get_passthru_args())
+    self.go_dist.execute_go_cmd('test', gopath=self.get_gopath(target), args=args,
+                                workunit_factory=self.context.new_workunit,
+                                workunit_labels=[WorkUnit.TEST])

contrib/go/tests/python/pants_test/contrib/go/tasks/test_go_compile_integration.py

@@ -6,8 +6,8 @@
                         unicode_literals, with_statement)
 
 import os
+import subprocess
 
-import pytest
 from pants.util.contextutil import temporary_dir
 from pants_test.contrib.go.tasks.go_tool import GoTool
 from pants_test.pants_run_integration_test import PantsRunIntegrationTest
@@ -17,19 +17,19 @@
 
 class GoCompileIntegrationTest(PantsRunIntegrationTest):
 
-  @pytest.mark.skipif('not GoTool.go_installed()', reason='requires `go` command.')
   def test_go_compile_simple(self):
     with temporary_dir(root_dir=self.workdir_root()) as workdir:
       args = ['compile',
               'contrib/go/examples/src/go/libA']
       pants_run = self.run_pants_with_workdir(args, workdir)
       self.assert_success(pants_run)
-      # TODO(cgibb): Is it appropriate to be calling a GoTask static method from
-      # an integration test?
-      goos_goarch = GoTask.lookup_goos_goarch()
-      expected_files = set('contrib.go.examples.src.go.{libname}.{libname}/'
-                           'pkg/{goos_goarch}/contrib/go/examples/src/go/{libname}.a'
-                           .format(libname=libname, goos_goarch=goos_goarch)
-                           for libname in ('libA', 'libB', 'libC', 'libD', 'libE'))
-      self.assert_contains_exact_files(os.path.join(workdir, 'compile', 'go'),
-                                       expected_files)
+      # TODO(jsirois): Kill this check and the GoTool utility by using Go subsystem
+      if GoTool.go_installed():
+        goos = subprocess.check_output(['go', 'env', 'GOOS']).strip()
+        goarch = subprocess.check_output(['go', 'env', 'GOARCH']).strip()
+        expected_files = set('contrib.go.examples.src.go.{libname}.{libname}/'
+                             'pkg/{goos}_{goarch}/contrib/go/examples/src/go/{libname}.a'
+                             .format(libname=libname, goos=goos, goarch=goarch)
+                             for libname in ('libA', 'libB', 'libC', 'libD', 'libE'))
+        self.assert_contains_exact_files(os.path.join(workdir, 'compile', 'go'),
+                                         expected_files)

contrib/go/tests/python/pants_test/contrib/go/tasks/test_go_run_integration.py

@@ -5,14 +5,11 @@
 from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
                         unicode_literals, with_statement)
 
-import pytest
-from pants_test.contrib.go.tasks.go_tool import GoTool
 from pants_test.pants_run_integration_test import PantsRunIntegrationTest
 
 
 class GoRunIntegrationTest(PantsRunIntegrationTest):
 
-  @pytest.mark.skipif('not GoTool.go_installed()', reason='requires `go` command.')
   def test_go_run_simple(self):
     args = ['run',
             'contrib/go/examples/src/go/hello',

contrib/go/tests/python/pants_test/contrib/go/tasks/test_go_test_integration.py

@@ -5,14 +5,11 @@
 from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
                         unicode_literals, with_statement)
 
-import pytest
-from pants_test.contrib.go.tasks.go_tool import GoTool
 from pants_test.pants_run_integration_test import PantsRunIntegrationTest
 
 
 class GoTestIntegrationTest(PantsRunIntegrationTest):
 
-  @pytest.mark.skipif('not GoTool.go_installed()', reason='requires `go` command.')
   def test_go_test_simple(self):
     args = ['test',
             'contrib/go/examples/src/go/libA']