Review comments

go/private/context.bzl

@@ -392,7 +392,7 @@ def go_context(ctx, attr = None):
     coverdata = None
     nogo = None
     if hasattr(attr, "_go_context_data"):
-        go_context_data = _flatten(attr._go_context_data)
+        go_context_data = _flatten_possibly_transitioned_attr(attr._go_context_data)
         if CgoContextInfo in go_context_data:
             cgo_context_info = go_context_data[CgoContextInfo]
         go_config_info = go_context_data[GoConfigInfo]
@@ -406,7 +406,7 @@ def go_context(ctx, attr = None):
     if hasattr(attr, "_go_config"):
         go_config_info = attr._go_config[GoConfigInfo]
     if hasattr(attr, "_stdlib"):
-        stdlib = _flatten(attr._stdlib)[GoStdLib]
+        stdlib = _flatten_possibly_transitioned_attr(attr._stdlib)[GoStdLib]
 
     mode = get_mode(ctx, toolchain, cgo_context_info, go_config_info)
     tags = mode.tags
@@ -866,12 +866,14 @@ go_config = rule(
 def _expand_opts(go, attribute_name, opts):
     return [go._ctx.expand_make_variables(attribute_name, opt, {}) for opt in opts]
 
+_LIST_TYPE = type([])
+
 # Used to get attribute values which may have been transitioned.
 # Transitioned attributes end up as lists.
 # We never use split-transitions, so we always expect exactly one element in those lists.
 # But if the attribute wasn't transitioned, it won't be a list.
-def _flatten(maybe_list):
-    if type(maybe_list) == "list":
+def _flatten_possibly_transitioned_attr(maybe_list):
+    if type(maybe_list) == _LIST_TYPE:
         if len(maybe_list) == 1:
             return maybe_list[0]
         else:

go/private/rules/binary.bzl

@@ -34,7 +34,6 @@ load(
 load(
     "//go/private/rules:transition.bzl",
     "go_transition",
-    "non_go_transition",
 )
 load(
     "//go/private:mode.bzl",
@@ -189,7 +188,6 @@ _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,
@@ -200,7 +198,6 @@ _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
@@ -230,7 +227,6 @@ _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`
@@ -283,7 +279,6 @@ _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`.

go/private/rules/library.bzl

@@ -31,10 +31,6 @@ load(
     "GoLibrary",
     "INFERRED_PATH",
 )
-load(
-    "//go/private/rules:transition.bzl",
-    "non_go_transition",
-)
 
 def _go_library_impl(ctx):
     """Implements the go_library() rule."""
@@ -63,7 +59,6 @@ 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.
@@ -73,7 +68,6 @@ 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,
@@ -116,7 +110,6 @@ 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.
@@ -144,7 +137,6 @@ 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`.
@@ -176,9 +168,6 @@ 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 = [GO_TOOLCHAIN],
     doc = """This builds a Go library from a set of source files that are all part of

go/private/rules/test.bzl

@@ -44,7 +44,6 @@ load(
 load(
     "//go/private/rules:transition.bzl",
     "go_transition",
-    "non_go_transition",
 )
 load(
     "//go/private:mode.bzl",
@@ -196,7 +195,6 @@ _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
@@ -206,7 +204,6 @@ _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,
@@ -236,7 +233,6 @@ _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`
@@ -309,7 +305,6 @@ _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`.

go/private/rules/transition.bzl

@@ -347,48 +347,6 @@ 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))

go/private/rules/wrappers.bzl

@@ -33,6 +33,8 @@ load(
     "go_test",
 )
 
+_SELECT_TYPE = type(select({"//conditions:default": ""}))
+
 def _cgo(name, kwargs):
     if "objc" in kwargs:
         fail("//{}:{}: the objc attribute has been removed. .m sources may be included in srcs or may be extracted into a separated objc_library listed in cdeps.".format(native.package_name(), name))
@@ -48,7 +50,7 @@ def go_binary_macro(name, **kwargs):
 
     if kwargs.get("goos") != None or kwargs.get("goarch") != None:
         for key, value in kwargs.items():
-            if type(value) == "select":
+            if type(value) == _SELECT_TYPE:
                 # In the long term, we should replace goos/goarch with Bazel-native platform
                 # support, but while we have the mechanisms, we try to avoid people trying to use
                 # _both_ goos/goarch _and_ native platform support.

tests/core/go_binary/configurable_attribute_bad_test.go

@@ -32,8 +32,8 @@ go_binary(
     srcs = [
         "main.go",
     ],
-	goos = "darwin",
-	goarch = "amd64",
+    goos = "darwin",
+    goarch = "amd64",
     gotags = select({
         "@io_bazel_rules_go//go/platform:linux": ["penguins"],
         "//conditions:default": ["nopenguins"],