Reset request_nogo to its default value in the stdlib transition (#4705)
**What type of PR is this?**
Bug fix
**What does this PR do? Why is it needed?**
`go_stdlib_transition` resets `//go/private:request_nogo` to `False`
along with the other tool settings. The standard library never runs
nogo, though, so the only effect of that reset is to move
`@io_bazel_rules_go//:stdlib` into a configuration of its own: neither
the configuration of a target built with default settings (e.g. a plain
`bazel build //some:go_binary`) nor that of a tool built via a plain
exec transition (e.g. a genrule tool) has the setting set.
That separate configuration isn't free: every Go rule resolves the C++
toolchain, so the toolchain and everything it depends on are analyzed
and, for toolchains that build runtimes from source, built once more.
Since `GoStdlib` takes the toolchain files as inputs, that duplicate
build also sits in front of every Go compilation on the critical path of
a cold build. In a build of BuildBuddy's executor on macOS with a
hermetic LLVM toolchain, this is a second from-source build of
compiler-rt (281 C compiles) on both the target and the exec side.
This resets the setting to its default value (`True`) instead of the
tool value. With that, the stdlib of a target built with default
settings shares the target's configuration, and the stdlib of nogo
(whose own configuration has `request_nogo = False`) shares the
configuration of plain exec tools rather than keeping one of its own.
There is no cycle risk: the stdlib rule only depends on `//:go_config`
and the toolchains, none of which reach nogo.
The same reasoning applies to two more places that set `request_nogo =
False` for targets that never run nogo, and the follow-up commits fix
those too so that a default build has no configuration forks left at
all:
* `non_go_tool_transition` inherited the value from the dict it shares
with `go_tool_transition`, which gave the builder, pack, the legacy
protoc toolchain and every `proto_library` a `go_proto_library` depends
on a configuration of their own.
* `go_context_data` transitioned its `coverdata` dependency, a
`go_tool_library`, which moved coverdata, the C++ toolchain it resolves
and the Go SDK targets of its toolchain into a configuration of their
own, once on the target and once on the exec side. The transition dates
back to #4016 and was also relied on by `cgo_context_data` (#4512),
which has since been removed (#4599).
**Which issues(s) does this PR fix?**
None filed.
**Other notes for review**
* Two tests for the stdlib, one per claim. On #4704 alone they fail with
`no stdlib is built in the configuration of //:plain, the stdlib
configurations differ from it in: private:request_nogo=null vs false`
and `a stdlib reachable from //:tool_user is not built in the
configuration of //:plain, differing in: private:request_nogo=null vs
false`.
* A new test verifies the general property: every target reachable from
a Go binary or from a genrule tool is built in the target configuration,
the exec configuration or the configuration of nogo. On the first commit
alone it fails with `121 targets are built in a configuration that
differs from the exec configuration in private:request_nogo=null vs
false: @io_bazel_rules_go//go/tools/coverdata:coverdata,
@go_sdk//:pack.exe, @go_sdk//:builder, ...` and the corresponding
message for the target configuration.
* `static_runtime_lib_inputs_test` expected the C++ runtime archive
twice among the link inputs, once per configuration of the binary and
its stdlib. With both in one configuration there is a single copy, which
is what the test asserts now; `stdlib_runtime_lib_inputs_test` still
covers the two-configuration case via a tag the stdlib transition drops.
* The test helper that reports differing settings now also covers
`//go/private` settings (prefixed with `private:`), since that is the
package the difference lives in.
4 files changed