Ignore pre-compiled stdlib only on 1.19 with experiments

[linzhp]

What type of PR is this?
Bug fix

What does this PR do? Why is it needed?
#3398 allowed people to use boringcrypto on 1.19 by ignoring the pre-compile stdlib when boringcrypto is request. However, #3401 over generalized the intention by ignore pre-compile stdlib for all experimental flags for all Go versions. In addition, rules_go add a "nocoverageredesign" experimental flag for Go 1.20. This means rules_go always ignores pre-compiled stdlib that users provide, even it's compiled correctly.

This restore the intention of #3398 by only ignoring pre-compiled stdlib on Go 1.19 with experiments. This allows people to add precompiled stdlib on 1.20+ to improve build performance.

Other notes for review

[fmeum]

Does this mean that the stdlib doesn't need to be rebuilt for other experiments? I think I don't fully understand why we need to rebuild in this one particular case but not others.

[linzhp]

There can be flags disabling experiments like "nocoverageredesign", "nounified" in go.sdk.experiments. These should be fine with the pre-compiled binary.

People can also pre-compile stdlib for Go 1.20 with some experiments on or off, which is the same as with the list of flags in go.sdk.experiments. Rules_go also added "nocoverageredesign" automatically. If we skip all pre-compiled stdlib whenever go.sdk.experiments is not empty, people can no longer use pre-compile stdlib anymore, even they compiled it correctly.

I don't object to the idea of not checking go.sdk.experiments at all, even for 1.19 and boryingcrypto. When pre-compile stdlib exists, it's a strong indication that people want to use it as much as possible. We should only rule out a few cases when we are very sure it it won't work, such as goos, goarch mismatch. The list of experimental flags are not in one of them.

[fmeum]

What happens if we assume the pre-compiled stdlib is fine but it isn't supported? Will it still be built in the correct version, just without us caching it properly? If that's the case we can remove the check for experiments entirely, but the current form of the PR is definitely fine.

[linzhp]

What happens if we assume the pre-compiled stdlib is fine but it isn't supported?

When is a stdlib not supported?

[fmeum]

What I'm wondering is what happens if we do not trigger a build of the standard library (by taking this branch:

rules_go/go/private/actions/stdlib.bzl

Line 42 in e558f56

source["stdlib"] = _sdk_stdlib(go)

), but there is no pre-compiled stdlib for the desired configuration. Does that fail or do we just pick up an incorrect stdlib (e.g. one for Linux when macOS is requested or a precompiled version without race instrumentation).

[linzhp]

I think there will be linking errors when the pre-compiled stdlib binary doesn't match the compilation mode for other packages. @sywhang to confirm

[fmeum]

I see, can this happen with Go experiments? In that case not triggering recompilation when it's needed could also result in linker errors. But I assume you confirmed that is not the case for this particular change, so that one seems safe to make.

[fmeum]

I see, can this happen with Go experiments? In that case not triggering recompilation when it's needed could also result in linker errors. But I assume you confirmed that is not the case for this particular change, so that one seems safe to make.

[linzhp]

It could happen with Go experiments, so if users want to pre-compile stdlib, it's the users' responsibility to give rules_go a pre-compiled binary that matches the Go experiment flags defined at Go sdk rules.

We can run go tool pack p $(bazel info output_base)/external/go_sdk/pkg/darwin_arm64/os.a|head -n 1 to determine the list of Go experiments os.a is compiled with, but I am not sure if rules_go can do that.

The other way to implement this PR is to recompile when Go version is 1.19 or older and go.sdk.experiments is not empty, because we are more confident that stdlib is compiled without Go experiments. For 1.20 or newer, the SDK doesn't come with pre-compiled stdlib, so it must be from users and rules_go can assume that users know what they are doing.

[fmeum]

The other way to implement this PR is to recompile when Go version is 1.19 or older and go.sdk.experiments is not empty, because we are more confident that stdlib is compiled without Go experiments. For 1.20 or newer, the SDK doesn't come with pre-compiled stdlib, so it must be from users and rules_go can assume that users know what they are doing.

That sounds good to me!

[linzhp]

I had to expose parse_version so I can determine the version range. Can you take another look?

[katre]

Thanks for this: changing my go_sdk.download(version = "1.20.3") to go_sdk.download(version = "1.19.13") cut my build times for a single go_binary from 69s to 8s.