build system: Improve failure mode for FEAUTRES_REQUIRED_ANY#20408
Merged
maribu merged 1 commit intoRIOT-OS:masterfrom Feb 22, 2024
Merged
build system: Improve failure mode for FEAUTRES_REQUIRED_ANY#20408maribu merged 1 commit intoRIOT-OS:masterfrom
maribu merged 1 commit intoRIOT-OS:masterfrom
Conversation
maribu
added
CI: ready for build
maribu
added
the
Type: cleanup
mguetschow
reviewed
Feb 22, 2024
Contributor
mguetschow
left a comment
mguetschow
left a comment
There was a problem hiding this comment.
Soft-ACK as I'm not very familiar with RIOT's make system, but looks sensible to me. Just two comment nits.
Failing to provide any of the required features can provide a message
such as:
There are unsatisfied feature requirements: periph_uart|periph_lpuart
This can be confusing and may hide the actual. E.g. above message
was generated when using SPI on the `msb-430` and `stdio_uart`. However,
the MSB-430 board *does* provide `periph_uart`, so this looks like a bug
in the feature resolution. This changes the failure mode of
`FEATURES_REQUIRED_ANY` to just pick the first of the alternatives
given if none of the alternative is usable, which gives in the example
the following message instead:
The following features may conflict: periph_spi periph_uart
Rationale: Both SPI and UART are provided by the same USART
peripheral
The output is less surprising and can provide non-obvious reasons
why `FEATURES_REQUIRED_ANY` failed to pick a feature. The downside is
that the alternatives are no longer visible. However, that output
likely was so confusing this might be for the best.
Co-authored-by: mguetschow <[email protected]>
maribu
force-pushed
the
makefiles/features_check.inc.mk
branch
from
01389ba to
d296523
Compare
Member
Author
|
Thanks. I applied the suggestions and squashed. |
benpicco
approved these changes
Feb 22, 2024
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution description
Failing to provide any of the required features can provide a message such as:
This can be confusing and may hide the actual. E.g. above message was generated when using SPI on the
msb-430andstdio_uart. However, the MSB-430 board does provideperiph_uart, so this looks like a bug in the feature resolution. This changes the failure mode ofFEATURES_REQUIRED_ANYto just pick the first of the alternatives given if none of the alternative is usable, which gives in the example the following message instead:The output is less surprising and can provide non-obvious reasons why
FEATURES_REQUIRED_ANYfailed to pick a feature. The downside is that the alternatives are no longer visible. However, that output likely was so confusing this might be for the best.Testing procedure
The build system unit tests have been updated to match the new failure mode.
Issues/PRs references
Needed for #20366, as the current failure mode of requiring
periph_uart|periph_lpuartwhen neitherperiph_uartnorperiph_lpuartis usable will trigger the test for requiring unknown features.