Skip to content
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.

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

APPLICATION_CONFIG_DIR, CONF_FILE do not always pick up local boards/*.conf #51621

Closed
gregshue opened this issue Oct 25, 2022 · 5 comments
Closed
Assignees

Comments

@gregshue
Copy link

Describe the bug
While creating a new sample in the MCUBoot module, in the local, top-level CMakeLists.txt I set APPLICATION_CONFIG_DIR to point to the existing sample, and set CONF_DIR to be a list of ${APPLICATION_CONF_DIR}/prj.conf and the local prj.conf. The expectation was that the build system would find and apply ${APPLICATION_CONF_DIR}/boards/${target}.conf. It did not, resulting in the new image being a different size than the original. I ended up copying over all boards/ and overlay files to the local sample to get the built image to match expectations.
See MCUBoot PR #1489 for the original situation.

Please also mention any information which could help others to understand
the problem you're facing:

  • What target platform are you using? All those in relevant sample.yaml and testcase.yaml files.
  • What have you tried to diagnose or workaround this issue? Multiple diagnostic builds.
  • Is this a regression? If yes, have you been able to "git bisect" it to a
    specific commit? Unknown.
  • ...

To Reproduce
Once mcuboot in zephyrproject-rtos has been updated with MCUBoot PR #1489:
Steps to reproduce the behavior:

  1. In bootloader/mcuboot/zephyr/samples/mcuboot_external_config/CMakeLists.txt add the following lines before find_package():
set(APPLICATION_CONFIG_DIR "\${ZEPHYR_MCUBOOT_MODULE_DIR}/boot/zephyr")

list(APPEND CONF_FILE ${APPLICATION_CONFIG_DIR}/prj.conf)
list(APPEND CONF_FILE ${CMAKE_CURRENT_SOURCE_DIR}/prj.conf)
  1. Delete bootloader/mcuboot/zephyr/samples/boards/
  2. west build -p always -b nrf52840kc_nrf52840 bootloader/mcuboot/zephyr/samples/mcuboot_external_config
  3. Compare the size of the built image with the size built from bootloader/mcuboot/boot/zephyr.
  4. See the difference.

Expected behavior
I expect the build system to pick up the boards directories from both APPLICATION_CONFIG_DIR and then my local boards/ directory so that I can cascade content together under the control of my local CMakeLists.txt.

Impact
What impact does this issue have on your progress (e.g., annoyance, showstopper)? This is a maintenance burden.

Logs and console output
If applicable, add console logs or other types of debug information
e.g Wireshark capture or Logic analyzer capture (upload in zip archive).
copy-and-paste text and put a code fence (```) before and after, to help
explain the issue. (if unable to obtain text log, add a screenshot)

Environment (please complete the following information):

  • OS: (e.g. Linux, MacOS, Windows): Linux
  • Toolchain (e.g Zephyr SDK, ...): Zephyr SDK
  • Commit SHA or Version used: Zephyr f93c682

Additional context
Add any other context that could be relevant to your issue, such as pin setting,
target configuration, ...

@gregshue gregshue added the bug The issue is a bug, or the PR is fixing a bug label Oct 25, 2022
@laurenmurphyx64 laurenmurphyx64 added priority: low Low impact/importance bug priority: medium Medium impact/importance bug and removed priority: low Low impact/importance bug labels Nov 1, 2022
@nordicjm
Copy link
Collaborator

nordicjm commented Nov 9, 2022

bootloader/mcuboot/zephyr/samples/mcuboot_external_config/CMakeLists.txt does not exist with that PR.
bootloader/mcuboot/zephyr/samples/boards/ does not exist with that PR.
Can you fix these this and check the reproduction steps work?

@nordicjm
Copy link
Collaborator

nordicjm commented Nov 9, 2022

And from what I can see this is not a bug, this is intended design.
https://docs.zephyrproject.org/latest/develop/application/index.html

Zephyr will use configuration files from the application’s configuration directory except for files with an absolute path provided by the arguments described earlier, for example CONF_FILE, OVERLAY_CONFIG, and DTC_OVERLAY_FILE.

The application configuration directory is defined by the APPLICATION_CONFIG_DIR variable.

APPLICATION_CONFIG_DIR will be set by one of the sources below with the highest priority listed first.

If APPLICATION_CONFIG_DIR is specified by the user with -DAPPLICATION_CONFIG_DIR= or in a CMake file before find_package(Zephyr) then this folder is used a the application’s configuration directory.

The application’s source directory.

And this is confirmed by the cmake files:

zephyr_get(APPLICATION_CONFIG_DIR)
if(DEFINED APPLICATION_CONFIG_DIR)
  string(CONFIGURE ${APPLICATION_CONFIG_DIR} APPLICATION_CONFIG_DIR)
  if(NOT IS_ABSOLUTE ${APPLICATION_CONFIG_DIR})
    get_filename_component(APPLICATION_CONFIG_DIR ${APPLICATION_CONFIG_DIR} ABSOLUTE)
  endif()
else()
  # Application config dir is not set, so we default to the  application
  # source directory as configuration directory.
  set(APPLICATION_CONFIG_DIR ${APPLICATION_SOURCE_DIR})
endif()

And also described as working like this on the sysbuild pages:

In the folder of the main application, create a new folder under sysbuild//. This folder will then be used as APPLICATION_CONFIG_DIR when building . As an example, if your main application includes my_sample then create a sysbuild/my_sample/ folder and place any configuration files in there as you would normally do:

All configuration files under the sysbuild/my_sample/ folder will now be used when my_sample is included in the build, and the default configuration files for my_sample will be ignored.

This give you full control on how images are configured when integrating those with application.

@nordicjm nordicjm removed the bug The issue is a bug, or the PR is fixing a bug label Nov 9, 2022
@gregshue
Copy link
Author

gregshue commented Nov 9, 2022

@nordicjm Thank you for the comments. I should not have referenced an open PR. The files moved and may move again. Fortunately, this module currently has only one sample beneath $mcuboot/zephyr/samples/. I ended up mistyping the path to the sample-specific boards/ subdirectory, but the issue requires that directory to be removed anyway. When I applied the pattern of changes to the current file versions in the new location of $mcuboot/zephyr/samples/modules/mcuboot/mcuboot_external_config/[boards/] and built with the command above I still ended up with a build size that is too large. This seems to be due to the build system function zephyr_file() not picking up the fragment in ${APPLICATION_CONFIG_DIR}/boards/

Perhaps I should try to reproduce it from the test suite that verifies this build functionality. Where is that suite?

@laurenmurphyx64 laurenmurphyx64 added Enhancement Changes/Updates/Additions to existing features and removed priority: medium Medium impact/importance bug labels Nov 15, 2022
@tejlmand
Copy link
Collaborator

While creating a new sample in the MCUBoot module, in the local, top-level CMakeLists.txt I set APPLICATION_CONFIG_DIR to point to the existing sample, and set CONF_DIR to be a list of ${APPLICATION_CONF_DIR}/prj.conf and the local prj.conf. The expectation was that the build system would find and apply ${APPLICATION_CONF_DIR}/boards/${target}.conf.

Which is a fundamental misunderstanding of how APPLICATION_CONFIG_DIR works.

Read the docs: https://docs.zephyrproject.org/latest/develop/application/index.html#application-configuration

If APPLICATION_CONFIG_DIR is specified by the user with -DAPPLICATION_CONFIG_DIR=<path> or in a CMake file before find_package(Zephyr) then this folder is used a the application’s configuration directory.

This means that all additional files, such as board specific files under ${APPLICATION_CONFIG_DIR}/boards are being used.
So when setting APPLICATION_CONFIG_DIR you're asking the build system to automatically pick up prj.conf and boards/<board.conf> to be picked up from there, but remember other rules still applies, see:
https://docs.zephyrproject.org/latest/build/kconfig/setting.html#the-initial-configuration

as also commented here: mcu-tools/mcuboot#1524 (review)
this is a misunderstanding on how the build and configuration system work.

@tejlmand tejlmand removed the Enhancement Changes/Updates/Additions to existing features label Nov 22, 2022
@tejlmand
Copy link
Collaborator

Closed as this request is a misunderstanding of how the feature works and how it is intended to work.

@tejlmand tejlmand closed this as not planned Won't fix, can't repro, duplicate, stale Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants