Build settings defined with allow_multiple = True cannot pass their default values to transition inputs without also being outputs

[crydell-ericsson]

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.

[fmeum]

@Wyverald This is a regression from 4 to 5.0.0 that I caused with my fix for #12888. I will see whether I can fix it quickly, perhaps it can be included into 5.1.0.

[fmeum]

@gregestren The underlying issue seems to be #13817 and I don't want to make it harder to properly fix that issue by layering another hack on top. I see the following options:

1. In the validation logic, special case allow_multiple build settings that only appear as inputs by cleaning them from the options without the conversion error.
2. When filling the defaults for the input build settings, do the conversion from string to singleton list.

To be honest, I am not particularly happy with either of them. What do you think would be the most maintainable fix?

[fmeum]

I created #14908 with the option I found the least intrusive. Let me know if you prefer a different one.

[anthpi]

Thank you for the prompt fix. I have verified it and it works according to out expectations and solves the problem.

[Wyverald]

@bazel-io fork 5.1