Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Makefile.include: check FEATURES_CONFLICT against FEATURES_USED #8925

Merged

Conversation

Projects
None yet
3 participants
@cladmi
Copy link
Contributor

commented Apr 11, 2018

The FEATURES_CONFLICT check should be done on used features to also
test for included optional features.

Contribution description

This make the FEATURES_CONFLICT test detect conflict with optional features included.

It can be tested with: FEATURES_OPTIONAL="periph_dac periph_spi" make -C examples/hello-world/

$ FEATURES_OPTIONAL="periph_dac periph_spi" make -C examples/hello-world/ 
BOARD=stm32f4discovery
make: Entering directory '/home/cladmi/git/work/RIOT/examples/hello-world'
The following features may conflict: periph_dac periph_spi
Rationale: On stm32f4discovery boards there are the same pins for the DAC and/or SPI_0.

EXPECT undesired behaviour!
Building application "hello-world" for "stm32f4discovery" with MCU "stm32f4".
...

Issues/PRs references

It will allow using FEATURES_CONFLICT for #8890

Makefile.include: check FEATURES_CONFLICT against FEATURES_USED
The FEATURES_CONFLICT check should be done on used features to also
test for included optional features.
@basilfx

This comment has been minimized.

Copy link
Member

commented Apr 11, 2018

Works as expected and I can repeat the same for the EFM32.

ACK

@basilfx

This comment has been minimized.

Copy link
Member

commented Apr 11, 2018

(since this affects the Makefiles, do we need another ACK, @cladmi, @kaspar030?)

@kaspar030

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2018

I wonder if there's some logic we can implement so that having two optional but conflicting features would select one, or that requiring one conflicting with an optional would just not select the optional.

@cladmi

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2018

@kaspar030 I think the second case could be quite easily done but it would be a new feature where here it's just fixing the check.

With the way it's done now, to handle the one required and one optional case, I would do it with a state in in the Makefile.dep iterations. In the first iterations, optional features are not taken in to account, then when USEMODULE is stable, keep iterating and integrate the optional features that do not conflict until USEMODULE is stable again.

@cladmi cladmi added this to the Release 2018.04 milestone Apr 12, 2018

@kaspar030

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2018

ok!

@kaspar030 kaspar030 merged commit 8ba55c4 into RIOT-OS:master Apr 12, 2018

2 checks passed

Murdock The build succeeded. runtime: 5m:53s
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@cladmi cladmi deleted the cladmi:pr/makefileinclude/features_conflict branch Apr 19, 2018

maxvankessel added a commit to maxvankessel/RIOT that referenced this pull request Apr 20, 2018

Merge pull request RIOT-OS#8925 from cladmi/pr/makefileinclude/featur…
…es_conflict

Makefile.include: check FEATURES_CONFLICT against FEATURES_USED
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.