string flag with allow_multiple=True represented as string or list depending on the number of times it was used

[konste]

Description of the bug:

multistring_flag = rule(
    implementation = _multistring_impl,
    build_setting = config.string(flag = True, allow_multiple = True),
)

When such flag is used in transition corresponding setting is string if the flag was used once or it is list if the flag was used more than once.

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

I provided compact repro here: https://github.com/Bazel-snippets/string_flag
Command to run: bazel build test

The repro defines two very similar flags:

string_list_flag = rule(
    build_setting = config.string_list(flag = True),
)

and

multistring_flag = rule(
    build_setting = config.string(flag = True, allow_multiple = True),
)

Rule test prints the value of both flags and in both cases it is a list.

Rule test also uses those flags in a transition like this:

def _test_transition_impl(settings, attr):
    string_list_flag = settings["//:string_list_flag"]
    multistring_flag = settings["//:multistring_flag"]

In this case string_list_flag is again a list as it should be, but multistring_flag is a string, which is a bug.
Interestingly if multistring_flag is used more than once on the command line then it is a list too.
Expected behavior: it should be a list always.

Which operating system are you running Bazel on?

Mac, Linux, Windows

What is the output of bazel info release?

5.1.1

[brentleyjones]

Is #13817 related?

[konste]

@brentleyjones thank you for connecting this issue with #13817
It is definitely in the same area, but the bugs are actually different.
#13817 is about the limitation that flags with allow_multiple=True cannot be defaulted to empty list.
This bug is different - it is specific to transitions and it shows that the value of the flag as seen by transition implementation function changes its type between string and list depending on how many times the flag is used on the command line.

[fmeum]

@konste Is it possible that this only happens when the build setting is not set to a non-default value on the command-line? I prepared #15655, could you give it a try? With your reproducer, I now always get a list in the transition.

[konste]

@fmeum I did some testing and found that the issue reproduces for multistring_flag when

1. flag definition has a single string default value and not set on the command line or
2. flag definition has a single string default value and it is set from the command line to exactly the same single value

In other cases I also get the list.

[fmeum]

That matches what I fixed, thanks for confirming.

[konste]

A couple more observations:

1. When multistring_flag is set on the command line to nothing - I get empty list, Good!
2. When multistring_flag has the default set to empty string - I get empty string in transition. Bad.

[konste]

Another something I don't understand at all:

1. Run the repro bazel build test to get Bazel server running
2. Edit the code to change the default values for both flags
3. Run the repro again.

Observation - output from the rule implementation function shows new default values, BUT the output from transition implementation function shows the old values! And those values never change until I shutdown Bazel server. On the next run the updated values are picked up. HUH?

[fmeum]

Another something I don't understand at all:

1. Run the repro bazel build test to get Bazel server running
2. Edit the code to change the default values for both flags
3. Run the repro again.

Observation - output from the rule implementation function shows new default values, BUT the output from transition implementation function shows the old values! And those values never change until I shutdown Bazel server. On the next run the updated values are picked up. HUH?

That sounds pretty bad and wouldn't be covered by my fix.

@gregestren Could there be some issue with transition caching related to the default value being encoded as null and not its actual value?

[gregestren]

Will have to check, thanks for the correctness observation...

[gregestren]

I shrunk the example a bit to reproduce the incrementality bug: https://gist.github.com/gregestren/f9e83396f3543acf3ad1e6a6e156d8f8

Diagnosing the root cause next. Definitely worth prioritizing. I'll probably fork to a new issue on diagnosis.