Trim transitioned Go settings on non-Go dependencies (#3108)

* Trim transitioned Go settings on non-Go dependencies

By resetting Go-specific settings changed by go_transition to their
previous values when crossing a non-deps dependency edge, e.g. srcs or
data, dependencies through those edges are not affected by the change in
Go settings. This has two advantages:

* Correctness: Previously, if a go_binary with linkmode = "c-archive"
  used another go_binary to generate srcs, that go_binary would also be
  built as a c-archive and thus fail to run during the build. This was
  observed in bazelbuild/bazel#14626.
* Performance: With the new Bazel flag
  --experimental_output_directory_naming_scheme=diff_against_baseline,
  transitions can return to the top-level configuration since the
  Starlark settings that were affected by a transition at some point
  during the build are no longer tracked explicitly in a setting, but
  computed as a configuration diff. Resetting the configuration for non-
  deps dependencies thus prevents redundant rebuilds of non-Go deps
  caused by changes in Go settings, achieving a version of
  "configuration trimming" for Go.

* Verify transition hermeticity for go_{binary,library,test}

The test verifies that Go settings such as gotags and linkmode do not
affect dependencies through attributes other than deps.

go/private/rules/BUILD.bazel

@@ -1,7 +1,21 @@
 load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
+load("@bazel_skylib//rules:common_settings.bzl", "string_setting")
+load(
+    ":transition.bzl",
+    "TRANSITIONED_GO_SETTING_KEYS",
+)
 
 exports_files(["library.bzl"])
 
+[
+    string_setting(
+        name = "original_" + setting.split(":")[1],
+        build_setting_default = "",
+        visibility = ["//visibility:private"],
+    )
+    for setting in TRANSITIONED_GO_SETTING_KEYS
+]
+
 filegroup(
     name = "all_rules",
     srcs = glob(["**/*.bzl"]),

go/private/rules/binary.bzl

@@ -30,6 +30,7 @@ load(
 load(
     "//go/private/rules:transition.bzl",
     "go_transition_rule",
+    "non_go_transition",
 )
 load(
     "//go/private:mode.bzl",
@@ -173,6 +174,7 @@ _go_binary_kwargs = {
     "attrs": {
         "srcs": attr.label_list(
             allow_files = go_exts + asm_exts + cgo_exts,
+            cfg = non_go_transition,
             doc = """The list of Go source files that are compiled to create the package.
             Only `.go` and `.s` files are permitted, unless the `cgo`
             attribute is set, in which case,
@@ -183,6 +185,7 @@ _go_binary_kwargs = {
         ),
         "data": attr.label_list(
             allow_files = True,
+            cfg = non_go_transition,
             doc = """List of files needed by this rule at run-time. This may include data files
             needed or other programs that may be executed. The [bazel] package may be
             used to locate run files; they may appear in different places depending on the
@@ -210,6 +213,7 @@ _go_binary_kwargs = {
         ),
         "embedsrcs": attr.label_list(
             allow_files = True,
+            cfg = non_go_transition,
             doc = """The list of files that may be embedded into the compiled package using
             `//go:embed` directives. All files must be in the same logical directory
             or a subdirectory as source files. All source files containing `//go:embed`
@@ -262,6 +266,7 @@ _go_binary_kwargs = {
             """,
         ),
         "cdeps": attr.label_list(
+            cfg = non_go_transition,
             doc = """The list of other libraries that the c code depends on.
             This can be anything that would be allowed in [cc_library deps]
             Only valid if `cgo` = `True`.
@@ -371,6 +376,9 @@ _go_binary_kwargs = {
             """,
         ),
         "_go_context_data": attr.label(default = "//:go_context_data"),
+        "_allowlist_function_transition": attr.label(
+            default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
+        ),
     },
     "toolchains": ["@io_bazel_rules_go//go:toolchain"],
     "doc": """This builds an executable from a set of source files,

go/private/rules/library.bzl

@@ -27,6 +27,10 @@ load(
     "GoLibrary",
     "INFERRED_PATH",
 )
+load(
+    "//go/private/rules:transition.bzl",
+    "non_go_transition",
+)
 
 def _go_library_impl(ctx):
     """Implements the go_library() rule."""
@@ -55,6 +59,7 @@ go_library = rule(
     attrs = {
         "data": attr.label_list(
             allow_files = True,
+            cfg = non_go_transition,
             doc = """
             List of files needed by this rule at run-time.
             This may include data files needed or other programs that may be executed.
@@ -64,6 +69,7 @@ go_library = rule(
         ),
         "srcs": attr.label_list(
             allow_files = go_exts + asm_exts + cgo_exts,
+            cfg = non_go_transition,
             doc = """
             The list of Go source files that are compiled to create the package.
             Only `.go` and `.s` files are permitted, unless the `cgo` attribute is set,
@@ -106,6 +112,7 @@ go_library = rule(
         ),
         "embedsrcs": attr.label_list(
             allow_files = True,
+            cfg = non_go_transition,
             doc = """
             The list of files that may be embedded into the compiled package using `//go:embed`
             directives. All files must be in the same logical directory or a subdirectory as source files.
@@ -133,6 +140,7 @@ go_library = rule(
             """,
         ),
         "cdeps": attr.label_list(
+            cfg = non_go_transition,
             doc = """
             List of other libraries that the c code depends on.
             This can be anything that would be allowed in [cc_library deps] Only valid if `cgo = True`.
@@ -164,6 +172,9 @@ go_library = rule(
             """,
         ),
         "_go_context_data": attr.label(default = "//:go_context_data"),
+        "_allowlist_function_transition": attr.label(
+            default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
+        ),
     },
     toolchains = ["@io_bazel_rules_go//go:toolchain"],
     doc = """This builds a Go library from a set of source files that are all part of

go/private/rules/test.bzl

@@ -40,6 +40,7 @@ load(
 load(
     "//go/private/rules:transition.bzl",
     "go_transition_rule",
+    "non_go_transition",
 )
 load(
     "//go/private:mode.bzl",
@@ -191,6 +192,7 @@ _go_test_kwargs = {
     "attrs": {
         "data": attr.label_list(
             allow_files = True,
+            cfg = non_go_transition,
             doc = """List of files needed by this rule at run-time. This may include data files
             needed or other programs that may be executed. The [bazel] package may be
             used to locate run files; they may appear in different places depending on the
@@ -200,6 +202,7 @@ _go_test_kwargs = {
         ),
         "srcs": attr.label_list(
             allow_files = go_exts + asm_exts + cgo_exts,
+            cfg = non_go_transition,
             doc = """The list of Go source files that are compiled to create the package.
             Only `.go` and `.s` files are permitted, unless the `cgo`
             attribute is set, in which case,
@@ -227,6 +230,7 @@ _go_test_kwargs = {
         ),
         "embedsrcs": attr.label_list(
             allow_files = True,
+            cfg = non_go_transition,
             doc = """The list of files that may be embedded into the compiled package using
             `//go:embed` directives. All files must be in the same logical directory
             or a subdirectory as source files. All source files containing `//go:embed`
@@ -299,6 +303,7 @@ _go_test_kwargs = {
             """,
         ),
         "cdeps": attr.label_list(
+            cfg = non_go_transition,
             doc = """The list of other libraries that the c code depends on.
             This can be anything that would be allowed in [cc_library deps]
             Only valid if `cgo` = `True`.
@@ -404,6 +409,9 @@ _go_test_kwargs = {
             default = "//go/tools/builders:lcov_merger",
             cfg = "target",
         ),
+        "_allowlist_function_transition": attr.label(
+            default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
+        ),
     },
     "executable": True,
     "test": True,

go/private/rules/transition.bzl

@@ -54,6 +54,32 @@ def filter_transition_label(label):
     else:
         return str(Label(label))
 
+# A list of rules_go settings that are possibly set by go_transition.
+# Keep their package name in sync with the implementation of
+# _original_setting_key.
+TRANSITIONED_GO_SETTING_KEYS = [
+    filter_transition_label(key)
+    for key in [
+        "//go/config:static",
+        "//go/config:msan",
+        "//go/config:race",
+        "//go/config:pure",
+        "//go/config:linkmode",
+        "//go/config:tags",
+    ]
+]
+
+def _original_setting_key(key):
+    if not "//go/config:" in key:
+        fail("_original_setting_key currently assumes that all Go settings live under //go/config, got: " + key)
+    name = key.split(":")[1]
+    return filter_transition_label("//go/private/rules:original_" + name)
+
+_SETTING_KEY_TO_ORIGINAL_SETTING_KEY = {
+    setting: _original_setting_key(setting)
+    for setting in TRANSITIONED_GO_SETTING_KEYS
+}
+
 def go_transition_wrapper(kind, transition_kind, name, **kwargs):
     """Wrapper for rules that may use transitions.
 
@@ -111,11 +137,15 @@ def go_transition_rule(**kwargs):
     return rule(**kwargs)
 
 def _go_transition_impl(settings, attr):
+    # NOTE: Keep the list of rules_go settings set by this transition in sync
+    # with POTENTIALLY_TRANSITIONED_SETTINGS.
+    #
     # NOTE(bazelbuild/bazel#11409): Calling fail here for invalid combinations
     # of flags reports an error but does not stop the build.
     # In any case, get_mode should mainly be responsible for reporting
     # invalid modes, since it also takes --features flags into account.
 
+    original_settings = settings
     settings = dict(settings)
 
     _set_ternary(settings, attr, "static")
@@ -173,6 +203,25 @@ def _go_transition_impl(settings, attr):
         linkmode_label = filter_transition_label("@io_bazel_rules_go//go/config:linkmode")
         settings[linkmode_label] = linkmode
 
+    for key, original_key in _SETTING_KEY_TO_ORIGINAL_SETTING_KEY.items():
+        old_value = original_settings[key]
+        value = settings[key]
+
+        # If the outgoing configuration would differ from the incoming one in a
+        # value, record the old value in the special original_* key so that the
+        # real setting can be reset to this value before the new configuration
+        # would cross a non-deps dependency edge.
+        if value != old_value:
+            # Encoding as JSON makes it possible to embed settings of arbitrary
+            # types (currently bool, string and string_list) into a single type
+            # of setting (string) with the information preserved whether the
+            # original setting wasn't set explicitly (empty string) or was set
+            # explicitly to its default  (always a non-empty string with JSON
+            # encoding, e.g. "\"\"" or "[]").
+            settings[original_key] = json.encode(old_value)
+        else:
+            settings[original_key] = ""
+
     return settings
 
 def _request_nogo_transition(settings, attr):
@@ -203,25 +252,13 @@ go_transition = transition(
         "//command_line_option:cpu",
         "//command_line_option:crosstool_top",
         "//command_line_option:platforms",
-        "@io_bazel_rules_go//go/config:static",
-        "@io_bazel_rules_go//go/config:msan",
-        "@io_bazel_rules_go//go/config:race",
-        "@io_bazel_rules_go//go/config:pure",
-        "@io_bazel_rules_go//go/config:tags",
-        "@io_bazel_rules_go//go/config:linkmode",
-    ]],
+    ] + TRANSITIONED_GO_SETTING_KEYS],
     outputs = [filter_transition_label(label) for label in [
         "//command_line_option:platforms",
-        "@io_bazel_rules_go//go/config:static",
-        "@io_bazel_rules_go//go/config:msan",
-        "@io_bazel_rules_go//go/config:race",
-        "@io_bazel_rules_go//go/config:pure",
-        "@io_bazel_rules_go//go/config:tags",
-        "@io_bazel_rules_go//go/config:linkmode",
-    ]],
+    ] + TRANSITIONED_GO_SETTING_KEYS + _SETTING_KEY_TO_ORIGINAL_SETTING_KEY.values()],
 )
 
-_common_reset_transition_dict = {
+_common_reset_transition_dict = dict({
     "@io_bazel_rules_go//go/config:static": False,
     "@io_bazel_rules_go//go/config:msan": False,
     "@io_bazel_rules_go//go/config:race": False,
@@ -230,7 +267,7 @@ _common_reset_transition_dict = {
     "@io_bazel_rules_go//go/config:debug": False,
     "@io_bazel_rules_go//go/config:linkmode": LINKMODE_NORMAL,
     "@io_bazel_rules_go//go/config:tags": [],
-}
+}, **{setting: "" for setting in _SETTING_KEY_TO_ORIGINAL_SETTING_KEY.values()})
 
 _reset_transition_dict = dict(_common_reset_transition_dict, **{
     "@io_bazel_rules_go//go/private:bootstrap_nogo": True,
@@ -366,6 +403,48 @@ go_transition.
 """,
 )
 
+def _non_go_transition_impl(settings, attr):
+    """Sets all Go settings to the values they had before the last go_transition.
+
+    non_go_transition sets all of the //go/config settings to the value they had
+    before the last go_transition. This should be used on all attributes of
+    go_library/go_binary/go_test that are built in the target configuration and
+    do not constitute advertise any Go providers.
+
+    Examples: This transition is applied to the 'data' attribute of go_binary so
+    that other Go binaries used at runtime aren't affected by a non-standard
+    link mode set on the go_binary target, but still use the same top-level
+    settings such as e.g. race instrumentation.
+    """
+    new_settings = {}
+    for key, original_key in _SETTING_KEY_TO_ORIGINAL_SETTING_KEY.items():
+        original_value = settings[original_key]
+        if original_value:
+            # Reset to the original value of the setting before go_transition.
+            new_settings[key] = json.decode(original_value)
+        else:
+            new_settings[key] = settings[key]
+
+        # Reset the value of the helper setting to its default for two reasons:
+        # 1. Performance: This ensures that the Go settings of non-Go
+        #    dependencies have the same values as before the go_transition,
+        #    which can prevent unnecessary rebuilds caused by configuration
+        #    changes.
+        # 2. Correctness in edge cases: If there is a path in the build graph
+        #    from a go_binary's non-Go dependency to a go_library that does not
+        #    pass through another go_binary (e.g., through a custom rule
+        #    replacement for go_binary), this transition could be applied again
+        #    and cause incorrect Go setting values.
+        new_settings[original_key] = ""
+
+    return new_settings
+
+non_go_transition = transition(
+    implementation = _non_go_transition_impl,
+    inputs = TRANSITIONED_GO_SETTING_KEYS + _SETTING_KEY_TO_ORIGINAL_SETTING_KEY.values(),
+    outputs = TRANSITIONED_GO_SETTING_KEYS + _SETTING_KEY_TO_ORIGINAL_SETTING_KEY.values(),
+)
+
 def _check_ternary(name, value):
     if value not in ("on", "off", "auto"):
         fail('{}: must be "on", "off", or "auto"'.format(name))