Transition on edges not self

As per bazelbuild/bazel#15157 currently we
can't configure any transition-relevant attributes using select. This is
because we use self-transitions on targets, and when this happens,
configurable attributes aren't passed to the transition, so we make
incorrect transition decisions.

Instead, insert a dummy rule which exists only to transition its
dependencies using an edge transition, which means the appropriate
attributes are resolved before the transition is called.

To achieve this, we symlink the actual built binaries into our
transitioning wrapper rules, and forward any providers needed.

Fixes bazelbuild#3103.

go/private/rules/BUILD.bazel

@@ -28,6 +28,7 @@ bzl_library(
         "//go/private:mode",
         "//go/private:providers",
         "//go/private/rules:transition",
+        "@bazel_skylib//lib:dicts",
     ],
 )
 
@@ -117,6 +118,7 @@ bzl_library(
         "//go/private:providers",
         "//go/private/rules:binary",
         "//go/private/rules:transition",
+        "@bazel_skylib//lib:dicts",
         "@bazel_skylib//lib:structs",
     ],
 )
@@ -133,6 +135,7 @@ bzl_library(
         "//go/private:mode",
         "//go/private:platforms",
         "//go/private:providers",
+        "//go/private/skylib/lib:dicts",
     ],
 )
 

go/private/rules/binary.bzl

@@ -29,7 +29,10 @@ load(
 )
 load(
     "//go/private/rules:transition.bzl",
-    "go_transition_rule",
+    "ForwardingPastTransitionProvider",
+    "forward_through_transition_impl",
+    "go_transition",
+    "transition_attrs",
 )
 load(
     "//go/private:mode.bzl",
@@ -43,6 +46,10 @@ load(
     "//go/private:rpath.bzl",
     "rpath",
 )
+load(
+    "@bazel_skylib//lib:dicts.bzl",
+    "dicts",
+)
 
 _EMPTY_DEPSET = depset([])
 
@@ -123,19 +130,14 @@ def _go_binary_impl(ctx):
         executable = executable,
     )
 
-    providers = [
+    providers_to_forward = [
         library,
         source,
         archive,
         OutputGroupInfo(
             cgo_exports = archive.cgo_exports,
             compilation_outputs = [archive.data.file],
         ),
-        DefaultInfo(
-            files = depset([executable]),
-            runfiles = runfiles,
-            executable = executable,
-        ),
     ]
 
     # If the binary's linkmode is c-archive or c-shared, expose CcInfo
@@ -164,9 +166,17 @@ def _go_binary_impl(ctx):
         ccinfo = cc_common.merge_cc_infos(
             cc_infos = [ccinfo] + [d[CcInfo] for d in source.cdeps],
         )
-        providers.append(ccinfo)
+        providers_to_forward.append(ccinfo)
 
-    return providers
+    forwarding_provider = ForwardingPastTransitionProvider(executable = executable, runfiles = runfiles, providers_to_forward = providers_to_forward)
+    return providers_to_forward + [
+        forwarding_provider,
+        DefaultInfo(
+            files = depset([executable]),
+            runfiles = runfiles,
+            executable = executable,
+        ),
+    ]
 
 go_binary_attrs = {
     "srcs": attr.label_list(
@@ -390,7 +400,15 @@ _go_binary_kwargs = {
 }
 
 go_binary = rule(**_go_binary_kwargs)
-go_transition_binary = go_transition_rule(**_go_binary_kwargs)
+
+go_transition_binary = rule(
+    implementation = forward_through_transition_impl,
+    attrs = dicts.add(transition_attrs, {
+        "is_windows": attr.bool(),
+        "transition_dep": attr.label(cfg = go_transition),
+    }),
+    executable = True,
+)
 
 def _go_tool_binary_impl(ctx):
     sdk = ctx.attr.sdk[GoSDK]

go/private/rules/test.bzl

@@ -39,12 +39,19 @@ load(
 )
 load(
     "//go/private/rules:transition.bzl",
-    "go_transition_rule",
+    "ForwardingPastTransitionProvider",
+    "forward_through_transition_impl",
+    "go_transition",
+    "transition_attrs",
 )
 load(
     "//go/private:mode.bzl",
     "LINKMODE_NORMAL",
 )
+load(
+    "@bazel_skylib//lib:dicts.bzl",
+    "dicts",
+)
 load(
     "@bazel_skylib//lib:structs.bzl",
     "structs",
@@ -53,6 +60,22 @@ load(
 def _testmain_library_to_source(go, attr, source, merge):
     source["deps"] = source["deps"] + [attr.library]
 
+go_transition_test = rule(
+    implementation = forward_through_transition_impl,
+    attrs = dicts.add(transition_attrs, {
+        "transition_dep": attr.label(cfg = go_transition),
+        "is_windows": attr.bool(),
+        # Workaround for bazelbuild/bazel#6293. See comment in lcov_merger.sh.
+        "_lcov_merger": attr.label(
+            executable = True,
+            default = "//go/tools/builders:lcov_merger",
+            cfg = "target",
+        ),
+    }),
+    executable = True,
+    test = True,
+)
+
 def _go_test_impl(ctx):
     """go_test_impl implements go testing.
 
@@ -167,13 +190,8 @@ def _go_test_impl(ctx):
     # source file is present, Bazel will set the COVERAGE_OUTPUT_FILE
     # environment variable during tests and will save that file to the build
     # events + test outputs.
-    return [
+    providers_to_forward = [
         test_archive,
-        DefaultInfo(
-            files = depset([executable]),
-            runfiles = runfiles,
-            executable = executable,
-        ),
         OutputGroupInfo(
             compilation_outputs = [internal_archive.data.file],
         ),
@@ -186,6 +204,21 @@ def _go_test_impl(ctx):
         testing.TestEnvironment(env),
     ]
 
+    forwarding_provider = ForwardingPastTransitionProvider(
+        executable = executable,
+        runfiles = runfiles,
+        providers_to_forward = providers_to_forward,
+    )
+
+    return providers_to_forward + [
+        forwarding_provider,
+        DefaultInfo(
+            files = depset([executable]),
+            runfiles = runfiles,
+            executable = executable,
+        ),
+    ]
+
 go_test_attrs = {
     "data": attr.label_list(
         allow_files = True,
@@ -443,7 +476,6 @@ _go_test_kwargs = {
 }
 
 go_test = rule(**_go_test_kwargs)
-go_transition_test = go_transition_rule(**_go_test_kwargs)
 
 def _recompile_external_deps(go, external_source, internal_archive, library_labels):
     """Recompiles some archives in order to split internal and external tests.

go/private/rules/transition.bzl

@@ -36,6 +36,10 @@ load(
     "//go/platform:crosstool.bzl",
     "platform_from_crosstool",
 )
+load(
+    "//go/private/skylib/lib:dicts.bzl",
+    "dicts",
+)
 
 def filter_transition_label(label):
     """Transforms transition labels for the current workspace.
@@ -54,7 +58,24 @@ def filter_transition_label(label):
     else:
         return str(Label(label))
 
-def go_transition_wrapper(kind, transition_kind, name, **kwargs):
+ForwardingPastTransitionProvider = provider(
+    """Provider for a go_transition_wrapper to forward information from its dep.
+
+    go_transition_wrapper symlinks outputs from some rule, after performing a
+    transition on the rule it depends on. It also forwards all of the providers
+    from that rule.
+
+    This provider allows structured passing of the required data in order to
+    perform that forwarding.
+    """,
+    fields = {
+        "executable": "File: The executable of the dep, which should be symlinked to.",
+        "runfiles": "Runfiles: The runfiles of the dep.",
+        "providers_to_forward": "[Provider]: The providers the transition-wrapping rule should expose.",
+    },
+)
+
+def go_transition_wrapper(kind, transition_kind, name, use_basename, keys_to_strip, **kwargs):
     """Wrapper for rules that may use transitions.
 
     This is used in place of instantiating go_binary or go_transition_binary
@@ -64,9 +85,34 @@ def go_transition_wrapper(kind, transition_kind, name, **kwargs):
     configuration identical to the default configuration.
     """
     transition_keys = ("goos", "goarch", "pure", "static", "msan", "race", "gotags", "linkmode")
+    keys_to_strip = [k for k in keys_to_strip if k not in transition_keys]
     need_transition = any([key in kwargs for key in transition_keys])
     if need_transition:
-        transition_kind(name = name, **kwargs)
+        transitioned_name = name + ".transitioned"
+
+        # Strip out attrs that are expected by the underlying rule.
+        # This means that we strip out things like srcs, because we don't need them,
+        # but preserve things like tags and exec_compatible_with,
+        # which are general to all tests rather than specific to go_test specifically,
+        # and should apply to generated tests and binaries too.
+        transition_kwargs = dicts.omit(kwargs, keys_to_strip)
+        transition_kwargs["name"] = name
+        transition_kwargs["transition_dep"] = transitioned_name
+        transition_kwargs["is_windows"] = select({
+            "@bazel_tools//src/conditions:windows": True,
+            "//conditions:default": False,
+        })
+        transition_kind(**transition_kwargs)
+
+        tags = kwargs.pop("tags", [])
+        if "manual" not in tags:
+            tags += ["manual"]
+        kwargs["tags"] = tags
+
+        if use_basename and "basename" not in kwargs:
+            kwargs["basename"] = name
+
+        kind(name = transitioned_name, **kwargs)
     else:
         kind(name = name, **kwargs)
 
@@ -105,13 +151,6 @@ transition_attrs = {
     ),
 }
 
-def go_transition_rule(**kwargs):
-    """Like "rule", but adds a transition and mode attributes."""
-    kwargs = dict(kwargs)
-    kwargs["attrs"].update(transition_attrs)
-    kwargs["cfg"] = go_transition
-    return rule(**kwargs)
-
 def _go_transition_impl(settings, attr):
     # NOTE(bazelbuild/bazel#11409): Calling fail here for invalid combinations
     # of flags reports an error but does not stop the build.
@@ -269,7 +308,7 @@ def _go_reset_target_impl(ctx):
 
     new_executable = None
     original_executable = default_info.files_to_run.executable
-    default_runfiles = default_info.default_runfiles
+    runfiles_wrapper = default_info
     if original_executable:
         # In order for the symlink to have the same basename as the original
         # executable (important in the case of proto plugins), put it in a
@@ -280,13 +319,13 @@ def _go_reset_target_impl(ctx):
             target_file = original_executable,
             is_executable = True,
         )
-        default_runfiles = default_runfiles.merge(ctx.runfiles([new_executable]))
+        runfiles_wrapper = _add_to_runfiles(ctx, runfiles_wrapper, new_executable)
 
     providers.append(
         DefaultInfo(
             files = default_info.files,
-            data_runfiles = default_info.data_runfiles,
-            default_runfiles = default_runfiles,
+            data_runfiles = runfiles_wrapper.data_runfiles,
+            default_runfiles = runfiles_wrapper.default_runfiles,
             executable = new_executable,
         ),
     )
@@ -327,3 +366,38 @@ def _set_ternary(settings, attr, name):
         label = filter_transition_label("@io_bazel_rules_go//go/config:{}".format(name))
         settings[label] = value == "on"
     return value
+
+def _symlink_file_to_rule_name(ctx, src):
+    output_file_name = ctx.label.name + (".exe" if ctx.attr.is_windows else "")
+    dst = ctx.actions.declare_file(paths.join(ctx.label.name, output_file_name))
+    ctx.actions.symlink(
+        output = dst,
+        target_file = src,
+        is_executable = True,
+    )
+    return dst
+
+def _add_to_runfiles(ctx, default_info, file):
+    if file == None:
+        return default_info
+
+    data_runfiles = default_info.data_runfiles.merge(ctx.runfiles([file]))
+    default_runfiles = default_info.data_runfiles.merge(ctx.runfiles([file]))
+    return struct(
+        data_runfiles = data_runfiles,
+        default_runfiles = default_runfiles,
+    )
+
+def forward_through_transition_impl(ctx):
+    forwarding_provider = ctx.attr.transition_dep[0][ForwardingPastTransitionProvider]
+    default_info = ctx.attr.transition_dep[0][DefaultInfo]
+
+    copied_executable = _symlink_file_to_rule_name(ctx, forwarding_provider.executable)
+    runfiles = _add_to_runfiles(ctx, default_info, copied_executable)
+
+    return [DefaultInfo(
+        executable = copied_executable,
+        files = default_info.files,
+        data_runfiles = runfiles.data_runfiles,
+        default_runfiles = runfiles.default_runfiles,
+    )] + forwarding_provider.providers_to_forward

go/private/rules/wrappers.bzl

@@ -24,11 +24,13 @@ load(
 load(
     "//go/private/rules:binary.bzl",
     "go_binary",
+    "go_binary_attrs",
     "go_transition_binary",
 )
 load(
     "//go/private/rules:test.bzl",
     "go_test",
+    "go_test_attrs",
     "go_transition_test",
 )
 load(
@@ -48,7 +50,7 @@ def go_library_macro(name, **kwargs):
 def go_binary_macro(name, **kwargs):
     """See docs/go/core/rules.md#go_binary for full documentation."""
     _cgo(name, kwargs)
-    go_transition_wrapper(go_binary, go_transition_binary, name = name, **kwargs)
+    go_transition_wrapper(go_binary, go_transition_binary, name = name, use_basename = True, keys_to_strip = go_binary_attrs.keys(), **kwargs)
     if kwargs.get("linkmode") in (LINKMODE_C_ARCHIVE, LINKMODE_C_SHARED):
         # Create an alias to tell users of the `.cc` rule that it is deprecated.
         native.alias(
@@ -62,4 +64,4 @@ def go_binary_macro(name, **kwargs):
 def go_test_macro(name, **kwargs):
     """See docs/go/core/rules.md#go_test for full documentation."""
     _cgo(name, kwargs)
-    go_transition_wrapper(go_test, go_transition_test, name = name, **kwargs)
+    go_transition_wrapper(go_test, go_transition_test, name = name, use_basename = False, keys_to_strip = go_test_attrs.keys(), **kwargs)

go/private/skylib/lib/BUILD.bazel

@@ -18,3 +18,9 @@ bzl_library(
     srcs = ["versions.bzl"],
     visibility = ["//go:__subpackages__"],
 )
+
+bzl_library(
+    name = "dicts",
+    srcs = ["dicts.bzl"],
+    visibility = ["//go:__subpackages__"],
+)