bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
22.99k stars 4.03k forks source link

Allow transition on incompatible and experimental native flags #20297

Open thesayyn opened 10 months ago

thesayyn commented 10 months ago

Description of the feature request:

Currently, --incompatible_* and --experimental_* flags are disallowed in transitions without any justification why. I am currently working on rules_proto's incompatible proto toolchain feature, therefore need to write some tests that needs to transition in order to test the logic.

Currently transitioning on these flags end up in an error, and the reason isn't clear.

Error in analysis_test_transition: Invalid transition output '//command_line_option:incompatible_enable_proto_toolchain_resolution'. Cannot transition on --experimental_* or --incompatible_* options

It was disabled with this commit; https://github.com/bazelbuild/bazel/commit/ff93b19e496513dd78be4b6bcee0f0105cc17d2b

Which category does this issue belong to?

No response

What underlying problem are you trying to solve with this feature?

Writing unit tests that test experimental and incompatible flags.

Which operating system are you running Bazel on?

No response

What is the output of bazel info release?

No response

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

Have you found anything relevant by searching the web?

No response

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

No response

fmeum commented 10 months ago

A more lightweight way to get you unstuck could be to add that particular flag to this allowlist: https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java;l=152?q=incompatible_enable_cc_toolchain_resolution&sq=&ss=bazel%2Fbazel

More generally, I definitely see the appeal in allowing all flags for analysis_test_transition to support Starlark unit tests. But I would personally be worried if rulesets could decide by themselves to flip experimental or incompatible flags in production.

Cc @comius

thesayyn commented 10 months ago

I worked around this by simply skipping targets based on flag presence. it still would be good to able to test both flag behaviors in tests without separate bazel invocations which complicates the CI setup. perhaps this forbidding behavior can be gated behind a flag so we can consciously allow it?

gregestren commented 9 months ago

I honestly don't remember the original rationale for banning those flags.

I'll review this with configurability.

Note we're talking about further reducing the set of transitionable flags. But we're thinking about stuff users should have no business transitioning on, like http://google3/third_party/bazel/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java;l=318;rcl=584873095.

We're also starting to explore the idea of "flag sets": named groups of flags. Sort of like --config but in BUILD files and attachable to rules. The idea would be to further standardize & curate the set of configurations that apply to a build, and reduce # of ad hoc configurations due to any combination of hundreds of flags changing.