@fmeum I've managed to fix the tests by adding pure = "on" to my go_binary test target.

Doesn't have to be in this PR, but should we try to flip the default to PIE globally? I think that would match the vanilla go toolchain?

Doesn't have to be in this PR, but should we try to flip the default to PIE globally? I think that would match the vanilla go toolchain?

If that's actually the case then let's do it here so that we don't have to introduce the Android special case at all.

If that's actually the case then let's do it here so that we don't have to introduce the Android special case at all.

Looks like it's on by default in androis/ios/macos and windows without race, but not linux/bsds.

See https://github.com/golang/go/blob/694182d77b1a0e3676214ad0e361bdbdafde33a1/src/internal/platform/supported.go#L235-L253

@alextercete Could you update the condition to match what native Go does?

@dzbarsky The failures look like cmd/internal/cov isn't available for all platforms - we may have to limit where we request it.

@dzbarsky The failures look like cmd/internal/cov isn't available for all platforms - we may have to limit where we request it.

So there's a few things going on here:

1. cmd/internal/cov only exists in go 1.20+, but some of our tests indicate go 1.16 in their go.mod. I'm not sure if we have an actual minimum version here, but we might as well condition the inclusion of those packages on 1.20+

This patch can do it:

--- a/go/private/actions/stdlib.bzl
+++ b/go/private/actions/stdlib.bzl
@@ -143,9 +143,11 @@ def _build_stdlib(go):
     if not go.mode.pure:
         args.add("-package", "runtime/cgo")
 
-    # For bzltestutil's coverage support.
-    args.add("-package", "cmd/internal/cov")
-    args.add("-package", "cmd/internal/bio")
+    version = parse_version(go.sdk.version)
+    if version and version[0] >= 1 and version[1] >= 20:
+        # For bzltestutil's coverage support.
+        args.add("-package", "cmd/internal/cov")
+        args.add("-package", "cmd/internal/bio")

(Pulled this out into #4489)

2. The reason this PR triggers this at all is because of the following check, which make us think we can't use the prebuilt SDK and need to build from source instead:
   

   rules_go/go/private/actions/stdlib.bzl

   Line 63 in e3ec029

   go.mode.linkmode == LINKMODE_NORMAL)

   
   I think that line should be changed to go.mode.linkmode in (LINKMODE_NORMAL, LINKMODE_PIE) since the prebuilt SDK should be fine to use for PIE linking.

Hope that helps!

@dzbarsky I tried your suggestion to fix the CI failures on Windows, but they didn't work. Any other ideas?

cc @linzhp for mocks

We can disable that target on Windows if it's failing. We are moving the mock support to https://github.com/uber-go/mock

I'm not sure if there are any targets I can disable. The new ones I've added are all tagged as manual, so shouldn't affect the build.

I've rebased onto the latest master and tried a few different ideas, but I still can't make CI pass on Windows.

@fmeum @dzbarsky Any advice on how to proceed?

@alextercete can you try marking these targets as

target_compatible_with = select({
    "@platforms//os:windows": ["@platforms//:incompatible"],
    "//conditions:default": [],
})

It turns out there was a legitimate failure on Windows related to testing for stripped portable executables. I've adjusted the test and now CI is green.

@alextercete thanks for powering through this one!