makefiles, build tests: omit recursive make#7507
Conversation
CI doesn't use buildtest, so reviewers need to test locally. |
|
yeah, sure I know - just for completeness 😉 IIRC we trigger Murdock even for typos in documentation of C files - which isn' checked either). |
btw. please go on and do so 😄 |
| BUILDTESTOK=true; \ | ||
| APP_RETRY=0; \ | ||
| for BOARD in $$($(MAKE) -s info-boards-supported); do \ | ||
| for BOARD in ${BOARDS}; do \ |
There was a problem hiding this comment.
Mhhhh, but this also includes blacklisted/non-whitelisted boards...
There was a problem hiding this comment.
did you actually try? Because I don't see that ...
There was a problem hiding this comment.
i.e., I test with tests/periph_rtc because of #6690 and just added BOARD_BLACKLIST := arduino-zero to its Makefile, which successfully excludes this board though it has the feature periph_rtc
There was a problem hiding this comment.
Mhh... I'm not sure where the BOARDS comes from. But make -s info-boards-supported calculates this... if this is done in another way, we might be able to remove or simplify the info-boards-supported target (but not in this PR)
There was a problem hiding this comment.
(I trust your tests on this for now, I just noticed that by looking at the code ;-))
| FEATURES_REQUIRED += $(FEATURES_OPTIONAL) | ||
|
|
||
| ifneq (, $(filter info-boards-supported info-boards-features-missing info-build, $(MAKECMDGOALS))) | ||
| ifneq (, $(filter buildtest info-boards-supported info-boards-features-missing info-build info-buildsizes info-buildsizes-diff, $(MAKECMDGOALS))) |
There was a problem hiding this comment.
@miri64 the code below generate ${BOARDS} but this ifneq guards the code to be only exec for specific make targets, e.g. for info-boards-supported but w/o this PR not for target buildtest which IMHO make[s] no sense. So I added all targets that require the boards list here and that way the recursive call of make info-boards-supported is obsolete which thereby also fixes described issues with the exported features variables, see e.g. #6690
|
so? anybody willing to test this one, IMHO it resolves some major issues and would therefore be nice to have soonish, or not? |
|
I'll have a look today :-) |
|
@kaspar030 what's your opinion/assessment on this one? You know our build system quiet well. |
|
@haukepetersen can you have a look at this one? You reported #5128, and this should fix that issue. |
haukepetersen
left a comment
There was a problem hiding this comment.
Yes, this fixes the issue I had in #5065, at least for all the examples I just tried. So ACK from my side.
|
and it fixes also the #5128, of course... |
|
Confirmed that make-info-boards-supported still works as expected. Nice one! |
|
Nice! |
This PR fixes problems with exported variables by omitting recursive make, and also fixes an issues with parameter usage for
sizecommand on macOS.Fixes issues discussed in #6690, and likely #5128, too.