dist/tools: add build system sanity check script #10179
Conversation
cladmi
added
Area: tests
|
The question I still have, is should I change the way of handling errors. I took this one from another script but fear it will not scale correctly when adding other tests. It would currently make all errors have only one header with I was thinking about adding a |
|
It is not critical how we present it, as long as it is easy for the user to find and correct them. This issue and the one with RIOTBASE, SCRIPTPATH, excluding the script file from the grep, etc are shared amongst all check scripts, so it may make more sense at some point to put all of that functionality in a shared file and |
|
I am not sure Should I still change something with this PR for the moment ? |
Add a script to execute sanity checks on build system files. It should prevent bad patterns to re-appear after being cleaned. Currently adds a check for using the content of `FEATURES` instead of `USEMODULE`. Modules should not check the content of FEATURES_PROVIDED/_REQUIRED/OPTIONAL Handling specific behaviors/dependencies should by checking the content of: * `USEMODULE` * maybe `FEATURES_USED` if it is not a module (== not a periph_)
cladmi
force-pushed
the
pr/dist/tools/build_system/check
branch
from
ae0bdb3 to
143e393
Compare
|
I tested this PR and got: When testing with #10060 it has no errors. It does make sense to enforce structure in the build system but I cannot say much more than that. |
|
Thank you for testing, and indeed, since I created the PR, the first error line was fixed! @MrKevinWeiss What you can answer, is would you like more output if you get this error on a PR. |
There was a problem hiding this comment.
Tested:
$ ./dist/tools/buildsystem_sanity_check/check.sh
Invalid build system patterns found by ./dist/tools/buildsystem_sanity_check/check.sh:
pkg/fatfs/Makefile.dep:ifneq (, $(filter periph_rtc, $(FEATURES_PROVIDED)))
sys/shell/commands/Makefile:ifneq (,$(filter periph_rtc,$(FEATURES_PROVIDED)))
|
@jcarrano thanks for ACKing, please just wait for @MrKevinWeiss feedback if he would like some changes now before merging. |
|
I think the output is sufficient. If I am editing a makefile and for some reason add a FEATURES_PROVIDED in the wrong section it specifies where it shouldn't be and what exactly it is. I was more stating that I don't understand the details of how this effects the build system and if there are conditions where the violation is warranted. Either way I think it is good and trust your ability so I too will ACK and go! |
|
Thank you for reviewing ! 👍 |
|
This also makes something like ifneq (,$(filter ws281x,$(USEMODULE)))
ifneq (,$(filter arch_avr8,$(FEATURES_PROVIDED)))
USEMODULE += ws281x_atmega
else
$(error No ws281x backend available for the current platform)
endif
endifimpossible. |
|
@benpicco, replace FEATURES_PROVIDED by FEATURES_USED and it should work. |
|
@aabadie that doesn't work - |
|
Well - there is a solution. |
Contribution description
Split from #10060
Add a script to execute sanity checks on build system files.
It should prevent bad patterns to re-appear after being cleaned.
Currently adds a check for using the content of
FEATURESinstead ofUSEMODULE.Modules should not check the content of FEATURES_PROVIDED/_REQUIRED/OPTIONAL
Handling specific behaviors/dependencies should by checking the content of:
USEMODULEFEATURES_USEDif it is not a module (== not a periph_)The script cannot currently be enabled by default as there are some fixes that are not included:
#10060
Testing procedure
The script correctly passes
shellcheckwithout errors.You can list the current errors by running:
It finds the current violations on using
FEATURES_PROVIDED,FEATURES_OPTIONALandFEATURES_REQUIREDto configure specific behavior.With the fixes to these, it runs silently without errors as done in #10060
Issues/PRs references
Split from #10060 and is part of working on #9913