From adb70a5f398a89191aa1d425b4dd8b24bf2f0b84 Mon Sep 17 00:00:00 2001
From: Marc-Antoine Ruel <maruel@gmail.com>
Date: Mon, 16 Oct 2023 21:20:54 +0000
Subject: [PATCH] Fix shac check on Windows

Share the Go modules across installation, while using separate sumdb
(via separate GOPATH) to work around race conditions.

Change-Id: Iae8ae78a2cbd2cc8aa7c4d76ba35a8d8e5812c50
Reviewed-on: https://fuchsia-review.googlesource.com/c/shac-project/shac/+/931379
Fuchsia-Auto-Submit: Oliver Newman <olivernewman@google.com>
Reviewed-by: Oliver Newman <olivernewman@google.com>
Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>
Reviewed-by: Anthony Fandrianto <atyfto@google.com>
---
 checks/common.star | 38 +++++++++++++++++++++++++++-----------
 checks/go.star     | 17 ++++++++++-------
 2 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/checks/common.star b/checks/common.star
index 58758c0..8669ec5 100644
--- a/checks/common.star
+++ b/checks/common.star
@@ -14,27 +14,33 @@
 
 def go_install(ctx, pkg, version):
     """Runs `go install`."""
+    env = go_env(ctx)
+    cache = _go_cache(ctx)
 
-    env = go_env()
+    # Save executables in a common directory.
+    env["GOBIN"] = cache + "/gobin"
 
-    # TODO(olivernewman): Implement proper cache directories for shac instead of
-    # creating a `.tools` directory, which requires making the root directory
-    # writable.
-    env["GOBIN"] = ctx.scm.root + "/.tools/gobin"
+    tool_name = pkg.split("/")[-1]
+
+    # Setting GOPATH is necessary on Windows when USERPROFILE is not set.
+    # Use one GOPATH per tool. Since both GOBIN and GOMODCACHE are redirected,
+    # in practice only sumdb is going to be unique
+    env["GOPATH"] = cache + "/" + tool_name
 
-    # TODO(olivernewman): Stop using a separate GOPATH for each tool, and instead
-    # install the tools sequentially. Multiple concurrent `go install` runs on the
-    # same GOPATH results in race conditions.
     ctx.os.exec(
         ["go", "install", "%s@%s" % (pkg, version)],
         allow_network = True,
         env = env,
     ).wait()
 
-    tool_name = pkg.split("/")[-1]
-    return "%s/%s" % (env["GOBIN"], tool_name)
+    tool_exec = tool_name
+    if ctx.platform.os == "windows":
+        tool_exec += ".exe"
+    return "%s/%s" % (env["GOBIN"], tool_exec)
 
-def go_env():
+def go_env(ctx):
+    """Returns environment variables to use when running Go tooling."""
+    cache = _go_cache(ctx)
     return {
         # Disable cgo as it's not necessary and not all development platforms have
         # the necessary headers.
@@ -44,7 +50,17 @@ def go_env():
             # to fail on some machines.
             "-buildvcs=false",
         ]),
+        # Share the Go module cache to reduce downloads.
+        "GOMODCACHE": cache + "/mod",
         # TODO(olivernewman): The default gopackagesdriver is broken within an
         # nsjail.
         "GOPACKAGESDRIVER": "off",
     }
+
+def _go_cache(ctx):
+    """Returns the shared Go cache."""
+
+    # TODO(olivernewman): Implement proper cache directories for shac instead of
+    # creating a `.tools` directory, which requires making the root directory
+    # writable.
+    return ctx.scm.root + "/.tools"
diff --git a/checks/go.star b/checks/go.star
index e3309ad..4cc99eb 100644
--- a/checks/go.star
+++ b/checks/go.star
@@ -62,9 +62,9 @@ def _gosec(ctx, version = "v2.15.0", level = "error", exclude = [
     affected_files = set(ctx.scm.affected_files())
     exe = go_install(ctx, "github.com/securego/gosec/v2/cmd/gosec", version)
     res = ctx.os.exec(
-        [exe, "-fmt=json", "-quiet", "-exclude=%s" % ",".join(exclude), "-exclude-dir=.tools", "./..."],
+        [exe, "-fmt=json", "-quiet", "-exclude=%s" % ",".join(exclude), "-exclude-dir=.tools", "-exclude-dir=internal/engine/testdata", "./..."],
         ok_retcodes = (0, 1),
-        env = go_env(),
+        env = go_env(ctx),
     ).wait()
     if not res.retcode:
         return
@@ -112,7 +112,7 @@ def _ineffassign(ctx, version = "v0.0.0-20230107090616-13ace0543b28"):
     exe = go_install(ctx, "github.com/gordonklaus/ineffassign", version)
     res = ctx.os.exec(
         [exe, "./..."],
-        env = go_env(),
+        env = go_env(ctx),
         # ineffassign's README claims that it emits a retcode of 1 if it returns any
         # findings, but it actually emits a retcode of 3.
         # https://github.com/gordonklaus/ineffassign/blob/4cc7213b9bc8b868b2990c372f6fa057fa88b91c/ineffassign.go#L70
@@ -143,7 +143,7 @@ def _staticcheck(ctx, version = "v0.4.3"):
       will be rolled from time to time.
     """
     exe = go_install(ctx, "honnef.co/go/tools/cmd/staticcheck", version)
-    env = go_env()
+    env = go_env(ctx)
     res = ctx.os.exec(
         [exe, "-f=json", "./..."],
         ok_retcodes = [0, 1],
@@ -192,7 +192,7 @@ def _shadow(ctx, version = "v0.7.0"):
             "-json",
             "./...",
         ],
-        env = go_env(),
+        env = go_env(ctx),
     ).wait()
 
     # Example output:
@@ -241,7 +241,7 @@ def no_fork_without_lock(ctx):
             "-json",
             "./...",
         ],
-        env = go_env(),
+        env = go_env(ctx),
     ).wait().stdout)
 
     # Skip the "execsupport" package since it contains the wrappers around Run()
@@ -285,7 +285,7 @@ def govet(
         ] +
         ["-" + a for a in analyzers] +
         ["./..."],
-        env = go_env(),
+        env = go_env(ctx),
     ).wait().stderr
 
     # output is of the form:
@@ -299,6 +299,9 @@ def govet(
     current_package_lines = []
     lines = output.splitlines()
     for i, line in enumerate(lines):
+        if line.startswith("warning: "):
+            # warning: GOPATH set to GOROOT () has no effect
+            continue
         if not line.startswith("# "):
             current_package_lines.append(line)
             if i + 1 < len(lines):