Skip to content

buildsystem: Fix boards/Makefile.features Inclusion#21334

Merged
crasbe merged 1 commit intoRIOT-OS:masterfrom
crasbe:pr/fix_boards_Makefile.include
Mar 31, 2025
Merged

buildsystem: Fix boards/Makefile.features Inclusion#21334
crasbe merged 1 commit intoRIOT-OS:masterfrom
crasbe:pr/fix_boards_Makefile.include

Conversation

@crasbe
Copy link
Copy Markdown
Contributor

@crasbe crasbe commented Mar 28, 2025

Contribution description

During the development of #21332 I noticed that the boards/Makefile.features I introduced in #21327 does not work and unfortunately it is unlikely to ever work, because it suffers from the classic hen-and-egg problem.

The $(RIOTBASE)/Makefile.features (which includes $(RIOTBASE)/boards/Makefile.features) is included before the $(RIOTBASE)/Makefile.dep is included in the dependency check and before the dependencies are resolved, the Make system will check that there are not feature conflicts or errors.

RIOT/Makefile.include

Lines 407 to 408 in e85269a

# process provided features
include $(RIOTBASE)/Makefile.features

RIOT/Makefile.include

Lines 428 to 429 in e85269a

# check if required features are provided and update $(FEATURES_USED)
include $(RIOTMAKE)/features_check.inc.mk

RIOT/Makefile.include

Lines 437 to 438 in e85269a

# process dependencies
include $(RIOTMAKE)/dependency_resolution.inc.mk

While it would be tempting to move the include of $(RIOTBASE)/boards/Makefile.features after the dependency resolution, this won't work because the feature check already failed at that point. I added a $(warning $(USEMODULE)) to the boards/Makefile.features and as you can see, the boards_common_adafruit-nrf52-bootloader USEMODULE is not set yet.
That obviously makes a ifdef that depends on the USEMODULES kind of pointless.

cbuec@W11nMate:~/RIOTstuff/riot-boards-common/RIOT$ BOARD=feather-nrf52840 make -C tests/sys/shell
make: Entering directory '/home/cbuec/RIOTstuff/riot-boards-common/RIOT/tests/sys/shell'
/home/cbuec/RIOTstuff/riot-boards-common/RIOT/boards/Makefile.features:3: app_metadata shell_cmds_default ps ztimer_msec shell_builtin_cmd_help_json
Building application "tests_shell" for "feather-nrf52840" with CPU "nrf52".

"make" -C /home/cbuec/RIOTstuff/riot-boards-common/RIOT/pkg/cmsis/
"make" -C /home/cbuec/RIOTstuff/riot-boards-common/RIOT/boards
...

The other tempting solution would be to move the dependency resolution before the feature check, however the dependency resolution depends on the features being set.

Therefore the not-super-elegant solution is to include the boards/Makefile.features during the dependency resolution.
Since the dependency resolution runs in a loop until all features are stable, this works as expected and the boards_common_adafruit-nrf52-bootloader is part of the USEMODULES after the second run of the dependency resolution:

cbuec@W11nMate:~/RIOTstuff/riot-boards-common/RIOT$ BOARD=feather-nrf52840 make -C tests/sys/shell
make: Entering directory '/home/cbuec/RIOTstuff/riot-boards-common/RIOT/tests/sys/shell'
/home/cbuec/RIOTstuff/riot-boards-common/RIOT/boards/Makefile.features:3: app_metadata board board_common_init core core_init core_lib core_msg core_panic core_thread cpu libc ps shell_builtin_cmd_help_json shell_cmds_default sys test_utils_interactive_sync test_utils_print_stack_usage ztimer_msec stdio_default boards_common_adafruit-nrf52-bootloader nrf52_vectors nrf5x_common_periph cpu_common cortexm_common cortexm_common_periph periph malloc_thread_safe shell_cmds shell stdio_uart stdio ztimer_core ztimer_extend ztimer ztimer_periph_rtt stdin stdio_cdc_acm usb_board_reset stdin shell_cmd_sys shell_cmd_app_metadata shell_cmd_ps app_metadata ps ztimer_core ztimer_convert_frac ztimer_convert_shift ztimer_core ztimer_extend ztimer_convert frac ztimer ztimer_periph_rtt
/home/cbuec/RIOTstuff/riot-boards-common/RIOT/boards/Makefile.features:3: app_metadata board board_common_init boards_common_adafruit-nrf52-bootloader core core_init core_lib core_msg core_panic core_thread cortexm_common cortexm_common_periph cpu cpu_common frac libc malloc_thread_safe newlib nrf52_vectors nrf5x_common_periph periph periph_common periph_gpio periph_pm periph_rtt periph_uart ps shell shell_builtin_cmd_help_json shell_cmd_app_metadata shell_cmd_ps shell_cmd_sys shell_cmds shell_cmds_default stdin stdio stdio_cdc_acm stdio_default stdio_uart sys test_utils_interactive_sync test_utils_print_stack_usage usb_board_reset ztimer ztimer_convert ztimer_convert_frac ztimer_convert_shift ztimer_core ztimer_extend ztimer_msec ztimer_periph_rtt boards_common_adafruit-nrf52-bootloader nrf52_vectors nrf5x_common_periph cpu_common cortexm_common cortexm_common_periph periph malloc_thread_safe newlib_syscalls_default div shell_cmds shell stdio isrpipe stdio_dispatch usbus_cdc_acm isrpipe stdio_available stdio_uart_rx isrpipe stdio_uart stdio_available usbus core_thread_flags event luid fmt tsrb ztimer_core ztimer_convert_frac ztimer_convert_shift ztimer_core ztimer_extend ztimer_convert frac ztimer ztimer_periph_rtt stdin stdio_cdc_acm usb_board_reset core_thread_flags tsrb stdin shell_cmd_sys shell_cmd_app_metadata shell_cmd_pm shell_cmd_ps app_metadata ps ztimer_core ztimer_convert_frac ztimer_convert_shift ztimer_core ztimer_extend ztimer_convert frac ztimer ztimer_periph_rtt
/home/cbuec/RIOTstuff/riot-boards-common/RIOT/boards/Makefile.features:3: app_metadata board board_common_init boards_common_adafruit-nrf52-bootloader core core_init core_lib core_msg core_panic core_thread core_thread_flags cortexm_common cortexm_common_periph cpu cpu_common div event fmt frac isrpipe libc luid malloc_thread_safe newlib newlib_syscalls_default nrf52_vectors nrf5x_common_periph periph periph_common periph_cpuid periph_gpio periph_gpio_ll_disconnect periph_gpio_ll_input_pull_down periph_gpio_ll_input_pull_up periph_gpio_ll_irq_edge_triggered_both periph_gpio_ll_irq_unmask periph_gpio_ll_open_drain periph_gpio_ll_open_drain_pull_up periph_gpio_ll_open_source periph_gpio_ll_open_source_pull_down periph_pm periph_rtt periph_uart periph_usbdev ps shell shell_builtin_cmd_help_json shell_cmd_app_metadata shell_cmd_pm shell_cmd_ps shell_cmd_sys shell_cmds shell_cmds_default stdin stdio stdio_available stdio_cdc_acm stdio_default stdio_dispatch stdio_uart stdio_uart_rx sys test_utils_interactive_sync test_utils_print_stack_usage tsrb usb_board_reset usbus usbus_cdc_acm ztimer ztimer_convert ztimer_convert_frac ztimer_convert_shift ztimer_core ztimer_extend ztimer_msec ztimer_periph_rtt boards_common_adafruit-nrf52-bootloader nrf52_vectors nrf5x_common_periph cpu_common cortexm_common cortexm_common_periph periph malloc_thread_safe div shell_cmds shell stdio isrpipe stdio_dispatch usbus_cdc_acm isrpipe stdio_available stdio_uart_rx isrpipe stdio_uart stdio_available usbus core_thread_flags event luid fmt tsrb ztimer_core ztimer_convert_frac ztimer_convert_shift ztimer_core ztimer_extend ztimer_convert frac ztimer ztimer_periph_rtt stdin periph_usbdev_clk stdio_cdc_acm usb_board_reset core_thread_flags tsrb stdin shell_cmd_sys shell_cmd_app_metadata shell_cmd_pm shell_cmd_ps app_metadata ps ztimer_core ztimer_convert_frac ztimer_convert_shift ztimer_core ztimer_extend ztimer_convert frac ztimer ztimer_periph_rtt
/home/cbuec/RIOTstuff/riot-boards-common/RIOT/boards/Makefile.features:3: app_metadata board board_common_init boards_common_adafruit-nrf52-bootloader core core_init core_lib core_msg core_panic core_thread core_thread_flags cortexm_common cortexm_common_periph cpu cpu_common div event fmt frac isrpipe libc luid malloc_thread_safe newlib newlib_syscalls_default nrf52_vectors nrf5x_common_periph periph periph_common periph_cpuid periph_gpio periph_gpio_ll_disconnect periph_gpio_ll_input_pull_down periph_gpio_ll_input_pull_up periph_gpio_ll_irq_edge_triggered_both periph_gpio_ll_irq_unmask periph_gpio_ll_open_drain periph_gpio_ll_open_drain_pull_up periph_gpio_ll_open_source periph_gpio_ll_open_source_pull_down periph_pm periph_rtt periph_uart periph_usbdev periph_usbdev_clk ps shell shell_builtin_cmd_help_json shell_cmd_app_metadata shell_cmd_pm shell_cmd_ps shell_cmd_sys shell_cmds shell_cmds_default stdin stdio stdio_available stdio_cdc_acm stdio_default stdio_dispatch stdio_uart stdio_uart_rx sys test_utils_interactive_sync test_utils_print_stack_usage tsrb usb_board_reset usbus usbus_cdc_acm ztimer ztimer_convert ztimer_convert_frac ztimer_convert_shift ztimer_core ztimer_extend ztimer_msec ztimer_periph_rtt boards_common_adafruit-nrf52-bootloader nrf52_vectors nrf5x_common_periph cpu_common cortexm_common cortexm_common_periph periph malloc_thread_safe div shell_cmds shell stdio isrpipe stdio_dispatch usbus_cdc_acm isrpipe stdio_available stdio_uart_rx isrpipe stdio_uart stdio_available usbus core_thread_flags event luid fmt tsrb ztimer_core ztimer_convert_frac ztimer_convert_shift ztimer_core ztimer_extend ztimer_convert frac ztimer ztimer_periph_rtt stdin periph_usbdev_clk stdio_cdc_acm usb_board_reset core_thread_flags tsrb stdin shell_cmd_sys shell_cmd_app_metadata shell_cmd_pm shell_cmd_ps app_metadata ps ztimer_core ztimer_convert_frac ztimer_convert_shift ztimer_core ztimer_extend ztimer_convert frac ztimer ztimer_periph_rtt
Building application "tests_shell" for "feather-nrf52840" with CPU "nrf52".

"make" -C /home/cbuec/RIOTstuff/riot-boards-common/RIOT/pkg/cmsis/
"make" -C /home/cbuec/RIOTstuff/riot-boards-common/RIOT/boards
"make" -C /home/cbuec/RIOTstuff/riot-boards-common/RIOT/boards/common/adafruit-nrf52-bootloader
...

Testing procedure

Testing this is a bit abstract as no board uses the global boards/Makefile.features yet, but I added this line to the boards/Makefile.features, which prints the $(USEMODULE) variable.

# SORT THIS ALPHABETICALLY BY COMMON BOARD NAME!

$(warning $(USEMODULE))

You can test it with every application you like, I used the tests/sys/shell and the feather-nrf52840 board.
You can test it with every board that uses the adafruit-nrf52-bootloader and you don't even have to have the actual board for testing this change.

Issues/PRs references

Fixes a bug introduced by #21327.

@crasbe crasbe added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Mar 28, 2025
@github-actions github-actions bot added the Area: build system Area: Build system label Mar 28, 2025
@crasbe crasbe mentioned this pull request Mar 28, 2025
Merged
@mguetschow mguetschow added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 28, 2025
@riot-ci
Copy link
Copy Markdown

riot-ci commented Mar 28, 2025

Murdock results

✔️ PASSED

8ea9a42 buildsystem: fix boards/Makefile.features inclusion

Success Failures Total Runtime
10287 0 10287 09m:20s

Artifacts

mguetschow
mguetschow approved these changes Mar 29, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, not great, but I don't have a better idea either. Thanks for fixing!

@mguetschow mguetschow enabled auto-merge March 29, 2025 07:14
@mguetschow mguetschow added this pull request to the merge queue Mar 29, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 29, 2025
@crasbe crasbe added this pull request to the merge queue Mar 29, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 29, 2025
@crasbe crasbe added this pull request to the merge queue Mar 31, 2025
Merged via the queue into RIOT-OS:master with commit 35afc64 Mar 31, 2025
28 checks passed
@crasbe crasbe deleted the pr/fix_boards_Makefile.include branch March 31, 2025 13:40
@mguetschow mguetschow added this to the Release 2025.04 milestone Apr 8, 2025
@crasbe crasbe mentioned this pull request Oct 30, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mguetschow mguetschow mguetschow approved these changes

Assignees

No one assigned

Labels

Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2025.04

Development

Successfully merging this pull request may close these issues.

3 participants

@crasbe @riot-ci @mguetschow