Skip to content

Commit

Permalink
Transition on edges not self
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
illicitonion committed May 23, 2022
1 parent cd744bd commit 56bbbb2
Show file tree
Hide file tree
Showing 7 changed files with 225 additions and 71 deletions.
3 changes: 3 additions & 0 deletions go/private/rules/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ bzl_library(
"//go/private:providers",
"//go/private:rpath",
"//go/private/rules:transition",
"@bazel_skylib//lib:dicts",
],
)

Expand Down Expand Up @@ -132,6 +133,7 @@ bzl_library(
"//go/private:providers",
"//go/private/rules:binary",
"//go/private/rules:transition",
"@bazel_skylib//lib:dicts",
"@bazel_skylib//lib:structs",
],
)
Expand All @@ -148,6 +150,7 @@ bzl_library(
"//go/private:mode",
"//go/private:platforms",
"//go/private:providers",
"//go/private/skylib/lib:dicts",
],
)

Expand Down
44 changes: 31 additions & 13 deletions go/private/rules/binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,11 @@ load(
)
load(
"//go/private/rules:transition.bzl",
"go_transition_rule",
"non_go_transition",
"ForwardingPastTransitionProvider",
"forward_through_transition_impl",
"go_transition",
"transition_attrs",
)
load(
"//go/private:mode.bzl",
Expand All @@ -44,6 +47,10 @@ load(
"//go/private:rpath.bzl",
"rpath",
)
load(
"@bazel_skylib//lib:dicts.bzl",
"dicts",
)

_EMPTY_DEPSET = depset([])

Expand Down Expand Up @@ -124,19 +131,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
Expand Down Expand Up @@ -165,11 +167,19 @@ 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_kwargs = {
go_binary_kwargs = {
"implementation": _go_binary_impl,
"attrs": {
"srcs": attr.label_list(
Expand Down Expand Up @@ -394,9 +404,17 @@ _go_binary_kwargs = {
""",
}

go_binary = rule(executable = True, **_go_binary_kwargs)
go_transition_binary = go_transition_rule(executable = True, **_go_binary_kwargs)
go_non_executable_transition_binary = go_transition_rule(executable = False, **_go_binary_kwargs)
go_binary = rule(executable = True, **go_binary_kwargs)
go_non_executable_transition_binary = rule(executable = False, **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]
Expand Down
52 changes: 42 additions & 10 deletions go/private/rules/test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,20 @@ load(
)
load(
"//go/private/rules:transition.bzl",
"go_transition_rule",
"non_go_transition",
"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",
Expand All @@ -54,6 +61,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.
Expand Down Expand Up @@ -168,13 +191,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],
),
Expand All @@ -187,7 +205,22 @@ def _go_test_impl(ctx):
testing.TestEnvironment(env),
]

_go_test_kwargs = {
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_kwargs = {
"implementation": _go_test_impl,
"attrs": {
"data": attr.label_list(
Expand Down Expand Up @@ -448,8 +481,7 @@ _go_test_kwargs = {
""",
}

go_test = rule(**_go_test_kwargs)
go_transition_test = go_transition_rule(**_go_test_kwargs)
go_test = 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.
Expand Down
Loading

0 comments on commit 56bbbb2

Please sign in to comment.