Makefile.include: perform checks for all relevant targets #21199
Conversation
|
I wonder if it makes sense to have the target check at all or to exclude targets where the check should not be executed. But both have a higher risk of breaking something if we miss something I guess? |
|
I think I like the approach of excluding targets where the test is skipped. I can only think of |
|
So essentially we could do something like this: # Warn if the selected board and drivers don't provide all needed features:
ifeq (, $(filter help generate-Makefile.ci, $(if $(MAKECMDGOALS), $(MAKECMDGOALS), all)))
...
endifWhat's odd though: $(MAKECMDGOALS) sometimes seems to be undefined for the 'help' target. I added The reason is a bit stupid and I don't really know how to fix it. This is the help target and it calls... make. But without a target I played around with this, and of course, Stack Overflow came to the rescure. Apparently, Log:``` RIOT/examples/openthread$ BOARD=nrf52dk make help /home/cbuec/riot-dev/RIOT/examples/openthread/../../Makefile.include:936: MAKECMDGOALS is 'help' all application_openthread.module auto_init.module bindist binfile board.module board_common_init.module boards_common_nrf52xxxdk.module check-toolchain-supported check_bindist clang-tidy clean clean-intermediates clean-pkg compile-commands core.module core_lib.module cortexm_common.module cortexm_common_periph.module cosy cpp11-compat.module cpp_new_delete.module cpu.module cpu_common.module debug debug-client debug-server dependency-debug dependency-debug-features-provided-kconfig distclean div.module doc eclipsesym eclipsesym.xml elffile eui_provider.module event.module flash flash-only flashfile fmt.module frac.module fuzz hexfile info-boards info-build info-build-json info-buildsize info-concurrency info-cpu info-emulated-boards info-features-missing info-features-provided info-features-required info-features-used info-kconfig-variables info-modules info-objsize info-packages info-programmers-supported info-rust info-toolchains-supported l2util.module libc.module link list-ttys lstfile luid.module malloc_thread_safe.module mosquitto_rsmb netdev.module newlib_syscalls_default.module nrf52_vectors.module nrf5x_common_periph.module objdump openthread-cli-ftd.module openthread-ftd.module openthread_contrib.module openthread_contrib_netdev.module periph.module periph_common.module pkg-build pkg-prepare preflash prepare_check_bindist preprocessor.module print-size ps.module random.module reset scan-build-analyze stdio.module stdio_uart.module sys.module term termdeps test-as-root/available test-input-hash test-input-hash-changed test-with-config/check-config test/available timex.module usb_id_check ztimer64.module ztimer_core.module ```So we mix and match with the command we already had: help:
# filter all targets starting with lowercase and containing lowercase letters, hyphens, or underscores; explicitly include generate-Makefile.ci
+ # inspired by: https://stackoverflow.com/a/26339924
- @$(MAKE) -qp | sed -ne 's/\(^[a-z][a-z_-]*\|generate-Makefile.ci\):.*/\1/p' | sort -u
+ @LC_ALL=C $(MAKE) -pRrq -f $(firstword $(MAKEFILE_LIST)) : 2>/dev/null | sed -ne 's/\(^[a-z][a-z_-]*\|generate-Makefile.ci\):.*/\1/p' | sort -u
+ # IMPORTANT: The line above must be indented by (at least one) actual TAB character - spaces do *not* work.
This yields what we previously had: Log:The comment about the tab is probably optional, because the tab was necessary previously as well. I don't really understand why, but the make gods probably have their reasons 🤣 |
|
I'm not sure if it's worth separating this into two commits, but it is somewhat related and somewhat unrelated. |
|
Please squash then :) |
5846576 to
2e9a01c
Compare
|
Something broke during squashing. |
Here's the change to undo: https://github.com/RIOT-OS/RIOT/compare/5846576fa50eda52a01c9abcb3439753e7c073e3..2e9a01c13b237c94a1f3def1d66335fca7afece1 |
2e9a01c to
240f451
Compare
|
Yes, I think I squashed it in the wrong order and then something else got inbetween as well. I thought about adding |
Yeah, I'd second that reasoning. In particular, |
Co-authored-by: mguetschow <mikolai.guetschow@tu-dresden.de>
Co-authored-by: mguetschow <mikolai.guetschow@tu-dresden.de>
7695d0a to
91dae97
Compare
|
Thank you @mguetschow :) |
Contribution description
This is mostly documented in #21198.
tl;dr: When building an application that specifies some restrictions such as
BOARD_WHITELIST,BOARD_BLACKLIST, etc, the check inMakefile.includeis only performed for theallandflashtargets, not for any other target that builds the code base.Therefore trying to build an application such as
examples/openthreadwith an unsupported board such asnrf52dkwill not be blocked, instead make will try to build it and fail on an unrelated error.I don't expect any bad sideeffects from this.
Testing procedure
Execute
BOARD=nrf52dk make -C examples/openthread. Make will quite with an error message that the board is not supported (as it should):On master: Execute
BOARD=nrf52dk make -C examples/openthread hexfile. Make will attempt to build the application but will fail due to an undefined constant.Log:
With this PR: Execute
BOARD=nrf52dk make -C examples/openthread hexfile. Make willIssues/PRs references
Fixes #21198.