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

CMake integration #31193

Closed
KKoovalsky opened this issue Jan 8, 2021 · 13 comments
Closed

CMake integration #31193

KKoovalsky opened this issue Jan 8, 2021 · 13 comments
Assignees
Labels
area: Build System Enhancement Changes/Updates/Additions to existing features

Comments

@KKoovalsky
Copy link

Hello!

I wanted to start work with Zephyr as my main RTOS but unfortunately I abandon my efforts.

The main problem is that I have a current project whose build system is based on CMake and it works pretty well. I wanted to switch from another RTOS ecosystem to Zephyr since it's also built on top of CMake and its driver list is quite long.

The problem is that Zephyr doesn't work well with CMake. Let me list few problems which made me turn Zephyr down.

Order of the find_package and project calls is mandatory. Normally find_package(Zephyr) must be called before project(...) command. Zephyr sets up all the toolchain related paths and this is why this order is needed, among others, I guess. Why not rely on CMake's toolchain files functionality and let user choose the toolchain on his own? Zephyr could provide toolchain files on his own that set the initial C flags and others. I think that it could simplify the build system complexity. The compiler flags and all the toolchain related boilerplate could be defined as cmake targets. This also makes the project really application-centric. In this form the coupling to the Zephyr project is quite high unfortunately.

When defining multiple static library targets for which I would like to have proper compiler flags provided, I can't really do it from CMake, because Zephyr doesn't define any target that has compiler flags attached to it. Instead COMPILER_OPT is used, which is a string that is appended to the compiler invocation.

If you really want to be application-centric don't define executable targets (like the app) inside the Zephyr CMake module. It is preferred to define library or interface targets, so the user could link to it easily.

I couldn't use target_compile_features. When I used set_target_properties(... CXX_STANDARD ... as a workaround the flags has been ignored by the setup. This means that I can't rely on any CMake feature because I am not really sure whether I have to enable the same function from the menuconfig.

My suggestion is to rework the build system step by step.

@KKoovalsky KKoovalsky added the Enhancement Changes/Updates/Additions to existing features label Jan 8, 2021
@yashi
Copy link
Collaborator

yashi commented Sep 29, 2021

(partially related to #8827 #36558)

check_include_file() doesn't work in module's CMakeLists.txt either. because module it doesn't know where to look for, and it wrongly find build machine's header file. it can be prevented by doing

set(CMAKE_SYSROOT ${SYSROOT_DIR})

But still we need to set CMAKE_REQUIRED_INCLUDES and others to make check_include_file() work properly.

openthread module does something like this

target_include_directories(ot-config INTERFACE
    $<TARGET_PROPERTY:zephyr_interface,INTERFACE_INCLUDE_DIRECTORIES>)
target_include_directories(ot-config SYSTEM INTERFACE
    $<TARGET_PROPERTY:zephyr_interface,INTERFACE_SYSTEM_INCLUDE_DIRECTORIES>)
target_compile_definitions(ot-config INTERFACE
    $<TARGET_PROPERTY:zephyr_interface,INTERFACE_COMPILE_DEFINITIONS>)
target_compile_options(ot-config INTERFACE
    $<TARGET_PROPERTY:zephyr_interface,INTERFACE_COMPILE_OPTIONS> -fno-builtin)

This allows us to build module code with Zephyr's compiler options.

Unfortunately, check_include_file() needs CMAKE_REQUIRED_INCLUDES to be set or flags given as additional param.

@KKoovalsky
Copy link
Author

I would like to contribute with reworking the CMake code, but firstly I would like to understand what is the rationale behind the current implementation.

Could you answer those questions or point me to the documentation where I could find answers to those questions?

  1. Why the order of calls:
find_package(Zephyr)
project(myApp)

is necessary?

  1. Why Zephyr CMake scripts perform complex actions to mutate CC, CXX variables to use the Zephyr's toolchain?
  2. Could CMake toolchain files be used instead? Could Zephyr rely on CMAKE_TOOLCHAIN_FILE?

@KKoovalsky
Copy link
Author

@tejlmand Could you support?

@tejlmand
Copy link
Collaborator

tejlmand commented Nov 3, 2021

Why the order of calls:

Please find some explanation here:
https://github.com/zephyrproject-rtos/zephyr/blob/main/doc/application/index.rst#creating-an-application

Why Zephyr CMake scripts perform complex actions to mutate CC, CXX variables to use the Zephyr's toolchain?

This has to do with the old migration from KBuild as @SebastianBoe has described but also the way Zephyr handles the compilers.
Zephyr will test the compiler for certain features and then cache the result with a unique compiler id in a user cache shared among all samples being built with that toolchain.
CMake can also test the compiler but the result of a test will only be stored in the build cache of a single sample to avoid re-testing on subsequent CMake re-runs of that sample. Thus building n-samples will test the compiler n-times

The Zephyr approach improves the speed significantly when building multiple samples with the same toolchain, so that building n-samples will only test the compiler 1 time.

Could CMake toolchain files be used instead? Could Zephyr rely on CMAKE_TOOLCHAIN_FILE?

There has been requests to support CMAKE_TOOLCHAIN_FILE also, and I do think it would make sense to have support for this in Zephyr. But I don't see Zephyr starting to provide CMake toolchain files this way.
One reason for that is the whole Kconfig / SoC / Boards infrastructure where the selection of a board determines the arch in use.
Requesting users to also point out a cross compiling toolchain in addition, with the risk of mismatching to rest of the system needs to be considered.

However, this should not prevent us from allow users with special wishes to specify CMAKE_TOOLCHAIN_FILE if they so like.
We just need to consider how such integration can happen safely and how it can be ensured in CI that such feature doesn't break later.

Also I have often seen developers putting compile flags in their toolchain files that are not essential for the toolchain / compiler itself, biut because it's convenient, such as release build flags, debug flags, etc.
Also some developers consider hardware flags as something to put into their toolchain file.
In Zephyr, all such build flags are controlled using Kconfig, so care must also be taken to ensure that it is clear what settings that will take precedence.

The short description above is part of the reason that other features have had higher priority until now.

@SebastianBoe
Copy link
Collaborator

Zephyr used CMAKE_TOOLCHAIN_FILE at some point.

But the toolchain file must be specified each time on the command line, and to migrate to CMake we needed to be competitive with KBuild, which can be invoked as simply as "make".

So we dropped support for it.

I have no regrets about this decision.

@tejlmand
Copy link
Collaborator

tejlmand commented Nov 3, 2021

But the toolchain file must be specified each time on the command line, and to migrate to CMake we needed to be competitive with KBuild, which can be invoked as simply as "make".

Since CMake 3.21 it can be specified as environment variable:
https://cmake.org/cmake/help/v3.21/envvar/CMAKE_TOOLCHAIN_FILE.html

Also with west, you may specify permanent CMake arguments, like:
west config build.cmake-args -- -DCMAKE_TOOLCHAIN_FILE=<file>
https://docs.zephyrproject.org/latest/guides/west/build-flash-debug.html#permanent-cmake-arguments

and thus avoiding to type it on each new CMake invocation.

I have no regrets about this decision.

me neither, but if someone finds it useful and valuable, as well as having the bandwidth to introduce it in a way that is compatible with existing Zephyr toolchain infrastructure, then I have no objections to such contribution.

@tejlmand
Copy link
Collaborator

tejlmand commented Nov 3, 2021

@KKoovalsky I just noticed your initial comment, so let me reply to some of your questions.

When defining multiple static library targets for which I would like to have proper compiler flags provided, I can't really do it from CMake, because Zephyr doesn't define any target that has compiler flags attached to it. Instead COMPILER_OPT is used, which is a string that is appended to the compiler invocation.

COMPILER_OPT is because this is a Kconfig configuration option that even allows you to append any compiler setting not already covered by Zephyr configuration system.

You should be able to find most commonly used flags that you require covered but as different compilers uses different flags for different compilers, there is a template for matching properties to flags, for example the template here:
https://github.com/zephyrproject-rtos/zephyr/blob/main/cmake/compiler/compiler_flags_template.cmake
and then the gcc mappings:
https://github.com/zephyrproject-rtos/zephyr/blob/main/cmake/compiler/gcc/compiler_flags.cmake

This ensures that you can enable a single CONFIG_SECURITY_CANARIES in Kconfig and have it correctly mapped to the right set of choices for the toolchain in use:

zephyr/CMakeLists.txt

Lines 110 to 111 in d5aa5c2

if(CONFIG_STACK_CANARIES)
zephyr_compile_options($<TARGET_PROPERTY:compiler,security_canaries>)

mapping to:

set_compiler_property(PROPERTY security_canaries -fstack-protector-all)
# Only a valid option with GCC 7.x and above, so let's do check and set.
check_set_compiler_property(APPEND PROPERTY security_canaries -mstack-protector-guard=global)

or
set_compiler_property(PROPERTY security_canaries -fstack-protector-all)

depending on compiler in use and it's know capabilities (or tested capabilities if some versions of compiler might support a flag).

But of course not everything is covered.

If you want to do things purely in CMake and avoid Kconfig then you can take a look at the zephyr_interface interface library. I assume it's something like that you're looking for.

I couldn't use target_compile_features.

This has been addressed here:
#39014

@SebastianBoe
Copy link
Collaborator

I know you know this @tejlmand, but for others I would like to clarify that neither
west nor CMake 3.21 existed at that time.

@tejlmand
Copy link
Collaborator

tejlmand commented Nov 4, 2021

but for others I would like to clarify that neither west nor CMake 3.21 existed at that time.

absolutely correct.
I'm simply mentioning the new possibilities because if supporting CMake toolchain files would benefit the project overall, for example with less maintenance work, better toolchain support, easier integration, etc. then this single reason has been addressed externally.

But so far I still think the solution introduced with the KBuild re-write is superior as I describe in other comments.

@boaks
Copy link

boaks commented May 25, 2022

I'm struggling a little at an similar issue for an module. Applying yashi/module-sample works well, but AutoConf.cmake fails ins some case, e.g. adding "check_include_file(net/socket.h HAVE_NET_SOCKET_H)" doesn't work.

Determining if the include file sys/socket.h exists failed with the following output:
Change Dir: repos/zephyr/coaps-zephyr/coaps-zephyr/build_nrf9160dk_nrf9160_ns/CMakeFiles/CMakeTmp

Run Build Command(s):/usr/bin/ninja cmTC_40463 && [1/2] Building C object CMakeFiles/cmTC_40463.dir/CheckIncludeFile.c.obj
FAILED: CMakeFiles/cmTC_40463.dir/CheckIncludeFile.c.obj
zephyr-sdk/arm-zephyr-eabi/bin/arm-zephyr-eabi-gcc -nostartfiles -nostdlib -isystem "zephyr-sdk/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/10.3.0/include" -isystem "zephyr-sdk/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/10.3.0/include-fixed" -Wl,--unresolved-symbols=ignore-in-object-files -Wl,--entry=0 -o CMakeFiles/cmTC_40463.dir/CheckIncludeFile.c.obj -c repos/zephyr/coaps-zephyr/coaps-zephyr/build_nrf9160dk_nrf9160_ns/CMakeFiles/CMakeTmp/CheckIncludeFile.c
repos/zephyr/coaps-zephyr/coaps-zephyr/build_nrf9160dk_nrf9160_ns/CMakeFiles/CMakeTmp/CheckIncludeFile.c:1:10: fatal error: sys/socket.h: No such file or directory
1 | #include <sys/socket.h>
| ^~~~~~~~~~~~~~
compilation terminated.
ninja: build stopped: subcommand failed.

Adding:

find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})

results just in a different error:

CMake Error at repos/zephyr/coaps-zephyr/zephyr/cmake/kconfig.cmake:295 (add_custom_target):
add_custom_target cannot create target "config-twister" because another
target with the same name already exists. The existing target is a custom
target created in source directory
"repos/zephyr/coaps-zephyr/coaps-zephyr". See documentation
for policy CMP0002 for more details.
Call Stack (most recent call first):
repos/zephyr/coaps-zephyr/zephyr/cmake/app/boilerplate.cmake:544 (include)
repos/zephyr/coaps-zephyr/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:24 (include)
repos/zephyr/coaps-zephyr/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:40 (include_boilerplate)
repos/zephyr/coaps-zephyr/modules/lib/tinydtls/zephyr/CMakeLists.txt:24 (find_package)

In my case, it's only one header, so I fixed it. But it would be nice, if that works as well for modules.

@zephyrbot
Copy link
Collaborator

Hi @tejlmand,

This issue, marked as an Enhancement, was opened a while ago and did not get any traction. Please confirm the issue is correctly assigned and re-assign it otherwise.

Please take a moment to review if the issue is still relevant to the project. If it is, please provide feedback and direction on how to move forward. If it is not, has already been addressed, is a duplicate, or is no longer relevant, please close it with a short comment explaining the reason.

@KKoovalsky you are also encouraged to help moving this issue forward by providing additional information and confirming this request/issue is still relevant to you.

Thanks!

@KKoovalsky
Copy link
Author

Hey!

I guess after such time the initial problem may be fixed with a different approach, or by adapting to the zephyr's build system requirements. :)

I haven't also used Zephyr for a while now, so I can't really give any input here. ;)

Thanks for the support anyway!

@marc-hb marc-hb closed this as completed Feb 20, 2024
@canatella
Copy link

This would still be a nice to have regarding integration with Conan (see #31745) and probably other projects. I resorted to copy paste the compile flags used by Zephyr in a Conan profile where I could just have pointed to the toolchain file if it was there. Then I discovered that find_zephyr does it self end up calling project which created some recursive error with cmake if I included it in any subproject.

I understand where you're coming from, but when we recently started our new project, we thought "cool, Zephyr is CMake based, it'll be easy to integrate with our current code". Then I was disappointed to see it's yet another project working around CMake in its own unique, incompatibles ways. Reading the few issues around this here makes it clear there are actual reasons about this, and I understand that there's a lot of cmake code that would need to evolve, but it would be nice if there were some plan to move toward a more standard way of doing things.

At least providing CMAKE_TOOLCHAIN_FILE support to enable compilation in other projects. No need for flashing or linking, but at the very least this would provide an easy way to build a static library to be integrated in the project later, without having to make it a Zephyr specific project so that if you're using conan, vcpkg or any other wrapper you can just drop -DCMAKE_TOOLCHAIN_FILE=zephyr/my-board.cmake and be good to go.

And then maybe by merging some features upstream? The global compiler flag cache is a nice thing and could maybe be useful to other projects. For the compiler features, maybe using try_run and try_compile with cmake -C common-cache.txt to populate the cache upfront could be a more standard way to do that? There's now preset files which could also be used to specify board specific variables.

Just my 2 cents here. All in all, it's working. I'm just sad that in 2024, it's still complicated to integrate two unrelated C++ projects even though they both use CMake.

Best wishes for 2025 ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Build System Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

No branches or pull requests

9 participants