From 1286800d34cc6f40a46b6e84eea0cffa503b5439 Mon Sep 17 00:00:00 2001
From: Jay Conrod <jay@engflow.com>
Date: Fri, 2 Jun 2023 02:16:03 -0700
Subject: [PATCH] go_download_sdk: apply extraction workaround to zips on
 non-windows OSs (#3563)

* go_download_sdk: apply extraction workaround to zips on non-windows OSs

The Go distribution contains at least one test file with an invalid
unicode name. Bazel cannot extract the distribution archive on some
operating systems and file systems; Darwin with AFS at least is affected.

For .tar.gz files, we workaround the failure in ctx.download_and_extract
by using the native system tar.

This PR applies a similar workaround for .zip files on non-Windows OSs.
Windows itself is not affected (ctx.download_and_extract works),
so the workaround is not applied there. This is only really needed
when you have a Darwin host and a Windows executor (don't ask).

For #2771

* use rename_files; rewrite comment

* version check
---
 go/private/sdk.bzl                            | 41 ++++++++++++++++---
 .../go_download_sdk/go_download_sdk_test.go   | 26 +++++++++++-
 2 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/go/private/sdk.bzl b/go/private/sdk.bzl
index 8a9bc642d6..1e6fadd85f 100644
--- a/go/private/sdk.bzl
+++ b/go/private/sdk.bzl
@@ -20,6 +20,10 @@ load(
     "//go/private:nogo.bzl",
     "go_register_nogo",
 )
+load(
+    "//go/private/skylib/lib:versions.bzl",
+    "versions",
+)
 
 MIN_SUPPORTED_VERSION = (1, 14, 0)
 
@@ -405,14 +409,27 @@ def _register_toolchains(repo):
 def _remote_sdk(ctx, urls, strip_prefix, sha256):
     if len(urls) == 0:
         fail("no urls specified")
+    host_goos, _ = detect_host_platform(ctx)
+
     ctx.report_progress("Downloading and extracting Go toolchain")
+
+    # TODO(#2771): After bazelbuild/bazel#18448 is merged and available in
+    # the minimum supported version of Bazel, remove the workarounds below.
+    #
+    # Go ships archives containing some non-ASCII file names, used in
+    # test cases for Go's build system. Bazel has a bug extracting these
+    # archives on certain file systems (macOS AFS at least, possibly also
+    # Docker on macOS with a bind mount).
+    #
+    # For .tar.gz files (available for most platforms), we work around this bug
+    # by using the system tar instead of ctx.download_and_extract.
+    #
+    # For .zip files, we use ctx.download_and_extract but with rename_files,
+    # changing certain paths that trigger the bug. This is only available
+    # in Bazel 6.0.0+ (bazelbuild/bazel#16052). The only situation where
+    # .zip files are needed seems to be a macOS host using a Windows toolchain
+    # for remote execution.
     if urls[0].endswith(".tar.gz"):
-        # BUG(#2771): Use a system tool to extract the archive instead of
-        # Bazel's implementation. With some configurations (macOS + Docker +
-        # some particular file system binding), Bazel's implementation rejects
-        # files with invalid unicode names. Go has at least one test case with a
-        # file like this, but we haven't been able to reproduce the failure, so
-        # instead, we use this workaround.
         if strip_prefix != "go":
             fail("strip_prefix not supported")
         ctx.download(
@@ -424,6 +441,18 @@ def _remote_sdk(ctx, urls, strip_prefix, sha256):
         if res.return_code:
             fail("error extracting Go SDK:\n" + res.stdout + res.stderr)
         ctx.delete("go_sdk.tar.gz")
+    elif (urls[0].endswith(".zip") and
+          host_goos != "windows" and
+          versions.is_at_least("6.0.0", versions.get())):
+        ctx.download_and_extract(
+            url = urls,
+            stripPrefix = strip_prefix,
+            sha256 = sha256,
+            rename_files = {
+                "go/test/fixedbugs/issue27836.dir/\336foo.go": "go/test/fixedbugs/issue27836.dir/thfoo.go",
+                "go/test/fixedbugs/issue27836.dir/\336main.go": "go/test/fixedbugs/issue27836.dir/thmain.go",
+            },
+        )
     else:
         ctx.download_and_extract(
             url = urls,
diff --git a/tests/core/go_download_sdk/go_download_sdk_test.go b/tests/core/go_download_sdk/go_download_sdk_test.go
index 4a5999ee10..61bd05b89f 100644
--- a/tests/core/go_download_sdk/go_download_sdk_test.go
+++ b/tests/core/go_download_sdk/go_download_sdk_test.go
@@ -57,6 +57,7 @@ func Test(t *testing.T) {
 	for _, test := range []struct {
 		desc, rule       string
 		optToWantVersion map[string]string
+		fetchOnly        string
 	}{
 		{
 			desc: "version",
@@ -70,7 +71,8 @@ go_download_sdk(
 
 `,
 			optToWantVersion: map[string]string{"": "go1.16"},
-		}, {
+		},
+		{
 			desc: "custom_archives",
 			rule: `
 load("@io_bazel_rules_go//go:deps.bzl", "go_download_sdk")
@@ -114,6 +116,21 @@ go_download_sdk(
 				"--@io_bazel_rules_go//go/toolchain:sdk_version=1.17.1": "go1.17.1",
 			},
 		},
+		{
+			// Cover workaround for #2771.
+			desc: "windows_zip",
+			rule: `
+load("@io_bazel_rules_go//go:deps.bzl", "go_download_sdk")
+
+go_download_sdk(
+    name = "go_sdk",
+		goarch = "amd64",
+		goos = "windows",
+		version = "1.20.4",
+)
+`,
+			fetchOnly: "@go_sdk//:BUILD.bazel",
+		},
 	} {
 		t.Run(test.desc, func(t *testing.T) {
 			origWorkspaceData, err := ioutil.ReadFile("WORKSPACE")
@@ -143,6 +160,13 @@ go_register_toolchains()
 				}
 			}()
 
+			if test.fetchOnly != "" {
+				if err := bazel_testing.RunBazel("fetch", test.fetchOnly); err != nil {
+					t.Fatal(err)
+				}
+				return
+			}
+
 			for opt, wantVersion := range test.optToWantVersion {
 				t.Run(wantVersion, func(t *testing.T) {
 					args := []string{