Bazel should warn if an unknown feature is given

[fzakaria]

Description of the bug:

You can provide Bazel with features with typos or completely non-existent and you wouldn't know about it.

Which category does this issue belong to?

CLI

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

Here you can see I am providing the feature blah and it doesn't do anything. This is also problematic if you give a comma separated features value rather than specifying it on the command line multiple times. --features foo --features bar vs --features foo,bar

❯ bazel build --features=blah //examples/c++:example_add
WARNING: Build option --features has changed, discarding analysis cache (this can be expensive, see https://bazel.build/advanced/performance/iteration-speed).
INFO: Analyzed target //examples/c++:example_add (0 packages loaded, 2956 targets configured).
INFO: Found 1 target...
Target //examples/c++:example_add up-to-date:
  bazel-bin/examples/c++/example_add
INFO: Elapsed time: 0.743s, Critical Path: 0.01s

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

release 7.0.2

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

N/A

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

N/A

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

No

Have you found anything relevant by searching the web?

Seems like this is the default behavior.

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

No response

[fzakaria]

If this is an worthwhile bug -- I'd like to contribute to the repository. If someone gave me some pointers, I can try to give it a go

[fmeum]

I think that there are valid use cases for having features mentioned in BUILD files and command lines that aren't available on the toolchain (say layering_check on Windows).

That said, the --features=foo,bla footgun is, in my opinion, definitely bad enough that it warrants some improvement. I have personally lost a few hours to it. Maybe showing the warning only when a feature isn't available and contains a , is a good start?

[fzakaria]

I think that there are valid use cases for having features mentioned in BUILD files and command lines that aren't available on the toolchain (say layering_check on Windows).

Even in this case and the comma separated, the fix of just emitting which features were not enabled seems sufficient. in the case of windows, the "correct" answer would be to selectively pick the feature depending on the os etc.. ?

Ideally, you want to know you have no warnings emitted I would imagine.

[martis42]

Spelling errors in features cost me some time already. I would welcome a possibility to find such problems efficiently. However, I agree that there are use cases for features actually not being offered by the current toolchain.

Could we have a flag which lists all used features which are not implemented by the toolchain? Ideally with an opt-in behavior to cause a build failure on not implemented features?

Yes this is yet another feature. But imho we need some way to debug feature resolution.

[fzakaria]

I am trying to work on a PR -- the command line parsing between client and server is intricate -- any pointers yo have would be great.

I have a small check in FeatureSet to disallow comma but it's a bit too low in the call stack.

[jungleraptor]

I'm kind of split on this

Is the idea that at the beginning of the build we list the features that weren't enabled? How would it work for features that are enabled on targets or in BUILD files?

I ask because we have a lot of custom toolchains with a lot of custom features that are specific to those toolchains, and in those cases, we probably don't want to get the output spammed with warnings about each of those instances (for example we implemented c++ standard versions as toolchain features, and that's applied to most of the targets in our graph).

Another example is if we're compiling with some gcc + stdlibc++ toolchain, but some target has libcpp_cxx20_enable_remove_features in that case it's nice that it's just silently ignored

But on the other hand - we implemented relwdbg as a toolchain feature, and in that situation we definitely would want a warning if the toolchain in use hasn't implemented it (similar to OPs issue).

I think maybe the bigger issue here is that the features a toolchain implements isn't some contract or API so it's hard to say when it's appropriate to warn. If there was a canonical interface it would be easier... Like if your toolchain doesn't implement opt - that should probably be an error.

[fzakaria]

I'm new to contributing -- but parsing the build graph -- what does it matter if they are on targets or BUILD files? The features available are defined by the toolchain -- seems problematic to then ask for features that don't exist. If they are in the BUILD shouldn't they also be conditionally selected so they are available for the correct platform?

Finally, there is probably a "strict" setting here that can be the easy compromise.

[comius]

It's intentional that unknown features don't cause an error. And it's intentional that Bazel doesn't emit any warnings. (Ok, some legacy warnings).

[fmeum]

@comius What about the case of --features=feature1,feature2 though? Arguably --features should be called --feature, but as long as it isn't, this particular pattern is very errorprone.

[fzakaria]

I started #21980 and happy to take it over the line given some direction on best way to implement + consensus.

My personal take was that I found the plural name of the config and silent failing when given commas to be a big footgun.

CC @Wyverald who wrote the Feature file.