Skip to content

buildsystem: Fix boards/Makefile.features Inclusion #21334

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 31, 2025
Merged

buildsystem: Fix boards/Makefile.features Inclusion #21334

merged 1 commit into from
Mar 31, 2025

Conversation

crasbe
Copy link
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

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
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
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