--features=feature_a,feature_b is not the same as --features=feature_a --features_b

[limdor]

Description of the bug:

In Bazel, you have to types of command line parameters, some that you can pass comma separated values like --test_tag_filters and some that you can pass multiple values specifying the parameter multiple times like --features.

The problem we had here, was that we tried to pass --features like if would be a comma separated value. (i.e. --features=featurea_a,feature_b. Intuitively we would have expected both feature_a and feature_b to be enabled, but that was not the case. We assume that Bazel takes feature_a,feature_b as a single string, and because if a feature is not implemented by a toolchain is ignored, that is fine.

Until here it is unfortunate but I can understand the behavior. However, why would it make sense this behavior? The only time that makes sense is if it would be possible to define a feature with the name feature_a,feature_b. However, if you try to do that, you will get a Bazel error as follows:

Error in create_cc_toolchain_config_info: in FeatureInfo instantiated at /home/xb/.cache/bazel/_bazel_xb/d322d9e7202fdf5f81e4f8ec750a774d/external/bazel_tools/tools/cpp/cc_toolchain_config_lib.bzl:395:23: A feature's name must consist solely of lowercase ASCII letters, digits, '.', '_', '+', and '-', got 'standard_cpp_14,standard_cpp_17'

Now we could discuss if this is a bug or a feature request. The documentation is clear that it expect a string and not a comma separated list. However we just strive for good user experience.
What we would expect is that Bazel gives an error if someone is passing a feature name that contains a comma ,.

One last thing, it is not so strange that the user expects that comma separated is allowed, in the end it is called --features (plural) and not --feature (singular).

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

Provide to enable multiple features this way and use --subcommand=pretty_print to see if they are applied.

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

6.2.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

[oquenchil]

This is just how Bazel multi string options work. Nothing that can be done by the C++ rules to process features the way it's suggested here. You will have to pass the features flag more than once, each one with the value.

You may want to file a generic issue for this so that it's addressed for all multi string options in Bazel.

[limdor]

@oquenchil for me this was all the time a generic issue, I did not expect anything specific for C++. Can you just not reopen it and assign it to the corresponding team?
@sgowroji anything I could do next time to make clear that this was a generic issue?

[gregestren]

Didn't mean to ping pong the team assignment above (I didn't see it sent here before I sent it to team-C++).

This sounds C++-specific to me. --features solely exists for consumption by the C++ rules, which should define the semantics they need. Not all multi-valued flags are interpreted the same way.

See, for example https://bazel.build/reference/be/general#config_setting:

Exact semantics vary between flags. For example, --copt doesn't support multiple values in the same instance: --copt=a,b produces ["a,b"] while --copt=a --copt=b produces ["a", "b"] (so values = { "copt": "a,b" } matches the former but not the latter). But --ios_multi_cpus (for Apple rules) does: -ios_multi_cpus=a,b and ios_multi_cpus=a --ios_multi_cpus=b both produce ["a", "b"]. Check flag definitions and test your conditions carefully to verify exact expectations.

There should be enough flexibility in the flag definitions to tune them according to the needs of whoever consumes them.

Note, for example:

bazel/src/main/java/com/google/devtools/build/lib/rules/apple/AppleCommandLineOptions.java

Lines 248 to 251 in 0fa5e35

	@Option(
	name = "ios_multi_cpus",
	allowMultiple = true,
	converter = CommaSeparatedOptionListConverter.class,

vs.

bazel/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java

Lines 650 to 654 in 0fa5e35

	@Option(
	name = "features",
	allowMultiple = true,
	defaultValue = "null",
	documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,

If you really mean this about multi-value flags in general, we'd need some agreement on common syntax that all consumers agree with. If you think the above is reasonable and not likely to break existing differences like described above, we could assign this to team-Core, which owns generic options parsing.

[oquenchil]

I'm not in the C++ rules team anymore but since the current behavior is documented and changing it now would cause more disruption than the benefit of the fix I'd recommend that this bug be closed.