Address review comments

go/private/rules/nogo.bzl

@@ -25,7 +25,7 @@ load(
 )
 load(
     "//go/private/rules:transition.bzl",
-    "go_exec_reset_transition",
+    "go_tool_transition",
 )
 
 def _nogo_impl(ctx):
@@ -103,7 +103,7 @@ _nogo = rule(
         ),
     },
     toolchains = ["@io_bazel_rules_go//go:toolchain"],
-    cfg = go_exec_reset_transition,
+    cfg = go_tool_transition,
 )
 
 def nogo(name, visibility = None, **kwargs):

go/private/rules/transition.bzl

@@ -238,12 +238,12 @@ _reset_transition_dict = dict(_common_reset_transition_dict, **{
 
 _reset_transition_keys = sorted([filter_transition_label(label) for label in _reset_transition_dict.keys()])
 
-def _go_exec_reset_transition_impl(settings, attr):
+def _go_tool_transition_impl(settings, attr):
     """Sets most Go settings to default values (use for external Go tools).
 
-    go_exec_reset_transition sets all of the //go/config settings to their
-    default values and disables nogo. This is used for Go tool binaries like
-    nogo itself. Tool binaries shouldn't depend on the link mode or tags of the
+    go_tool_transition sets all of the //go/config settings to their default
+    values and disables nogo. This is used for Go tool binaries like nogo
+    itself. Tool binaries shouldn't depend on the link mode or tags of the
     target configuration and neither the tools nor the code they potentially
     generate should be subject to nogo's static analysis. This transition
     doesn't change the platform (goos, goarch), but tool binaries should also
@@ -255,42 +255,37 @@ def _go_exec_reset_transition_impl(settings, attr):
         settings[filter_transition_label(label)] = value
     return settings
 
-go_exec_reset_transition = transition(
-    implementation = _go_exec_reset_transition_impl,
+go_tool_transition = transition(
+    implementation = _go_tool_transition_impl,
     inputs = _reset_transition_keys,
     outputs = _reset_transition_keys,
 )
 
-_non_go_exec_reset_transition_dict = dict(_common_reset_transition_dict, **{
-    "@io_bazel_rules_go//go/private:bootstrap_nogo": False,
-})
-
-_non_go_exec_reset_transition_keys = sorted([filter_transition_label(label) for label in _non_go_exec_reset_transition_dict.keys()])
-
-def _go_non_go_exec_reset_tool_transition_impl(settings, attr):
+def _non_go_tool_transition_impl(settings, attr):
     """Sets all Go settings to default values (use for external non-Go tools).
 
-    go_non_go_exec_reset_transition sets all of the //go/config settings as well
-    as the nogo settings to their default values. This is used for all tools that
-    are not themselves targets created from rules_go rules and thus do not read
+    non_go_tool_transition sets all of the //go/config settings as well as the
+    nogo settings to their default values. This is used for all tools that are
+    not themselves targets created from rules_go rules and thus do not read
     these settings. Resetting all of them to defaults prevents unnecessary
     configuration changes for these targets that could cause rebuilds.
 
-    Examples: This transition is applied to attributes referening proto_library
+    Examples: This transition is applied to attributes referencing proto_library
     targets or protoc directly.
     """
     settings = dict(settings)
-    for label, value in _non_go_exec_reset_transition_dict.items():
+    for label, value in _reset_transition_dict.items():
         settings[filter_transition_label(label)] = value
+    settings[filter_transition_label("@io_bazel_rules_go//go/private:bootstrap_nogo")] = False
     return settings
 
-go_non_go_exec_reset_tool_transition = transition(
-    implementation = _go_non_go_exec_reset_tool_transition_impl,
-    inputs = _non_go_exec_reset_transition_keys,
-    outputs = _non_go_exec_reset_transition_keys,
+non_go_tool_transition = transition(
+    implementation = _non_go_tool_transition_impl,
+    inputs = _reset_transition_keys,
+    outputs = _reset_transition_keys,
 )
 
-def _go_exec_reset_target_impl(ctx):
+def _go_reset_target_impl(ctx):
     t = ctx.attr.dep[0]  # [0] seems to be necessary with the transition
     providers = [t[p] for p in [GoLibrary, GoSource, GoArchive] if p in t]
 
@@ -324,51 +319,50 @@ def _go_exec_reset_target_impl(ctx):
     )
     return providers
 
-go_exec_reset_target = rule(
-    implementation = _go_exec_reset_target_impl,
+go_reset_target = rule(
+    implementation = _go_reset_target_impl,
     attrs = {
         "dep": attr.label(
             mandatory = True,
-            cfg = go_exec_reset_transition,
+            cfg = go_tool_transition,
         ),
         "_allowlist_function_transition": attr.label(
             default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
         ),
     },
-    doc = """Forwards providers from a target and applies go_exec_reset_transition.
+    doc = """Forwards providers from a target and applies go_tool_transition.
 
-go_exec_reset_target depends on a single target, built using go_exec_reset_transition.
-It forwards Go providers and DefaultInfo.
+go_reset_target depends on a single target, built using go_tool_transition. It
+forwards Go providers and DefaultInfo.
 
-This is used to work around a problem with building tools: tools should be
+This is used to work around a problem with building tools: Go tools should be
 built with 'cfg = "exec"' so they work on the execution platform, but we also
-need to apply go_exec_reset_transition, so for example, a tool isn't built as a
-shared library with race instrumentation. This acts as an intermediately rule
-so we can apply both transitions.
+need to apply go_tool_transition so that e.g. a tool isn't built as a shared
+library with race instrumentation. This acts as an intermediate rule that allows
+to apply both both transitions.
 """,
 )
 
-go_non_go_exec_reset_target = rule(
-    implementation = _go_exec_reset_target_impl,
+non_go_reset_target = rule(
+    implementation = _go_reset_target_impl,
     attrs = {
         "dep": attr.label(
             mandatory = True,
-            cfg = go_non_go_exec_reset_tool_transition,
+            cfg = non_go_tool_transition,
         ),
         "_allowlist_function_transition": attr.label(
             default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
         ),
     },
-    doc = """Forwards providers from a target and applies go_exec_reset_transition.
+    doc = """Forwards providers from a target and applies non_go_tool_transition.
 
-go_exec_reset_target depends on a single target, built using go_exec_reset_transition.
-It forwards Go providers and DefaultInfo.
+non_go_reset_target depends on a single target, built using
+non_go_tool_transition. It forwards Go providers and DefaultInfo.
 
-This is used to work around a problem with building tools: tools should be
-built with 'cfg = "exec"' so they work on the execution platform, but we also
-need to apply go_exec_reset_transition, so for example, a tool isn't built as a
-shared library with race instrumentation. This acts as an intermediately rule
-so we can apply both transitions.
+This is used to work around a problem with building tools: Non-Go tools should
+be built with 'cfg = "exec"' so they work on the execution platform, but they
+also shouldn't be affected by Go-specific config changes applied by
+go_transition.
 """,
 )
 

go/tools/builders/BUILD.bazel

@@ -1,5 +1,5 @@
 load("//go:def.bzl", "go_binary", "go_source", "go_test")
-load("//go/private/rules:transition.bzl", "go_exec_reset_target")
+load("//go/private/rules:transition.bzl", "go_reset_target")
 
 go_test(
     name = "filter_test",
@@ -93,7 +93,7 @@ go_binary(
     visibility = ["//visibility:public"],
 )
 
-go_exec_reset_target(
+go_reset_target(
     name = "go_path",
     dep = ":go_path-bin",
     visibility = ["//visibility:public"],
@@ -127,7 +127,7 @@ go_binary(
     visibility = ["//visibility:private"],
 )
 
-go_exec_reset_target(
+go_reset_target(
     name = "go-protoc",
     dep = ":go-protoc-bin",
     visibility = ["//visibility:public"],

proto/BUILD.bazel

@@ -5,7 +5,7 @@ load(
 )
 load(
     "//go/private/rules:transition.bzl",
-    "go_non_go_exec_reset_target",
+    "non_go_reset_target",
 )
 load(
     "//proto/wkt:well_known_types.bzl",
@@ -116,8 +116,8 @@ go_proto_compiler(
     ] + WELL_KNOWN_TYPE_RULES.values(),
 )
 
-go_non_go_exec_reset_target(
-    name = "protoc_reset_target_",
+non_go_reset_target(
+    name = "protoc",
     dep = "@com_google_protobuf//:protoc",
     visibility = ["//visibility:public"],
 )

proto/compiler.bzl

@@ -23,7 +23,7 @@ load(
 )
 load(
     "//go/private/rules:transition.bzl",
-    "go_exec_reset_target",
+    "go_reset_target",
 )
 
 GoProtoCompiler = provider(
@@ -200,7 +200,7 @@ _go_proto_compiler = rule(
         "_protoc": attr.label(
             executable = True,
             cfg = "exec",
-            default = "//proto:protoc_reset_target_",
+            default = "//proto:protoc",
         ),
         "_go_context_data": attr.label(
             default = "//:go_context_data",
@@ -212,7 +212,7 @@ _go_proto_compiler = rule(
 def go_proto_compiler(name, **kwargs):
     plugin = kwargs.pop("plugin", "@com_github_golang_protobuf//protoc-gen-go")
     reset_plugin_name = name + "_reset_plugin_"
-    go_exec_reset_target(
+    go_reset_target(
         name = reset_plugin_name,
         dep = plugin,
         visibility = ["//visibility:private"],

proto/def.bzl

@@ -33,7 +33,7 @@ load(
 )
 load(
     "//go/private/rules:transition.bzl",
-    "go_non_go_exec_reset_tool_transition",
+    "non_go_tool_transition",
 )
 load(
     "@rules_proto//proto:defs.bzl",
@@ -149,11 +149,11 @@ go_proto_library = rule(
     implementation = _go_proto_library_impl,
     attrs = {
         "proto": attr.label(
-            cfg = go_non_go_exec_reset_tool_transition,
+            cfg = non_go_tool_transition,
             providers = [ProtoInfo],
         ),
         "protos": attr.label_list(
-            cfg = go_non_go_exec_reset_tool_transition,
+            cfg = non_go_tool_transition,
             providers = [ProtoInfo],
             default = [],
         ),

tests/core/transition/hermeticity_test.go

@@ -111,6 +111,7 @@ func assertDependsCleanlyOnWithFlags(t *testing.T, targetA, targetB string, flag
 	out, err := bazel_testing.BazelOutput(append(
 		[]string{
 			"cquery",
+			"--transitions=full",
 			query,
 		},
 		flags...,