Intro
As mentioned in #13974, I'm in the process of moving our current project from USCRPL/mbed-cmake to mbed os first-party CMake.
I thought it might be interesting for others to share our progress, our questions, feedback and issues we might be facing. I think that github is better than the forum for this as it is easier to share code, other repos and reference issues and PR
Our project is quite complex, with a main src directory containing our product firmware (main program). We have different drivers and libraries that are being used by the firmware.
We also have a lot of spikes: standalone projects (with their own main.cpp) to test components, features, libs, technical solutions, etc.
The following repository is a simpler example of our big project, and I'll use it to test all the features.
How it works now
USCRPL/mbed-cmake allows us to have the spikes and the main program live in the same repository and compile together without any issues. We then use openocd to flash the .bin we want to use in our product.
USCRPL/mbed-cmake first creates a mbed-os-static STATIC library (https://github.com/USCRPL/mbed-cmake/blob/c0b0f7d4080bba179b9390e877eb0c1e7467f6d0/cmake/MbedOS-GCCArm.cmake#L35-L37) which is then wrapped into an mbed-os INTERFACE that defined the linker script and link options (https://github.com/USCRPL/mbed-cmake/blob/c0b0f7d4080bba179b9390e877eb0c1e7467f6d0/cmake/MbedOS-GCCArm.cmake#L50-L58). It's this mbed-os INTERFACE that our main program, spikes drivers and libs link with.
From a user perspective, mbed-os-static is compiled once for the main program and for all the spikes, drivers and libs. Adding a new spike with just a main.cpp file, will only increment the whole compilation steps number by one.
First try with Mbed OS first-party CMake
On the other hand, mbed os first-party cmake "recompiles" all the needed sources for each target depending on it. Adding a new spike linking with mbed-os will throw in the compilation a few hundred more "steps". Looking at the output, one can clearly see that mbed os or target files are compiled multiple times for the libs, the spikes and the main program.
Issues
And then it stops as the way it handles the linker script is not made for multiple add_executable targets.
Another issue was that adding another executable and calling mbed_set_mbed_target_linker_script resulted in the following error about .compile_time_defs.txt:
CMake Error: Files to be generated by multiple different commands: "/Users/ladislas/dev/ladislas/mbed-cmake-template/build/compile_time_defs.txt"
The issue comes from here: https://github.com/ARMmbed/mbed-os/blob/33a7e66a0794c91fddc19f9217a81910d5f4a23f/tools/cmake/toolchain.cmake#L22-L23 and is easily fixed by using:
- file(GENERATE OUTPUT "${CMAKE_BINARY_DIR}/compile_time_defs.txt" CONTENT "${_compile_definitions}\n")
- set(${definitions_file} @${CMAKE_BINARY_DIR}/compile_time_defs.txt PARENT_SCOPE)
+ file(GENERATE OUTPUT "${CMAKE_BINARY_DIR}/${target}.compile_time_defs.txt" CONTENT "${_compile_definitions}\n")
+ set(${definitions_file} @${CMAKE_BINARY_DIR}/${target}.compile_time_defs.txt PARENT_SCOPE)
Note that the two files generated are identical. If they can never be different, it would be better to juste generate one and use it for all executable.
Questions
Please note I'm neither a CMake expert nor a .ld expert.
From my understanding: for the same mbed target (DISCO_F769NI in our case), the linker script is always the same. If we change target, we must change the linker script. In real life projects this happens seldom as your board/target is usually the same for a long period of time. But even if we need to change it, a new run of mbed-tools configure will provide everything needed.
Currently mbed-os, mbed-core, mbed-rtos are defined as INTERFACE libraries, meaning they are not compiled on their own (as static or object libraries would be), but become part of the target (other library or executable) that links with it.
So calling mbed_set_mbed_target_linker_script for each executable will also call mbed_set_toolchain_options multiple times.
My first question is:
What is the reason for using
INTERFACEfor the whole mbed-os? Why not use a static library wrapped in an interface?
And secondly:
Is supporting a project structure such as mine with multiple
executabletargets on the roadmap?
I'd be happy to help in this case.
Thanks for reading till here and also thanks for bringing CMake to mbed-os! :)
(cc @0xc0170 @hugueskamba @rajkan01 @multiplemonomials @ProExpertProg)
DISCO_F769NI
arm-none-eabi-gcc --version
arm-none-eabi-gcc (GNU Arm Embedded Toolchain 9-2020-q2-update) 9.3.1 20200408 (release)
33a7e66a0794c91fddc19f9217a81910d5f4a23f
mbed --version
1.10.4
mbed-tools --version
3.5.1.dev36
to do
I haven't had a chance to poke around the official CMake scripts that much, but I do agree that building the linker script multiple times seems like the wrong way to do it since it doesn't change. It would be better to just generate it once and then attach it as part of a target's link flags (how mbed-cmake does it).
Also, it definitely doesn't seem right if the entire source code is getting built once for each target. My understanding is that the mbed source code is supposed to get built as an object library, then the objects are supposed to get attached using TARGET_SOURCES to an interface library. But it sounds like maybe the source files themselves got added as the interface sources, rather than the object files.
@ladislas thank you for raising this issue.Please take a look at the following comments:
We cannot automatically identify a release based on the version of Mbed OS that you have provided.
Please provide either a single valid sha of the form #abcde12 or #3b8265d70af32261311a06e423ca33434d8d80de
or a single valid release tag of the form mbed-os-x.y.z .
E.g. 'master' is not a unique single snapshot.
NOTE: If there are fields which are not applicable then please just add 'n/a' or 'None'.This indicates to us that at least all the fields have been considered.
Please update the issue header with the missing information, the issue will not be mirroredto our internal defect tracking system or investigated until this has been fully resolved.
Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers.
Internal Jira reference: https://jira.arm.com/browse/IOTOSM-2888
My understanding is that the mbed source code is supposed to get built as an object library, then the objects are supposed to get attached using TARGET_SOURCES to an interface library.
@multiplemonomials it was since https://github.com/ARMmbed/mbed-os/commit/e7c0d93ad4b2eee09a16d22d91452ee3eaad2185 that changed it from OBJECT to INTERFACE.
Would OBJECT be better than STATIC libraries?
Yeah that commit is... not right, or at least it's not correct to the design we originally planned (I worked with Martin and Hugues to plan out how the object and interface libraries would be used). The interface libraries do need to be there, _but_ there also need to be object libraries as well. The object libraries would be used to compile the sources, then the compiled objects from those libraries should get added via target_sources to the interface libraries.
Thanks for the feedback, we will review and respond to all issues/questions.
@ladislas can we convert example blinky to be able to reproduce the issue with spikes? I've checked your template repository, it does not reference mbedtools, so I might need an assistance to reproduce it there (how to).
Is supporting a project structure such as mine with multiple executable targets on the roadmap?
I would say it should work as CMake libraries should be reusable. We are currently looking at testing and it might be the same use case.
2 or more applications
2 spikes as you call, just 2 different applications using the same mbed-os library ? I've created a branch with 2 main object libraries to test with https://github.com/0xc0170/mbed-os/tree/cmake-fix-objects - can you test ?
linker issue
we will review if we can fix the linker not to be dependent on application target (it's related to the first one so we might want to first fix that and then get to this one).
We initially worked only on object libraries but we faced cmake issues with object libraries (we should be able to find PRs/commits) and converted to interface libraries. We exposed the main libraries mbed-os and mbed-baremetal as interface as well - it has some drawbacks as mentioned here, we should be able to address them. The branch I shared above was a concept we had previously.
From my understanding: for the same mbed target (DISCO_F769NI in our case), the linker script is always the same. If we change target, we must change the linker script. In real life projects this happens seldom as your board/target is usually the same for a long period of time. But even if we need to change it, a new run of mbed-tools configure will provide everything needed.
Consider ld file being a template that is changed based on app needs (sections could be adjusted, an example is bootloader). Therefore we preprocess linker scripts. If you change an app, linker script must be regenerated - reusing the same template. We always wanted to have one .ld template and fill it in.
Also for linker script file, if you can provide us a repo where we can reproduce, we will review in more detail.
can we convert example blinky to be able to reproduce the issue with spikes? I've checked your template repository, it does not reference mbedtools, so I might need an assistance to reproduce it there (how to).
@0xc0170 yes, I'll work on a PR! I'll also push to my template on a specific branch for testing.
I'll also test your branch and let you know.
And thanks for the explanation on the linker file. 馃憤
Okay, I've got time to take a look into what's going on here.
I think I found the source of some of these errors, but before that, I got distracted by a giant, horrible, flashing red flag:
Detecting C Compiler ABI -- Failed
This is the same issue that we talked about at our meeting, and it seems that it still hasn't been fixed. Basically, the compiler flags that CMake is using to do all of its internal tests are extremely messed up, and you guys just papered over the issue with:
set(CMAKE_C_COMPILER_WORKS TRUE)
set(CMAKE_CXX_COMPILER_WORKS TRUE)
This is a really bad idea, because it means that all sorts of things will be subtly screwed up later on, since CMake cannot successfully run compilation or linking tests.
The real way to fix it is to add the needed compiler flags to CMake's global flag variables, like I explained. So, I made a PR to fix this: https://github.com/ARMmbed/mbed-os/pull/13987 . Please take a look when you have a chance!
@ladislas
When you submit your PR, this also needs to get fixed in mbed-os/CMakeLists.txt:
- COMMAND "arm-none-eabi-cpp" ${_linker_preprocess_definitions} -x assembler-with-cpp -E -Wp,-P
+ COMMAND ${CMAKE_CXX_COMPILER} ${_linker_preprocess_definitions} -x assembler-with-cpp -E -Wp,-P
The existing code requires arm-none-eabi-cpp to be on the PATH, which is not guaranteed. Instead, it's better to use the compiler that we already know the path to. The options -x assembler-with-cpp -E are enough to convert the compiler into a preprocessor.
Update: I understand a little better why the current method was used (compiling all mbed sources for each target). The issue is that you have a set of base Mbed sources, mbed-core. You also have a number of extra modules, like mbed-rtos. However, these extra modules can add extra compile definitions (such as MBED_CONF_RTOS_PRESENT) that change the behavior of source files in mbed-core. This means that you can theoretically end up with a different product in mbed-core depending on what extra modules you link.
IMO, this is somewhat bad code design to have things tightly coupled like this, and it's hard to fix at the build system level. However, I also don't think it's a good solution to require rebuilding all mbed sources for each target. For now, I'm going to try at least making mbed-baremetal and mbed-rtos separate libraries to fix that dependency, but I'm not sure how many extra modules have this behavior.
OK! I think I figured out a method that works to fix this.
@ladislas Check my branch here: https://github.com/multiplemonomials/mbed-os/tree/cmake-object-libs
It should fix your issue, I've converted mbed-os, mbed-baremetal, and mbed-storage-kvstore to the new system so they should only get built once regardless of how many targets they're used in.
I realize that the solution I told you guys originally doesn't actually work given the constraints above. However, I figured out something that should work for what you need.
For the main OS targets, I used this strategy:
mbed-base-flags and mbed-rtos-flags. All flags (options, include dirs, definitions) that used to get applied to mbed-core and mbed-rtos now get put in these interface targets.mbed-baremetal-obj and mbed-os-obj. All mbed-core sources get added to mbed-baremetal-obj to be compiled. mbed-os-obj receives the RTOS sources as well as the core sources.mbed-baremetal and mbed-os to wrap the object libraries.For extra modules, I used this strategy:
add_library(mbed-xxxxx INTERFACE) into add_library(mbed-xxxxx OBJECT EXCLUDE_FROM_ALL). EXCLUDE_FROM_ALL, of course, prevents the library from being built unless something actually depends on it.mbed-xxxx links to either mbed-base-flags or something that doesNote that this process makes some assumptions:
Let me know what you think about this plan -- I'm hoping that it can make life easier for us users. But if it seems totally unworkable, let me know as I have another idea that might also be feasible.
Thanks @multiplemonomials , we will review but most likely the next week (some of us are out of office in the next days)
I run initial compiler checks to see what flags and files get into the app. I'll look at the structure the next week.
Compiling K64F for GCC ARM shows startup is duplicated:
arm-none-eabi/bin/ld.exe: CMakeFiles/mbed-os-example-blinky.dir/mbed-os/targets/TARGET_Freescale/TARGET_MCUXpresso_MCUS/TARGET_MCU_K64F/device/TOOLCHAIN_GCC_ARM/startup_MK64F12.S.obj: in function `__isr_vector': (.isr_vector+0x0): multiple definition of `__isr_vector'; mbed-os/CMakeFiles/mbed-os-obj.dir/targets/TARGET_Freescale/TARGET_MCUXpresso_MCUS/TARGET_MCU_K64F/device/TOOLCHAIN_GCC_ARM/startup_MK64F12.S.obj:(.isr_vector+0x0): first defined here
ARMClang:
OOLCHAIN_ARM_STD\startup_MK64F12.S
armclang: error: ARM Compiler does not support '-mcpu=Cortex-M4-mcpu=cortex-m4'
armclang: error: ARM Compiler does not support '-mcpu=Cortex-M4-mcpu=cortex-m4'
armclang: error: armasm does not support CPU 'cortex-m4-mcpu=cortex-m4'
armclang: error: armasm does not support FPU 'unknown'
@ladislas if you can share the multiple target example, or we shall create our own to test the fix?
Oh whoops, there was an issue where the wrong linker script path was passed for GCC_ARM. I pushed a new commit to #13987 to fix it. I also found a variable name conflict with the profile scripts that was probably causing the arm compiler error, pushed a fix for that too.
if you can share the multiple target example, or we shall create our own to test the fix?
@0xc0170 yes, I'm working on a simple example, I'll share it later today.
@multiplemonomials @0xc0170
Alright, I've setup a working example of our project structure, available here:
https://github.com/ladislas/mbed-cmake-template/tree/feature/move-to-vanilla-cmake
Instructions are available here:
https://github.com/ladislas/mbed-cmake-template/tree/feature/move-to-vanilla-cmake#-wip---from-uscrplmbed-cmake-to-mbed-os-first-party-cmake
If you want to compare the same template with USCRPL/mbed-cmake, you can follow the instructions in develop
OK @ladislas I tested your example, and my cmake-object-libs branch that I linked earlier is able to compile it correctly with no other changes and only 247 files compiled.
@multiplemonomials that's great news, I've just tested it myself and I can confirm it compiles both the main src and the spike without any problems :)
The number of files to compile (spike + lib + src) is also much lower than it was previously:
Most helpful comment
OK @ladislas I tested your example, and my cmake-object-libs branch that I linked earlier is able to compile it correctly with no other changes and only 247 files compiled.