Remove the `request_nogo` setting (#4761) **What type of PR is this?** Other **What does this PR do? Why is it needed?** `//go/private:request_nogo` was added in #2922 so that tools built in the legacy host configuration, which couldn't apply the transition `go_context_data` used to set it to `True`, would fall back to the noop nogo. Host mode is gone, #4611 removed that transition and flipped the default to `True`, and since #4755 no rules_go transition sets it at all. `nogo_active` therefore only ever depends on `bootstrap_nogo`, the actual cycle guard that `go_tool_transition` sets for nogo and other Go tools. This removes the setting and leaves `bootstrap_nogo` as the only nogo setting: * `nogo_active` only checks `bootstrap_nogo`. * `bootstrap_nogo = False` is part of the dict of default values that the tool transitions start from, and only `go_tool_transition` sets it to `True`. The separate dict for the tool value and the overrides in the non-Go tool and stdlib transitions that reset it to `False` are gone. **Which issues(s) does this PR fix?** **Other notes for review**
diff --git a/go/private/BUILD.bazel b/go/private/BUILD.bazel index c9426e6..0c9a4b0 100644 --- a/go/private/BUILD.bazel +++ b/go/private/BUILD.bazel
@@ -162,8 +162,8 @@ ], ) -# Usually false. This is only true when we are building nogo itself in which -# because that rule uses an incoming transition to switch this to true. +# Usually false. go_tool_transition sets this to true when building nogo and +# other Go tools. bool_setting( name = "bootstrap_nogo", build_setting_default = False, @@ -171,24 +171,13 @@ visibility = ["//visibility:public"], ) -# Defaults to True. No rules_go transition changes it; setting it to False on -# the command line turns nogo off everywhere. -bool_setting( - name = "request_nogo", - build_setting_default = True, - scope = "universal", - visibility = ["//visibility:public"], -) - # Controls whether nogo alias will reference a noop or the configured nogo. -# True (the default) means nogo runs. bootstrap_nogo, which go_tool_transition -# sets when building nogo and other Go tools, makes this False, so nogo's own -# dependencies resolve the nogo alias to a noop (circular dep guard). +# bootstrap_nogo makes this False, so nogo's own dependencies resolve the nogo +# alias to a noop (circular dep guard). config_setting( name = "nogo_active", flag_values = { ":bootstrap_nogo": "False", - ":request_nogo": "True", }, visibility = ["//visibility:public"], )
diff --git a/go/private/rules/transition.bzl b/go/private/rules/transition.bzl index 3c2e79d..828aa78 100644 --- a/go/private/rules/transition.bzl +++ b/go/private/rules/transition.bzl
@@ -183,12 +183,8 @@ # this so that their configurations differ from the default one in as few # settings as possible, as each difference forks the configuration of # everything that is resolved in it, e.g. the C++ toolchain. -_common_reset_transition_dict = dict({ - # Not False: bootstrap_nogo already keeps nogo from running for tools, and - # a further difference from the default would give them a configuration of - # their own, along with everything they resolve in it, e.g. the C++ - # toolchain. - "//go/private:request_nogo": True, +_reset_transition_dict = dict({ + "//go/private:bootstrap_nogo": False, "//go/config:static": False, "//go/config:msan": False, "//go/config:race": False, @@ -199,12 +195,6 @@ "//go/config:pgoprofile": Label("//go/config:empty"), }, **{setting: "" for setting in _SETTING_KEY_TO_ORIGINAL_SETTING_KEY.values()}) -# The defaults plus bootstrap_nogo, the only setting with a tool value: it -# keeps nogo from running on nogo's own dependencies. -_reset_transition_dict = dict(_common_reset_transition_dict, **{ - "//go/private:bootstrap_nogo": True, -}) - _reset_transition_keys = sorted(_reset_transition_dict.keys()) _stdlib_keep_keys = sorted([ @@ -234,6 +224,10 @@ new_settings = dict(settings, **_reset_transition_dict) for key in TOOL_INHERITED_SETTING_KEYS: new_settings[key] = _setting_before_go_transition(settings, key) + + # Keeps nogo from running on nogo's own dependencies, which would be a + # dependency cycle. + new_settings["//go/private:bootstrap_nogo"] = True return new_settings go_tool_transition = transition( @@ -256,12 +250,7 @@ Examples: This transition is applied to attributes referencing proto_library targets or protoc directly. """ - settings = dict(settings, **_reset_transition_dict) - - # Non-Go tools don't run nogo, so use the default value rather than the - # tool value to share the configuration of plain exec tools. - settings["//go/private:bootstrap_nogo"] = False - return settings + return dict(settings, **_reset_transition_dict) non_go_tool_transition = transition( implementation = _non_go_tool_transition_impl, @@ -282,11 +271,6 @@ if label not in _stdlib_keep_keys: settings[label] = value settings["//go/config:tags"] = [t for t in settings["//go/config:tags"] if t in _TAG_AFFECTS_STDLIB] - - # The standard library doesn't run nogo, so use the default value rather - # than the tool value to share the configuration of both regular targets - # and plain exec tools. - settings["//go/private:bootstrap_nogo"] = False settings["//command_line_option:collect_code_coverage"] = False return settings
diff --git a/tests/core/nogo/exec_config/exec_config_test.go b/tests/core/nogo/exec_config/exec_config_test.go index 5e3dde7..2907303 100644 --- a/tests/core/nogo/exec_config/exec_config_test.go +++ b/tests/core/nogo/exec_config/exec_config_test.go
@@ -66,8 +66,8 @@ // Under --incompatible_exclude_starlark_flags_from_exec_config, that flag would // be reset to its default across the cfg = "exec" edge into nogo's // dependencies, re-activating nogo on them and reintroducing the cycle. Marking -// bootstrap_nogo (and request_nogo) with scope = "universal" exempts them from -// that reset, which is what this test guards. +// bootstrap_nogo with scope = "universal" exempts it from that reset, which is +// what this test guards. func TestNoCycleWithExcludedStarlarkFlags(t *testing.T) { const flag = "--incompatible_exclude_starlark_flags_from_exec_config" err := bazel_testing.RunBazel("build", "//:lib", flag)
diff --git a/tests/core/transition/tool_settings_test.go b/tests/core/transition/tool_settings_test.go index 06ef286..5d685f1 100644 --- a/tests/core/transition/tool_settings_test.go +++ b/tests/core/transition/tool_settings_test.go
@@ -462,8 +462,8 @@ // TestToolConfigurationDiffersFromExecOnlyInBootstrapNogo verifies that nogo, // which is built with go_tool_transition, differs from a plain exec tool in no // rules_go setting but bootstrap_nogo, the one that keeps it from analyzing -// itself. Any further difference, such as request_nogo, forks the -// configuration of everything the tool resolves in it, e.g. the C++ toolchain. +// itself. Any further difference forks the configuration of everything the +// tool resolves in it, e.g. the C++ toolchain. // Options outside rules_go's settings are not compared. func TestToolConfigurationDiffersFromExecOnlyInBootstrapNogo(t *testing.T) { diffs := getGoOptions(t, execToolConfig(t), nogoConfig(t))