switching from bazel test to coverage causes GoStdlib to be rebuilt with go1.20

[com6056]

What version of rules_go are you using?

v0.38.1 with go1.20

What version of gazelle are you using?

v0.29.0

What version of Bazel are you using?

5.4.0

Does this issue reproduce with the latest releases of all the above?

Yes

What operating system and processor architecture are you using?

macOS 13.2, M1 Max (but reproduces on our Linux systems as well)

Any other potentially useful information about your toolchain?

Related thread in Bazel Slack: https://bazelbuild.slack.com/archives/CDBP88Z0D/p1677506103789169
Potentially related issue: golang/go#58848

What did you do?

Ran bazel test //foo/bar:bar_test --nocache_test_results, see actions like this show up

    GoStdlib external/io_bazel_rules_go/stdlib_/pkg; 10s darwin-sandbox

Ran bazel coverage //foo/bar:bar_test --nocache_test_results, see actions like this show up again:

    GoStdlib external/io_bazel_rules_go/stdlib_/pkg; 10s darwin-sandbox

What did you expect to see?

The GoStdlib actions to not be run again

What did you see instead?

The GoStdlib actions ran again

[fmeum]

Go 1.20 stopped shipping the standard library in precompiled form and current version of Bazel aren't good at preventing rebuilds. The situation should improve with bazelbuild/bazel#16910, so if you can, please try last_green after that has been merged.

[com6056]

Bringing in the info from the Slack thread, seems to be a separate issue from bazelbuild/bazel#16910.

When diffing the 2 configs, we don't see anything standing out:

diff test_config coverage_config
1,2c1,2
< BuildConfigurationValue c3cee25049c1cec9cc8257e5ee797b1ced86dd67427ffa666cb2917c41b58d0d:
< Skyframe Key: BuildConfigurationKey[c3cee25049c1cec9cc8257e5ee797b1ced86dd67427ffa666cb2917c41b58d0d]
---
> BuildConfigurationValue d9700ee9a063bff991db657d797c04995659d0648b00bf8228d2071c73cc65ea:
> Skyframe Key: BuildConfigurationKey[d9700ee9a063bff991db657d797c04995659d0648b00bf8228d2071c73cc65ea]
34c34
<   collect_code_coverage: false
---
>   collect_code_coverage: true

I was able to reproduce with the following setup:

WORKSPACE:

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
    name = "io_bazel_rules_go",
    sha256 = "6b65cb7917b4d1709f9410ffe00ecf3e160edf674b78c54a894471320862184f",
    urls = [
        "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.39.0/rules_go-v0.39.0.zip",
        "https://github.com/bazelbuild/rules_go/releases/download/v0.39.0/rules_go-v0.39.0.zip",
    ],
)

load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")

go_rules_dependencies()

go_register_toolchains(version = "1.20.2")

BUILD.bazel:

load("@io_bazel_rules_go//go:def.bzl", "go_test")

go_test(
    name = "foo_test",
    srcs = ["foo_test.go"],
)

foo_test.go:

package main

import "testing"

func TestFoo(t *testing.T) {}

And then switch back and forth between bazel test //:foo_test and bazel coverage //:foo_test, you should see GoStdlib (among other recompilation actions) being run every time you switch.

@fmeum did find that we were leaking the coverage C flags leak into the stdlib build:

(cd /home/fhenneke/.cache/bazel/_bazel_fhenneke/a5f23fe7f7be068f86b3d79714a6b275/execroot/__main__ && \
  exec env - \
    CC=/usr/bin/gcc \
    CGO_CFLAGS='-U_FORTIFY_SOURCE -fstack-protector -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -fno-canonical-system-headers -Wno-builtin-macro-redefined -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted"' \
    CGO_ENABLED=1 \
    CGO_LDFLAGS='-fuse-ld=gold -Wl,-no-as-needed -Wl,-z,relro,-z,now -B/usr/bin -pass-exit-codes -lm' \
    GOARCH=amd64 \
    GOOS=linux \
    GOPATH='' \
    GOROOT=external/go_sdk \
    GOROOT_FINAL=GOROOT \
    PATH=/usr/bin:/bin \
  bazel-out/k8-opt-exec-2B5CBBC6-ST-1d3589a9f1f7/bin/external/go_sdk/builder_reset/builder stdlib -sdk external/go_sdk -installsuffix linux_amd64 -out bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/stdlib_ -experiment nocoverageredesign -package std -package runtime/cgo)

vs.

(cd /home/fhenneke/.cache/bazel/_bazel_fhenneke/a5f23fe7f7be068f86b3d79714a6b275/execroot/__main__ && \
  exec env - \
    CC=/usr/bin/gcc \
    CGO_CFLAGS='-U_FORTIFY_SOURCE -fstack-protector -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -fno-canonical-system-headers -Wno-builtin-macro-redefined -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted"' \
    CGO_ENABLED=1 \
    CGO_LDFLAGS='--coverage -fuse-ld=gold -Wl,-no-as-needed -Wl,-z,relro,-z,now -B/usr/bin -pass-exit-codes -lm' \
    GOARCH=amd64 \
    GOOS=linux \
    GOPATH='' \
    GOROOT=external/go_sdk \
    GOROOT_FINAL=GOROOT \
    PATH=/usr/bin:/bin \
  bazel-out/k8-opt-exec-2B5CBBC6-ST-96ff34cf280f/bin/external/go_sdk/builder_reset/builder stdlib -sdk external/go_sdk -installsuffix linux_amd64 -out bazel-out/k8-fastbuild/bin/external/io_bazel_rules_go/stdlib_ -experiment nocoverageredesign -package std -package runtime/cgo)

I attempted to patch this (which I might be totally wrong about how I did it) to see if I could get the flags for GoStdlib passed in CGO_CFLAGS and CGO_LDFLAGS to be the same but wasn't able to get anywhere unfortunately:

diff --git a/go/private/actions/stdlib.bzl b/go/private/actions/stdlib.bzl
index f8bfbeef..d4a7afbd 100644
--- a/go/private/actions/stdlib.bzl
+++ b/go/private/actions/stdlib.bzl
@@ -95,7 +95,7 @@ def _build_stdlib(go):
         ldflags = [
             option
             for option in extldflags_from_cc_toolchain(go)
-            if option not in ("-lstdc++", "-lc++")
+            if option not in ("-lstdc++", "-lc++", "--coverage", "-fprofile-instr-generate")
         ]
         env.update({
             "CGO_ENABLED": "1",
diff --git a/go/private/context.bzl b/go/private/context.bzl
index 1451895a..1c548488 100644
--- a/go/private/context.bzl
+++ b/go/private/context.bzl
@@ -105,6 +105,8 @@ _COMPILER_OPTIONS_DENYLIST = {
     "--coverage": None,
     "-ftest-coverage": None,
     "-fprofile-arcs": None,
+    "-fprofile-instr-generate": None,
+    "-fcoverage-mapping": None,
 }
 
 _LINKER_OPTIONS_DENYLIST = {

[fmeum]

I will submit a PR that drops the coverage flags from the stdlib action (they are useless), but that doesn't prevent the rebuild. I think that this hits a limitation of diff_against_baseline, but maybe we can make it smarter - will need to think more about this.

[fmeum]

#3521 and #3522 do everything we can from the Go side.

I submitted bazelbuild/bazel#18002 to improve the way Bazel computes the diff-based output directory suffix.

[linzhp]

This may be related to #2693

[fmeum]

This should be fixed with Bazel 6.4.0 and later with --experimental_output_directory_naming_scheme=diff_against_dynamic_baseline, which is the default with Bazel 7.