Ensure that GOBIN is not set when installing a golang hook

ashanbrown · ashanbrown · commit fe5390c068dc · 2019-01-27T13:34:47.000-08:00
If GOBIN is set, it will be used as the install path instead of the first item
from GOPATH followed by "/bin".  If it is used, commands will not be isolated
between different repos.
diff --git a/pre_commit/languages/golang.py b/pre_commit/languages/golang.py
@@ -7,6 +7,7 @@
 import pre_commit.constants as C
 from pre_commit import git
 from pre_commit.envcontext import envcontext
+from pre_commit.envcontext import UNSET
 from pre_commit.envcontext import Var
 from pre_commit.languages import helpers
 from pre_commit.util import clean_path_on_failure
@@ -21,6 +22,7 @@
 
 def get_env_patch(venv):
     return (
+        ('GOBIN', UNSET),
         ('PATH', (os.path.join(venv, 'bin'), os.pathsep, Var('PATH'))),
     )
 
@@ -69,6 +71,7 @@ def install_environment(prefix, version, additional_dependencies):
         else:
             gopath = directory
         env = dict(os.environ, GOPATH=gopath)
+        env.pop('GOBIN', None)
         cmd_output('go', 'get', './...', cwd=repo_src_dir, env=env)
         for dependency in additional_dependencies:
             cmd_output('go', 'get', dependency, cwd=repo_src_dir, env=env)
diff --git a/tests/repository_test.py b/tests/repository_test.py
@@ -14,6 +14,7 @@
 from pre_commit import parse_shebang
 from pre_commit.clientlib import CONFIG_SCHEMA
 from pre_commit.clientlib import load_manifest
+from pre_commit.envcontext import envcontext
 from pre_commit.languages import golang
 from pre_commit.languages import helpers
 from pre_commit.languages import node
@@ -71,7 +72,7 @@ def _test_hook_repo(
     path = make_repo(tempdir_factory, repo_path)
     config = make_config_from_repo(path, **(config_kwargs or {}))
     ret = _get_hook(config, store, hook_id).run(args)
-    assert ret[0] == expected_return_code
+    assert ret[0] == expected_return_code, "output was: {}".format(ret[1])
     assert _norm_out(ret[1]) == expected
 
 
@@ -267,6 +268,16 @@ def test_golang_hook(tempdir_factory, store):
     )
 
 
+def test_golang_hook_still_works_when_gobin_is_set(tempdir_factory, store):
+    gobin_dir = tempdir_factory.get()
+    with envcontext([('GOBIN', gobin_dir)]):
+        _test_hook_repo(
+            tempdir_factory, store, 'golang_hooks_repo',
+            'golang-hook', [], b'hello world\n',
+        )
+    assert os.listdir(gobin_dir) == [], "hook should not be installed in $GOBIN"
+
+
 def test_rust_hook(tempdir_factory, store):
     _test_hook_repo(
         tempdir_factory, store, 'rust_hooks_repo',
