build system: don't optionally use conflicting features

[maribu]

Contribution description

As the title says: If a feature is optional and using it would cause a feature conflict, the expectation is that it doesn't end up being used. This adds the required testing.

An additional a unit test to cover this case was added, so that no a future regression would be detected.

Testing procedure

See #16008 (comment)

Issues/PRs references

Detected in #16008 (comment)

[fjmolinas]

Can we really avoid this? Dependency resolution is iterative. If in round one there is only FEATURES_OPTIONAL=a it will get used, but if in round n you get FEATURES_BLACKLIST=a there will still be a conflict. I don't see the harm in the change though, but I don't think it's avoidable, so we can still get this in.

[maribu]

Can we really avoid this? Dependency resolution is iterative. If in round one there is only FEATURES_OPTIONAL=a it will get used, but if in round n you get FEATURES_BLACKLIST=a there will still be a conflict. I don't see the harm in the change though, but I don't think it's avoidable, so we can still get this in.

Both blacklist and list of conflicts should do my understanding not grow. But transitive dependencies to modules might pull in a required feature too late, as an optional feature conflicting with that could be already used.

Still, best effort is likely good enough here for all but a few rare cases. And these could be addressed by tweaking the dependency resolution so that the conflicting required feature gets added in the some round as the optional feature it conflicts with. (But I hope that such occurrences won't show up soon, so that we migrate away in time.)

[fjmolinas]

Both blacklist and list of conflicts should do my understanding not grow. But transitive dependencies to modules might pull in a required feature too late, as an optional feature conflicting with that could be already used.

I think MODULES and PKG can blacklist things...or maybe it's only TOOLCHAINS? I'm unsure.

[fjmolinas]

Still, best effort is likely good enough here for all but a few rare cases. And these could be addressed by tweaking the dependency resolution so that the conflicting required feature gets added in the some round as the optional feature it conflicts with. (But I hope that such occurrences won't show up soon, so that we migrate away in time.)

Yes, of course, just pointing it out. I doubled checked and mostly its arch features that are blacklisted, and those are FEATURES_REQUIRED. There is a couple of cases where bootloader* and ble* are blacklisted as well.

[fjmolinas]

Still, best effort is likely good enough here for all but a few rare cases. And these could be addressed by tweaking the dependency resolution so that the conflicting required feature gets added in the some round as the optional feature it conflicts with. (But I hope that such occurrences won't show up soon, so that we migrate away in time.)

Yes, of course, just pointing it out. I doubled checked and mostly its arch features that are blacklisted, and those are FEATURES_REQUIRED. There is a couple of cases where bootloader* and ble* are blacklisted as well.

Maybe those can be removed? Maybe just a doc comment? Or we stick with best effort? What do you think?

[maribu]

Yes, of course, just pointing it out. I doubled checked and mostly its arch features that are blacklisted, and those are FEATURES_REQUIRED. There is a couple of cases where bootloader* and ble* are blacklisted as well.

Maybe those can be removed? Maybe just a doc comment? Or we stick with best effort? What do you think?

I'm not sure if there is indeed so much to gain that it would justify the effort, having the migration to Kconfig in mind. (Just thinking about the amount of quirks in the build system that can be dropped afterwards make me smile :-))

[fjmolinas]

Lets spin up the ci

[fjmolinas]

Please squash @maribu

[fjmolinas]

This change seems unneeded now (moving it up)

[maribu]

Moved down again and squashed

[fjmolinas]

ACK!