Skip to content

Conversation

@tejlmand
Copy link
Contributor

@tejlmand tejlmand commented May 17, 2024

This PR refactors sysbuild entry code by creating a CMake sysbuild
module for image processing and place sysbuild entry code in a
SysbuildLists.txt file.

The SysbuildLists.txt file will be use as template for applications
which doesn't provide their own entry file.

An application may place a SysbuildLists.txt file at its toplevel
folder. The SysbuildLists.txt file is similar in nature to the
CMakeLists.txt file for the application and allows application
developers to adjust how an application is built with sysbuild.

Also part of this PR is APPLICATION_CONFIG_DIR support in sysbuild.


Additional background info

The name SysbuildLists.txt is proposed as it allows the file to be placed at the sample's top-level folder, next to the CMakeLists.txt.
Also the name reflects the intention of the file.
However, one implication is that the file is included using include() statement inside sysbuild, meaning the use of add_subdirectory() must include bin dir, like this:

https://github.com/zephyrproject-rtos/zephyr/blob/1f494ea61a401af32d42264e6b75e9eee1a29dc6/share/sysbuild/SysbuildLists.txt#L10-L12

An alternative to SysbuildLists.txt could be a subfolder, like <sample>/sysbuild/CMakeLists.txt, but that makes it less visible as well as less clear of the intention (being entry point for sysbuild).

@zephyrbot zephyrbot requested review from jeremybettis and nashif May 17, 2024 09:40
@tejlmand tejlmand force-pushed the sysbuild/application_config_dir branch from 861153a to 29302df Compare May 17, 2024 09:45
@tejlmand tejlmand requested review from MarekPieta and pdunaj May 17, 2024 09:59
@tejlmand tejlmand force-pushed the sysbuild/application_config_dir branch 2 times, most recently from 96bd5c1 to fb7432b Compare May 17, 2024 12:22
Copy link
Contributor

@MarekPieta MarekPieta left a comment

Choose a reason for hiding this comment

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

PR adds the missing feature and allows me to configure application in more flexible way. Thanks a lot for the change!

Still it may be good to let someone with more experience in CMake and Sysbuild to take a look to ensure it would always work as expected

Copy link
Contributor

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

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

Seems to completely break things, without:

Loading Zephyr module(s) (Zephyr repository): extensions sysbuild_extensions python west root zephyr_module boards shields hwm_v2 sysbuild_kconfig native_simulator_sb_extensions
-- Found Python3: /usr/bin/python (found suitable version "3.11.8", minimum required is "3.8") found components: Interpreter
-- Cache files will be written to: /home/jamie/.cache/zephyr
-- Found west (found suitable version "1.2.0", minimum required is "0.14.0")
-- Board: nrf52840dk, qualifiers: nrf52840
Parsing /tmp/aa/zephyr/share/sysbuild/Kconfig
Loaded configuration '/tmp/aa/zephyr/samples/hello_world/_AA/empty.conf'
Merged configuration '/tmp/aa/zephyr/samples/hello_world/_AA/empty.conf'
Merged configuration '/tmp/aa/zephyr/samples/hello_world/_AA/empty.conf'
Configuration saved to '/tmp/aa/zephyr/samples/hello_world/_AA/zephyr/.config'
Kconfig header saved to '/tmp/aa/zephyr/samples/hello_world/_AA/autoconf.h'
-- 
   *********************************
   * Running CMake for hello_world *
   *********************************

Loading Zephyr default modules (Zephyr repository).
-- Application: /tmp/aa/zephyr/samples/hello_world
-- CMake version: 3.29.2
-- Found Python3: /usr/bin/python (found suitable version "3.11.8", minimum required is "3.8") found components: Interpreter
-- Cache files will be written to: /home/jamie/.cache/zephyr
-- Zephyr version: 3.6.99 (/tmp/aa/zephyr)
-- Found west (found suitable version "1.2.0", minimum required is "0.14.0")
-- Board: nrf52840dk, qualifiers: nrf52840
-- ZEPHYR_TOOLCHAIN_VARIANT not set, trying to locate Zephyr SDK
-- Found host-tools: zephyr 0.16.5 (/opt/zephyr-sdk-0.16.5-rc1)
-- Found toolchain: zephyr 0.16.5 (/opt/zephyr-sdk-0.16.5-rc1)
-- Found Dtc: /opt/zephyr-sdk-0.16.5-rc1/sysroots/x86_64-pokysdk-linux/usr/bin/dtc (found suitable version "1.6.0", minimum required is "1.4.6")
-- Found BOARD.dts: /tmp/aa/zephyr/boards/nordic/nrf52840dk/nrf52840dk_nrf52840.dts
-- Generated zephyr.dts: /tmp/aa/zephyr/samples/hello_world/_AA/hello_world/zephyr/zephyr.dts
-- Generated devicetree_generated.h: /tmp/aa/zephyr/samples/hello_world/_AA/hello_world/zephyr/include/generated/devicetree_generated.h
-- Including generated dts.cmake file: /tmp/aa/zephyr/samples/hello_world/_AA/hello_world/zephyr/dts.cmake
Parsing /tmp/aa/zephyr/Kconfig
Loaded configuration '/tmp/aa/zephyr/boards/nordic/nrf52840dk/nrf52840dk_nrf52840_defconfig'
Merged configuration '/tmp/aa/zephyr/samples/hello_world/prj.conf'
Merged configuration '/tmp/aa/zephyr/samples/hello_world/_AA/hello_world/zephyr/.config.sysbuild'
Configuration saved to '/tmp/aa/zephyr/samples/hello_world/_AA/hello_world/zephyr/.config'
Kconfig header saved to '/tmp/aa/zephyr/samples/hello_world/_AA/hello_world/zephyr/include/generated/autoconf.h'
-- Found GnuLd: /opt/zephyr-sdk-0.16.5-rc1/arm-zephyr-eabi/arm-zephyr-eabi/bin/ld.bfd (found version "2.38")
-- The C compiler identification is GNU 12.2.0
-- The CXX compiler identification is GNU 12.2.0
-- The ASM compiler identification is GNU
-- Found assembler: /opt/zephyr-sdk-0.16.5-rc1/arm-zephyr-eabi/bin/arm-zephyr-eabi-gcc
-- Configuring done (5.9s)
-- Generating done (0.1s)
-- Build files have been written to: /tmp/aa/zephyr/samples/hello_world/_AA/hello_world
-- Configuring done (8.6s)
-- Generating done (0.0s)
-- Build files have been written to: /tmp/aa/zephyr/samples/hello_world/_AA

With:

Loading Zephyr module(s) (Zephyr repository): sysbuild_default
-- Found Python3: /usr/bin/python (found suitable version "3.11.8", minimum required is "3.8") found components: Interpreter
-- Cache files will be written to: /home/jamie/.cache/zephyr
-- Found west (found suitable version "1.2.0", minimum required is "0.14.0")
-- Board: nrf52840dk, qualifiers: nrf52840
Parsing /tmp/aa/zephyr/share/sysbuild/Kconfig
Loaded configuration '/tmp/aa/zephyr/samples/hello_world/_AA/empty.conf'
Merged configuration '/tmp/aa/zephyr/samples/hello_world/_AA/empty.conf'
Merged configuration '/tmp/aa/zephyr/samples/hello_world/_AA/empty.conf'
Configuration saved to '/tmp/aa/zephyr/samples/hello_world/_AA/zephyr/.config'
Kconfig header saved to '/tmp/aa/zephyr/samples/hello_world/_AA/autoconf.h'
-- 
   *********************************
   * Running CMake for hello_world *
   *********************************

Loading Zephyr default modules (Zephyr repository).
-- Application: /tmp/aa/zephyr/samples/hello_world
-- CMake version: 3.29.2
-- Found Python3: /usr/bin/python (found suitable version "3.11.8", minimum required is "3.8") found components: Interpreter
-- Cache files will be written to: /home/jamie/.cache/zephyr
-- Zephyr version: 3.6.99 (/tmp/aa/zephyr)
-- Found west (found suitable version "1.2.0", minimum required is "0.14.0")
-- Board: nrf52840dk, qualifiers: nrf52840
-- ZEPHYR_TOOLCHAIN_VARIANT not set, trying to locate Zephyr SDK
-- Found host-tools: zephyr 0.16.5 (/opt/zephyr-sdk-0.16.5-rc1)
-- Found toolchain: zephyr 0.16.5 (/opt/zephyr-sdk-0.16.5-rc1)
-- Found Dtc: /opt/zephyr-sdk-0.16.5-rc1/sysroots/x86_64-pokysdk-linux/usr/bin/dtc (found suitable version "1.6.0", minimum required is "1.4.6")
-- Found BOARD.dts: /tmp/aa/zephyr/boards/nordic/nrf52840dk/nrf52840dk_nrf52840.dts
-- Generated zephyr.dts: /tmp/aa/zephyr/samples/hello_world/_AA/hello_world/zephyr/zephyr.dts
-- Generated devicetree_generated.h: /tmp/aa/zephyr/samples/hello_world/_AA/hello_world/zephyr/include/generated/devicetree_generated.h
-- Including generated dts.cmake file: /tmp/aa/zephyr/samples/hello_world/_AA/hello_world/zephyr/dts.cmake
Parsing /tmp/aa/zephyr/Kconfig
Loaded configuration '/tmp/aa/zephyr/boards/nordic/nrf52840dk/nrf52840dk_nrf52840_defconfig'
Merged configuration '/tmp/aa/zephyr/samples/hello_world/prj.conf'
Merged configuration '/tmp/aa/zephyr/samples/hello_world/_AA/hello_world/zephyr/.config.sysbuild'
Configuration saved to '/tmp/aa/zephyr/samples/hello_world/_AA/hello_world/zephyr/.config'
Kconfig header saved to '/tmp/aa/zephyr/samples/hello_world/_AA/hello_world/zephyr/include/generated/autoconf.h'
-- Found GnuLd: /opt/zephyr-sdk-0.16.5-rc1/arm-zephyr-eabi/arm-zephyr-eabi/bin/ld.bfd (found version "2.38")
-- The C compiler identification is GNU 12.2.0
-- The CXX compiler identification is GNU 12.2.0
-- The ASM compiler identification is GNU
-- Found assembler: /opt/zephyr-sdk-0.16.5-rc1/arm-zephyr-eabi/bin/arm-zephyr-eabi-gcc
-- Configuring done (5.8s)
-- Generating done (0.1s)
-- Build files have been written to: /tmp/aa/zephyr/samples/hello_world/_AA/hello_world
-- The C compiler identification is GNU 13.2.1
-- The CXX compiler identification is GNU 13.2.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Configuring done (9.0s)
-- Generating done (0.0s)
-- Build files have been written to: /tmp/aa/zephyr/samples/hello_world/_AA

Bottom output looks like it's building native using PC host GCC or similar

@tejlmand tejlmand force-pushed the sysbuild/application_config_dir branch from fb7432b to e829d1f Compare May 21, 2024 09:04
@tejlmand
Copy link
Contributor Author

Seems to completely break things, without:

Not broken, but sysbuild itself was looking up a toolchain (which isn't needed because sysbuild itself doesn't compile any code)
So this was only a cosmetic issue, albeit a confusing one.

Notice the lines above which are identifying the correct toolchain used for the image build.

Fixed by restoring LANGUAGES <empty> in sysbuild which somehow had disappeared.

Comment on lines 32 to 33
Copy link
Contributor

Choose a reason for hiding this comment

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

Something is amiss here, inside of hello_world have create a folder with a random name, setting this to the full absolute path works, setting this to just the folder name (relative to hello_world) does not work but continues with the default file, prefixing it with ../ makes sysbuild fail:

CMake Error at /tmp/aa/zephyr/cmake/modules/kconfig.cmake:323 (message):
  File not found: /tmp/aa/zephyr/samples/hello_world/../test/sysbuild.conf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you setting this from inside a CMake file ? (like SysbuildLists.txt or similar)
Or as command line argument ?

APPLICATION_CONFIG_DIR should follow same rules as the x_ROOT folders, that is if specified inside a CMake file, then absolute path should be given.

Especially considering that a relative path set inside an image SysbuildLists.txt and passed to sysbuild and other images as a relative value could suddenly be treated differently by each image.

In CMake, one can easily use ${CMAKE_CURRENT_LIST_DIR}/<path> and avoid ambiguity, and there is no real gain in saving chars in this case.
On command line we support relative paths, and there they are treated relative to the application dir.

https://docs.zephyrproject.org/latest/develop/application/index.html#boards

Note

When specifying BOARD_ROOT in a CMakeLists.txt, then an absolute path must be provided, for example list(APPEND BOARD_ROOT ${CMAKE_CURRENT_SOURCE_DIR}/). When using -DBOARD_ROOT= both absolute and relative paths can be used. Relative paths are treated relatively to the application directory.

However, it seems we have a general issue in sysbuild related to handling of relative paths because when giving a relative root, then it's expanded for images, based on main image location, but not correctly expanded for sysbuild itself.
Short CMake output:

$ west build -p -dbuild -b nrf52840dk/nrf52840 samples/hello_world/ --sysbuild --cmake-only -- -DBOARD_ROOT=foo -DSB_CONFIG_BOOTLOADER_MCUBOOT=y
-- west build: generating a build system
Loading Zephyr module(s) (Zephyr base): sysbuild_default
-- Found Python3: /usr/bin/python3 (found suitable version "3.8.10", minimum required is "3.8") found components: Interpreter 
-- Cache files will be written to: /home/tora/.cache/zephyr
-- Found west (found suitable version "1.2.0", minimum required is "0.14.0")
CMake Warning at /projects/github/ncs/zephyr/cmake/modules/boards.cmake:136 (message):
  BOARD_ROOT element without a 'boards' subdirectory:                                                                                                                                                                                                                                                                         

  /projects/github/ncs/zephyr/share/sysbuild/foo                                                                                                                                                                                                                                                                              

  Hints:                                                                                                                                                                                                                                                                                                                      

    - if your board directory is '/foo/bar/boards/<ARCH>/my_board' then add '/foo/bar' to BOARD_ROOT, not the entire board directory                                                                                                                                                                                          
    - if in doubt, use absolute paths                                                                                                                                                                                                                                                                                         
...
   *****************************
   * Running CMake for mcuboot *
   *****************************

Loading Zephyr default modules (Zephyr base).
-- Application: /projects/github/ncs/bootloader/mcuboot/boot/zephyr
...
CMake Warning at /projects/github/ncs/zephyr/cmake/modules/boards.cmake:136 (message):
  BOARD_ROOT element without a 'boards' subdirectory:

  /projects/github/ncs/zephyr/samples/hello_world/foo

-- 
   *********************************
   * Running CMake for hello_world *
   *********************************

Loading Zephyr default modules (Zephyr base).
-- Application: /projects/github/ncs/zephyr/samples/hello_world
...
CMake Warning at /projects/github/ncs/zephyr/cmake/modules/boards.cmake:136 (message):
  BOARD_ROOT element without a 'boards' subdirectory:

  /projects/github/ncs/zephyr/samples/hello_world/foo

  Hints:

    - if your board directory is '/foo/bar/boards/<ARCH>/my_board' then add '/foo/bar' to BOARD_ROOT, not the entire board directory
    - if in doubt, use absolute paths
Call Stack (most recent call first):
  /projects/github/ncs/zephyr/cmake/modules/zephyr_default.cmake:132 (include)
  /projects/github/ncs/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:66 (include)
  /projects/github/ncs/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:92 (include_boilerplate)
  CMakeLists.txt:7 (find_package)

Copy link
Contributor

Choose a reason for hiding this comment

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

Being set on command line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opened:
APPLICATION_CONFIG_DIR should follow principle of ROOT settings, #73065
Sysbuild treats relative ROOT settings differently in sysbuild vs. image build, #73066

@nordicjm thanks for discovering this issue. As it turns out, it's a more fundamental issue which this PR exposes.

Copy link
Contributor

@57300 57300 left a comment

Choose a reason for hiding this comment

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

I like the core proposal, but I don't like the SysbuildLists.txt suggestion.

  • Having to use absolute paths with add_subdirectory() would be annoying. When trying to use a relative path as one would expect, CMake gives no indication that it's relative to share/sysbuild, so it's hard to debug.
  • I don't think the name is all that self-explanatory. Considering there's also a sysbuild.cmake that could be placed next to a SysbuildLists.txt, it's not obvious which file would be used for what. It reminds me of how a board directory may contain a board.cmake and a CMakeLists.txt and it's not immediately clear why you'd need both.
  • To me, the similarity to CMakeLists.txt also suggests that every sysbuild directory should have a SysbuildLists.txt in it, not just the entry point. Maybe someone would want CMakeLists.txt to only be processed by Zephyr build systems, and SysbuildLists.txt only by sysbuild, which would allow their directory subtrees to overlap (like with CMake and Kconfig), but as of today that's not even possible.
  • If we were using Makefiles in Zephyr, would we want to call this a Sysbuildfile?

I would much prefer the sysbuild/CMakeLists.txt alternative. This path already shows up in Zephyr modules, so using it in samples wouldn't be as much of a stretch. Of course it has to be documented either way.

One thing I like here is how the Sysbuild package minimizes the role of share/sysbuild/CMakeLists.txt. I think it would be interesting to take it further in the future, by removing the need to go through that file when $APP_DIR/sysbuild/CMakeLists.txt exists, so that this could be used as the sole entry point. Then, sysbuild could be configured by running either:

cmake $ZEPHYR_BASE/share/sysbuild -DAPP_DIR=$PWD ... # current, works on any app
cmake sysbuild ...                                   # new, optional shortcut

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not SB_APPLICATION_CONFIG_DIR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point.

Will adjust.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@tejlmand
Copy link
Contributor Author

tejlmand commented May 22, 2024

I like the core proposal, but I don't like the SysbuildLists.txt suggestion.

@57300 Thanks a lot for the feedback.
You raise some the same concerns I have, although I ended with a different preference.
But i'm open to good arguments. I see pros / cons in both approaches.

  • Having to use absolute paths with add_subdirectory() would be annoying. When trying to use a relative path as one would expect, CMake gives no indication that it's relative to share/sysbuild, so it's hard to debug.

I completely agree, and that's why I want to extend the sysbuild_add_subdirectory() to be able to handle this case.
This means that sysbuild_add_subdirectory() shall be able to take a relative path and adjust it to APP_DIR, as well as construct the proper bin-dir name, so that user can just do sysbuild_add_subdirectory(<rel-path>) (instead of add_subdirectory(<abs-path> <bin-dir>)).

But a little care must be taken to not introduce unwanted surprises, as well proper behavior on all levels.
Hence the reason for leaving this extension to sysbuild_add_subdirectory() out of the initial PR in order to keep focus purely on the overall principle and be able to discuss and decide on the principler and the naming.

  • I don't think the name is all that self-explanatory. Considering there's also a sysbuild.cmake that could be placed next to a SysbuildLists.txt, it's not obvious which file would be used for what. It reminds me of how a board directory may contain a board.cmake and a CMakeLists.txt and it's not immediately clear why you'd need both.

But I don't see how the set {sysbuild/CMakeLists.txt, sysbuild.cmake} vs the set {SysbuildLists.txt, sysbuild.cmake} changes that.
True, they are not on same level, but the fact that sysbuild/CMakeLists.txt is one level below sysbuild.cmake could make it less intuitive that sysbuild/CMakeLists.txt is actually the entry point, and not sysbuild.cmake.
So far, I don't see the comment in favor of one or the other solution.
There are two files, and we must clearly describe the difference between those two and when to use which.
Regardless of the solution it may not be intuitive to users when to use which one.

  • To me, the similarity to CMakeLists.txt also suggests that every sysbuild directory should have a SysbuildLists.txt in it, not just the entry point. Maybe someone would want CMakeLists.txt to only be processed by Zephyr build systems, and SysbuildLists.txt only by sysbuild, which would allow their directory subtrees to overlap (like with CMake and Kconfig), but as of today that's not even possible.

This relates partly to the above reply of sysbuild_add_subdirectory(). We do have the sysbuild_add_subdirectory() which could allow us to prefer SysbuildLists.txt if both SysbuildLists.txt and a CMakeLists.txt exists, and thereby allow directory subtrees to overlap. I initially didn't intend to open this discussion, but if it helps sketch the future direction and helps deciding a direction, then it could be a valuable discussion to have early.

  • If we were using Makefiles in Zephyr, would we want to call this a Sysbuildfile?

We would probably name the file Makefile.sysbuild and allow users to invoke using make -f Makefile.sysbuild.
Unfortunately CMake doesn't allow us to substitute the name of CMakeLists.txt in similar way as make does 😞

Note, if it purely on the naming, then CMakeLists.sysbuild is also an option, but still suffers the limitation of SysbuildLists.txt.

I think it would be interesting to take it further in the future, by removing the need to go through that file when $APP_DIR/sysbuild/CMakeLists.txt exists, so that this could be used as the sole entry point. Then, sysbuild could be configured by running either:

This is a very good use-case, and could be one which would make the sysbuild/CMakeLists.txt having an edge over the current proposal. But it would mean the sysbuild package would have to be properly exported (if wanting to use the shortcut), but I see that as a minor detail.


Extra note, although sysbuild today mainly targets a single board with one or more SoCs, each having one or more CPU clusters, then there is also the use case of building for multiple boards, as discussed: #40555 (comment)

For example one could create a project utilizing SYSBuild to build an audio system consisting of a BLE audio gateway and a headset, each built and flashed to different devices.
In such a case, SYSBuild is building and flashing for a system, not just a multi-core SoC.

In the above case, one might create a pure sysbuild project having a SysbuildLists.txt file, but no CMakeLists.txt file because that project cannot be built as a regular Zephyr project (it has no sources) becuase the project is just collect and configuring other projects for a special use-case. In this case, having to create a <highlevel-app>/sysbuild/CMakeLists.txt may not be as clean as just having a <highlevel-app>/SysbuildLists.txt file.

@nordicjm
Copy link
Contributor

I agree with @57300 in that using a standard filename is better <app_dir>/sysbuild/CMakeLists.txt as opposed to SysbuildLists.txt

@57300
Copy link
Contributor

57300 commented May 22, 2024

  • I don't think the name is all that self-explanatory. Considering there's also a sysbuild.cmake that could be placed next to a SysbuildLists.txt, it's not obvious which file would be used for what. It reminds me of how a board directory may contain a board.cmake and a CMakeLists.txt and it's not immediately clear why you'd need both.

But I don't see how the set {sysbuild/CMakeLists.txt, sysbuild.cmake} vs the set {SysbuildLists.txt, sysbuild.cmake} changes that. True, they are not on same level, but the fact that sysbuild/CMakeLists.txt is one level below sysbuild.cmake could make it less intuitive that sysbuild/CMakeLists.txt is actually the entry point, and not sysbuild.cmake. So far, I don't see the comment in favor of one or the other solution. There are two files, and we must clearly describe the difference between those two and when to use which. Regardless of the solution it may not be intuitive to users when to use which one.

I agree that neither option is truly intuitive and that users will likely have to refer to examples or docs either way. My point is just that I don't believe SysbuildLists.txt (or similar) has a significant advantage in this regard, even though that was the motivation for it as you stated initially. So, you're right in that this comment was not in favor of either solution.

Extra note, although sysbuild today mainly targets a single board with one or more SoCs, each having one or more CPU clusters, then there is also the use case of building for multiple boards, as discussed: #40555 (comment)

(...)

In the above case, one might create a pure sysbuild project having a SysbuildLists.txt file, but no CMakeLists.txt file because that project cannot be built as a regular Zephyr project (it has no sources) becuase the project is just collect and configuring other projects for a special use-case. In this case, having to create a <highlevel-app>/sysbuild/CMakeLists.txt may not be as clean as just having a <highlevel-app>/SysbuildLists.txt file.

For this use case, I think the ideal solution would be to support building without a DEFAULT_IMAGE, so that a dummy CMakeLists.txt could indeed be dropped when not needed. Then, it should be possible to move sysbuild/CMakeLists.txt in its place (one level up) so that we don't depend on this subdirectory for pure sysbuild samples.

All in all, I'm still more interested in the sysbuild/CMakeLists.txt option, but I can address the sysbuild_add_subdirectory() discussion later.

@tejlmand
Copy link
Contributor Author

My point is just that I don't believe SysbuildLists.txt (or similar) has a significant advantage in this regard, even though that was the motivation for it as you stated initially.

The motivation was between the application dir Zephyr CMakeLists.txt and SysbuildLists.txt.
Having both at toplevel application dir makes it more visible that it's an entry point.
The naming difference between those clearly indicates which is for Zephyr CMake build and which is for sysbuild.

Not related to the file sysbuild.cmake and SysbuildLists.txt.

Then, it should be possible to move sysbuild/CMakeLists.txt in its place (one level up) so that we don't depend on this subdirectory for pure sysbuild samples.

I could fear that could result in other types of confusion, because in this case you cannot determine by looking at the toplevel folder whether it's a regular Zephyr sample or a dedicated highlevel sysbuild sample.

So even in such a case, then I would prefer to have the sysbuild/CMakeLists.txt for consistency.

@tejlmand
Copy link
Contributor Author

by @nordicjm

I agree with @57300 in that using a standard filename is better <app_dir>/sysbuild/CMakeLists.txt as opposed to SysbuildLists.txt

by @57300

All in all, I'm still more interested in the sysbuild/CMakeLists.txt option, but I can address the sysbuild_add_subdirectory() discussion later.

will switch to sysbuild/CMakeLists.txt, as I am not against any of the options, just had to open PR with one of them, and there I had a slight preference for a file with a dedicated name related to sysbuild.

@tejlmand tejlmand force-pushed the sysbuild/application_config_dir branch from e829d1f to 3d5b456 Compare May 23, 2024 12:53
@tejlmand
Copy link
Contributor Author

will switch to sysbuild/CMakeLists.txt

@nordicjm + @57300 done

tejlmand added 3 commits May 24, 2024 14:52
Use of ARGV1 is undefined when number of arguments to function is less
than 2. Therefore switch to ARGN which holds arguments beyond required
arguments.
If there are no optional arguments, then ARGN is just an empty list,
thus making it safe to use.

Signed-off-by: Torsten Rasmussen <[email protected]>
This commit refactors sysbuild entry code by creating a CMake sysbuild
module for image processing and place sysbuild entry code in a
<app>/sysbuild/CMakeLists.txt file.

A template/CMakeLists.txt file will be use as template for applications
which doesn't provide their own entry file.

An application may create a sysbuild/CMakeLists.txt file.
The sysbuild/CMakeLists.txt file is similar in nature to the
toplevel CMakeLists.txt file but intended to used by sysbuild.
This allows application developers to adjust how an application is
built with sysbuild.

Signed-off-by: Torsten Rasmussen <[email protected]>
APPLICATION_CONFIG_DIR is supported in Zephyr and allows to adjust the
location from which prj.conf and friends are picked up.

This also works for images when using sysbuild, however sysbuild itself
ignores the value of APPLICATION_CONFIG_DIR, meaning that sysbuild only
accepts sysbuild.conf located directly in the sample folder.

Extend sysbuild to support APPLICATION_CONFIG_DIR so sysbuild follows
regular Zephyr CMake behavior.

Introduce SB_APPLICATION_CONFIG_DIR to allow changing the location
for sysbuild only, without propagating the value to images.

Signed-off-by: Torsten Rasmussen <[email protected]>
@tejlmand tejlmand force-pushed the sysbuild/application_config_dir branch from 3d5b456 to 8093a4e Compare May 27, 2024 07:59
@carlescufi carlescufi requested a review from 57300 May 27, 2024 10:01
@carlescufi carlescufi requested a review from nordicjm May 27, 2024 10:02
@carlescufi carlescufi changed the title sysbuild: support SysbuildLists.txt as entry point and support APPLICATION_CONFIG_DIR sysbuild: support sysbuild/CMakeLists.txt as entry point and support APPLICATION_CONFIG_DIR May 27, 2024
Copy link
Contributor

@57300 57300 left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

Copy link
Member

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

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

I like the new sysbuild/CMakeLists.txt entry point name.

@fabiobaltieri fabiobaltieri merged commit 2b427f1 into zephyrproject-rtos:main May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants