-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Description
Description of the problem / feature request:
If the input of a transition contains a build setting specified with config.string() that has allow_multiple set to True, and the setting is left at its default value, the transition can not considered valid without the setting also being one of the outputs.
As of #13971, the validation that is performed after a transition has been applied in StarlarkTransition::validate() validates both the inputs and the outputs of a transition. If the inputs include a setting specified with config.string() with allow_multiple = True, part of the validation will involve verifying that the values corresponding to this setting is a list. However, when setting the build_setting_default of the setting, it is only possible to specify a string. Therefore, if the setting retains its default value, the validation of the transition will fail when checking if it is a list if the transition implementation itself does not alter the value.
A workaround for this is to set the input setting as an output, checking if it is a string, and returning the value inside a list if it is. This workaround is undesirable since it would have to be done for every transition that uses the relevant setting as an input, and I imagine that adding this false variation might have a needless impact on performance as well.
Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.
foo.bzl
P = provider(fields = ["value"])
def _s_impl(ctx):
return [P(value = ctx.build_setting_value)]
def _t_impl(settings, attr):
if "foo" in settings["//:a"]: # //:a is only used to decide the output, so it shouldn't be an output itself
return {"//:b": ["bar"]}
else:
return {"//:b": ["baz"]}
def _r_impl(ctx):
print(ctx.attr.setting[0][P].value)
s = rule(
implementation = _s_impl,
build_setting = config.string(allow_multiple = True, flag = True),
)
t = transition(
implementation = _t_impl,
inputs = ["//:a"],
outputs = ["//:b"],
)
r = rule(
implementation = _r_impl,
attrs = {
"setting": attr.label(cfg = t),
"_allowlist_function_transition": attr.label(default = "@bazel_tools//tools/allowlists/function_transition_allowlist")
},
)BUILD
load(
"foo.bzl",
"s",
"r",
)
s(
name = "a",
build_setting_default = "", # Has to be a string, cannot be []
)
s(
name = "b",
build_setting_default = "", # Has to be a string, cannot be []
)
r(
name = "c",
setting = ":b"
)In this example, running bazel build :c causes Bazel to throw an error, but setting any value for //:a other than the default value, e.g. bazel build :c --//:a=something lets it run as intended.
What operating system are you running Bazel on?
Red Hat Enterprise Linux Server 7.9
What's the output of bazel info release?
release 5.0.0
If bazel info release returns "development version" or "(@Non-Git)", tell us how you built Bazel.
N/A
Have you found anything relevant by searching the web?
PR #13971 seems to be where the bug was introduced.
Issue #13817 describes a situation where users would like to be able to set an empty list as the default value of an option with allow_multiple set to True.