-
-
Notifications
You must be signed in to change notification settings - Fork 678
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use edge transitions not self transitions #3130
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -29,8 +29,11 @@ load( | |||||
) | ||||||
load( | ||||||
"//go/private/rules:transition.bzl", | ||||||
"go_transition_rule", | ||||||
"ForwardingPastTransitionProvider", | ||||||
"forward_through_transition_impl", | ||||||
"go_transition", | ||||||
"non_go_transition", | ||||||
"transition_attrs", | ||||||
) | ||||||
load( | ||||||
"//go/private:mode.bzl", | ||||||
|
@@ -44,6 +47,10 @@ load( | |||||
"//go/private:rpath.bzl", | ||||||
"rpath", | ||||||
) | ||||||
load( | ||||||
"@bazel_skylib//lib:dicts.bzl", | ||||||
"dicts", | ||||||
) | ||||||
|
||||||
_EMPTY_DEPSET = depset([]) | ||||||
|
||||||
|
@@ -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 | ||||||
|
@@ -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( | ||||||
|
@@ -394,9 +404,25 @@ _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) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
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), | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that this is only an internal attribute, we could help the IntelliJ plugin here by giving it a name recognized by its aspect, see fmeum/rules_meta@8d57be3. |
||||||
}), | ||||||
executable = True, | ||||||
) | ||||||
go_non_executable_transition_transition_binary = rule( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
implementation = forward_through_transition_impl, | ||||||
attrs = dicts.add(transition_attrs, { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
"is_windows": attr.bool(), | ||||||
"transition_dep": attr.label(cfg = go_transition), | ||||||
}), | ||||||
executable = False, | ||||||
) | ||||||
|
||||||
def _go_tool_binary_impl(ctx): | ||||||
sdk = ctx.attr.sdk[GoSDK] | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,13 +39,20 @@ load( | |
) | ||
load( | ||
"//go/private/rules:transition.bzl", | ||
"go_transition_rule", | ||
"ForwardingPastTransitionProvider", | ||
"forward_through_transition_impl", | ||
"go_transition", | ||
"non_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", | ||
|
@@ -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. | ||
|
||
|
@@ -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 = [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above, could stay as |
||
test_archive, | ||
DefaultInfo( | ||
files = depset([executable]), | ||
runfiles = runfiles, | ||
executable = executable, | ||
), | ||
OutputGroupInfo( | ||
compilation_outputs = [internal_archive.data.file], | ||
), | ||
|
@@ -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( | ||
|
@@ -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. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe leave this as
providers
as these providers are still forwarded only in addition to being advertised by the rule itself?