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 implementation #568

Merged
merged 2 commits into from May 8, 2021
Merged

cmake implementation #568

merged 2 commits into from May 8, 2021

Conversation

ghost
Copy link

@ghost ghost commented Feb 24, 2021

A initial test of a modern cmake implementation.

All flags from lbuild is mapped to a proper CMake model also per-file flags. CMake can be responsible for compiler choice and build type without implications. Flags like -O, -std and -ffile-attr-map is filtered and controlled by CMake.

Full support for DSP library.

CMake interface targets

  • modm_warnings - lbuild defined warnings
  • modm_options - lbuild defined build options
  • modm - modm itself (all headers is defines as system headers to isolate modm warnings from users. Easy changeable, do we need a option for that?)

Cached entries

MODM_DBG_UI     (tui | gdbgui)
MODM_BMP_PORT   0
MODM_ITM_FCPU   auto
MODM_GCC_PATH   read from enviroment variable GCC_PATH if present or defaults to '/usr/local/arm-none-eabi'

A draft example on usage is here. (I need a better place to put it?)

@salkinium
Copy link
Member

salkinium commented Feb 24, 2021

Yes, I'm very glad you're integrating your changes back into modm!

All examples needs to be changed.

No? We're not adding more indirection by hiding our main.cpp files in a src folder just because CMake has an opinion on that. It works fine now, so why change it? Besides I'm not taking tips on best-practices from some API as weird as CMake *drops mic* *runs away screaming*.

Some compiler options are automatically managed by cmake and conflicts with scons

Nah, CMake must not automatically manage compile flags. The optimization is -Os for release and -Og for debug. Why would CMake know any better? Why does CMake even modify these compile flags? It doesn't know it's compiling for an embedded target. We also had to remove all default settings from SCons for the same reasons.

PS: Note that we ran out of free credits for CircleCI for this month, and thus speed has been degraded significantly.

@salkinium salkinium linked an issue Feb 25, 2021 that may be closed by this pull request
@rleh
Copy link
Member

rleh commented Feb 26, 2021

Maybe aux_source_directory(<dir> <variable>) (https://cmake.org/cmake/help/latest/command/aux_source_directory.html) helps to avoid having the src/ directory?

@rleh
Copy link
Member

rleh commented Feb 26, 2021

@Jasa Are you aware of #558? In the pull request some parts of the build system generator are reworked.

.gitignore Outdated Show resolved Hide resolved
@ghost ghost requested a review from rleh February 27, 2021 13:44
@ghost ghost marked this pull request as draft March 1, 2021 10:59
@ghost
Copy link
Author

ghost commented Mar 2, 2021

@salkinium please is head comment, a few questions there. The only functionality missing is flags on file level, is there any examples on this?. I think, it turned out to be a simple but strong CMake integration.

I have some strange error's "ResouceBusy" in the circleci, anyone knows what's that about?.

@salkinium
Copy link
Member

The CMSIS-DSP examples use per-file compile flags. We've run out of free credits for this month, so CircleCI is running our CI on vapour.

@salkinium
Copy link
Member

What IDE should I test this PR with? CLion? QT Creator? VS Code? I'm using Sublime Text, but that's not an IDE…

@salkinium
Copy link
Member

A draft example on usage is here. (I need a better place to put it?)

The CMake options should be put into the CMake module.md documentation. I'd be happy to have a CMake-only example in modm, with the src/main.cpp mechanism and a small header only library integration.

@ghost
Copy link
Author

ghost commented Mar 2, 2021

What IDE should I test this PR with? CLion? QT Creator? VS Code? I'm using Sublime Text, but that's not an IDE…

Personally i use vscode in the moment. clion also works flawlessly. but a simple text editor will also work, some times i use neo-vim. vscode requires some plugins before it works acceptable.

Plugins:
C/C++
CMake Tools
CMake - syntax highlighting
Clang-format

@ghost ghost marked this pull request as ready for review March 7, 2021 20:21
@ghost
Copy link
Author

ghost commented Mar 7, 2021

Full support for DSP library and per-file flags from lbuild is now implemented. My Python skills is very very rusty and per-files flags was not trivial to implement so i hope the code is not to terrible. No new features will be added for now.

@rleh rleh requested review from rleh and removed request for rleh March 8, 2021 01:07
@rleh
Copy link
Member

rleh commented Mar 8, 2021

I've only skimmed the code so far and haven't had time to test the new CMake generator on my computer.

I would actually still like to remove the Makefile from the CMake generator, since CMake (by now) should be able to generate a Makefile itself, right? I'll have a look at it next week...

@salkinium
Copy link
Member

salkinium commented Mar 16, 2021

Ok, I've gotten around to using this in VSCode: I like it a lot! Good job!
A couple of thoughts on the experience:

  • I installed the C/C++ and CMake Tools extensions. This automatically recognized the generated CMakeLists.txt and configured the project. I was able to use code completion, build and program very easily. This is nice!
  • VSCode asked me to choose a toolchain Kit. I guess this is what you meant with "CMake chooses the compiler". However, the setting seems to be ignored, because we hardcoded this into the generator. I also don't see how choosing the kit is useful, modm really only works with two compilers…
  • I found these CMake profiles to choose from, but I have no idea what flags they set or how the IDE/CMake would know what flags are appropriate.
  • I found no settings to change the C++ standard, or add any flags inside the IDE.
  • The build path is not respected. Not sure if CMake supports build path relocation when called from an IDE.
  • The program and size targets worked fine, after I removed the additional src/ in the path ;-p (see below)
  • I don't think the debug targets make sense. When called from the IDE, GDB refuses to launch, because Cannot enable the TUI when output is not a terminal. Even if it did, using the TUI inside an IDE is a little stupid.
  • I didn't find a way to set the fcpu frequency for the log-itm target from inside the IDE without modifying CMakeLists.txt.
  • I tried using the CortexDebug extension, but I already just failed miserably to even launch it?
  • MODM_GCC_PATH is undefined, it was previously automatically detected with FIND_PROGRAM.

I think we should have an lbuild option for using the src/ mechanism or our non-src example mechanism. It should default to using src/ and will be set to non-src by the examples lbuild.xml file. That way external projects (and specific modm examples) can use the proper src/ mechanism without globbing.

@rleh
Copy link
Member

rleh commented Mar 16, 2021

I don't think the debug targets make sense. When called from the IDE, GDB refuses to launch, because Cannot enable the TUI when output is not a terminal. Even if it did, using the TUI inside an IDE is a little stupid.

I does not make sense to be used in an IDE (we would need some adapter to the IDE-Debugger-Interface...) but people like me will call the debug targets from terminal (e.g. the terminal inside VSCode)

@rleh

This comment has been minimized.

@ghost
Copy link
Author

ghost commented Mar 16, 2021

* VSCode asked me to choose a toolchain Kit. I guess this is what you meant with "CMake chooses the compiler". However, the setting seems to be ignored, because we hardcoded this into the generator. I also don't see how choosing the kit is useful, modm really only works with two compilers…

You can have different compiler versions. So why not let the user choose what he want.

* I found these CMake profiles to choose from, but I have no idea what flags they set or how the IDE/CMake would know what flags are appropriate.

Flags can be changed, see below how to configure CMake. Some IDE's support it.

* I found no settings to change the C++ standard, or add any flags inside the IDE.

It is set in the root CMakeLists.txt.

* The build path is not respected. Not sure if CMake supports build path relocation when called from an IDE.

You can set the build path to whatever you want but not from 2 places at once. It might not work with everything if lbuild rules, but it could be fix in the CMakeFile.txt for the CI.

* The `program` and `size` targets worked fine, after I removed the additional `src/` in the path ;-p (see below)

CMake generates the build script (make, ninja, whatever) in the build folder and you can run `make (program | size | gdb) from there. It has nothing to do with the src folder. Something is definitely wrong if that's the case.

* I don't think the `debug` targets make sense. When called from the IDE, GDB refuses to launch, because `Cannot enable the TUI when output is not a terminal`. Even if it did, using the TUI inside an IDE is a little stupid.

It makes sense to me, are cmake users not allowed to debug? CMake actually works and often used without IDE. Don't use vscode as reference, is not that perfect.

cmake --build .. --config Debug
make gdb
* I didn't find a way to set the fcpu frequency for the `log-itm` target from inside the IDE without modifying CMakeLists.txt.

You can configure everything from IDE, ccmake, cmake-gui (if installed) or command line. look here

Screenshot from 2021-03-16 15-05-02

* I tried using the CortexDebug extension, but I already just failed miserably to even launch it?

Never tried it.

* `MODM_GCC_PATH` is undefined, it was previously automatically detected with `FIND_PROGRAM`.

CMake reads enviroment variable GCC_PATH and sets MODM_GCC_PATH (cached variable) if present. I can change this behavior for the default CMakeLists.txt to the CI. The old solution is platform dependent and will not work everywhere. The library path is totally different on many platforms because gcc is configured differently. On my machine the GCC install path is /usr/local and the binaries is in /usr/local/bin and the library path is /usr/local/arm-none-eabi which appears in the elf files.

It's programmed to give a warning and it works here:

CMake Warning at CMakeLists.txt:16 (message):
   Enviroment variable GCC_PATH not set.

   Defaults to: /usr/local/arm-none-eabi

I think we should have an lbuild option for using the src/ mechanism or our non-src example mechanism.

The default generated CMakeLists.txt is the non-src version and it's working with the CI and all examples. (note. it's not overwritten by lbuild if it exists). The example project is a src version. lbuild don't know anything about other organizations source structure so why worry about that.

My goal in the design was to keep it flexible so it easily can be modified to everyone's platform and there build setup. instead of forcing a build setup from modm's CI to anyone. The only thing modm/CMakelists.txt and the included file worries about is how modm itself should be build and nothing else. The lbuild generated main CMakeLists.txt with globing is only for the CI and the user is supposed to make his own for his needs.

@ghost
Copy link
Author

ghost commented Mar 17, 2021

This CMake implementation don't change any flags regarding building modm. Only the responsibility is changed since CMake isn't acting like scons. This can be verified by: make VERBOSE=1

Example:
[ 14%] Building CXX object modm/CMakeFiles/modm.dir/ext/gcc/new_delete.cpp.obj cd /home/jsa/workplace/modm_starter_project/build/modm && /usr/bin/ccache /usr/local/bin/arm-none-eabi-g++ -isystem /home/jsa/workplace/modm_starter_project/modm/ext -isystem /home/jsa/workplace/modm_starter_project/modm/ext/cmsis/core -isystem /home/jsa/workplace/modm_starter_project/modm/ext/cmsis/device -isystem /home/jsa/workplace/modm_starter_project/modm/ext/crashcatcher/include -isystem /home/jsa/workplace/modm_starter_project/modm/src -Os -DNDEBUG -fdiagnostics-color=always -fdiagnostics-color=never -ffile-prefix-map=/usr/local/arm-none-eabi=. -ffile-prefix-map=/home/jsa/workplace/modm_starter_project=. -fdata-sections -ffunction-sections -finline-limit=10000 -fshort-wchar -fsingle-precision-constant -funsigned-bitfields -funsigned-char -fwrapv -g3 -gdwarf-3 -mcpu=cortex-m4 -mfloat-abi=hard -mfpu=fpv4-sp-d16 -mthumb -fno-exceptions -fno-rtti -fno-unwind-tables -fstrict-enums -fuse-cxa-atexit -W -Wall -Wdouble-promotion -Wduplicated-cond -Werror=format -Werror=maybe-uninitialized -Werror=overflow -Werror=sign-compare -Wextra -Wlogical-op -Wno-redundant-decls -Wpointer-arith -Wundef -Wno-volatile -Woverloaded-virtual -std=gnu++2a -o CMakeFiles/modm.dir/ext/gcc/new_delete.cpp.obj -c /home/jsa/workplace/modm_starter_project/modm/ext/gcc/new_delete.cpp

@salkinium
Copy link
Member

Ok, I definitely want to work these changes into modm, since I want to have a standardized way of integrating third-party code via CMake.

However, since this is all fairly complicated and overwhelming for me, I want to do the following:

  1. Finish the Makefile generator before I forget what I did there.
  2. Split up our compile flags into optional and required flags. The PerFileAttr class is a good idea and can be reused here for all. I also want to have better docs for the compile flags.
  3. Integrate the split flags into this PR.
  4. Deal with the remaining issues.

@ghost

This comment has been minimized.

@ghost
Copy link
Author

ghost commented Mar 20, 2021

What is the true purpose for -ffile-prefix-map=${MODM_GCC_PATH}=. in modm.
Yes it will shorten the FILE macro and prevent gdb from loading GCC header files. Is there something I'm missing?

@salkinium
Copy link
Member

salkinium commented Mar 20, 2021

Yes, we want to archive ELF files automatically for group projects, so that anyone can debug the currently running firmware on the hardware. That's why we added the artifact store for scons, the GNU Build ID and the relative paths. In theory, the artifact path is on a server, and you should just get the current build id from the log and do scons debug firmware={build_id hash} and it should work on your computer like magic.

So to be specific, since people have different OSes with different project paths, the absolute paths will prevent the ELF file from finding the right sources.

@ghost
Copy link
Author

ghost commented Mar 21, 2021

I have made a few changes you might like @salkinium. I think maybe this is a better way to handle conflicts with external modules and in-source builds are not allowed now.

Now no flags are filtered except -Ox which is set to:

  • MinSizeRel: -Os
  • Relase: -O2
  • Debug: None

Target interfaces defined:

  • modm_options - compiler flags only
  • modm_warnings - warnings flags only
  • modm_arch_options - architecture flags only
  • modm - linker flags + architecture flags

All modm sources is compiled with lbuild generated flags no matter what.
The same applies to the user code if the following targets is defines:

target_link_libraries(MainUserCode
  PRIVATE
  modm_options
  modm_warnings
  modm)

If a external xyz_module with conflicting flags are added the flags from modm_options has to be redefined locally. Following applies:

target_compile_features(xyz_module INTERFACE cxx_std_17)    # adds -std=c++17

target_compile_options(xyz_module 
  PRIVATE
  -fdata-sections -ffunction-sections etc....
  $<$<COMPILE_LANGUAGE:CXX>:-fno-exceptions -fno-rtti -fno-unwind-tables -fstrict-enums -fuse-cxa-atexit>)

target_link_libraries(xyz_module
 PRIVATE
 modm_arch_options
 modm_warnings) 

The build path is not respected. Not sure if CMake supports build path relocation when called from an IDE.

You cheated on me, CMake make has never done that. It's your own Makefile there are creating the build path. 😃

@salkinium
Copy link
Member

salkinium commented Apr 9, 2021

Ok, sorry for the long wait. The latest changes are indeed much cleaner, I like it much more. I'd prefer to merge this sooner rather than later, even without the flags splitting support for the other build systems (not motivated to do that right now). Todo before merge:

  • module.md documentation needs an update.
  • Makefile wrapper needs to be removed.
  • Add start-group and end-group linker flags. not needed for object library
  • Adapt the tools/scripts/compile_examples.py script for calling the CMake directly.
  • Properly default MODM_GCC_PATH.
  • Relicense the top-level CMakeLists.txt to BSD 2-clause.

@jnewcomb
Copy link

jnewcomb commented Apr 25, 2021

@Jasa Hi - suggestion..
Force map file generation by default?
I added:

-Wl,-Map=${target}.map

to:

set(LINKFLAGS...)

in:
ModmConfiguration.cmake

@salkinium
Copy link
Member

@Jasa any idea how to add start-group and end-group linker flags? They are pretty important for modm.

Are you ok with relicensing your CMakeLists.txt as BSD 2-clause?

@ghost
Copy link
Author

ghost commented May 1, 2021

@Jasa any idea how to add start-group and end-group linker flags? They are pretty important for modm.

If you have a really good reason. CMake has a more convenient way to achieve the same, In modm/CMakeLists.txt you will find add_library(modm OBJECT and it has the same effect. (everything is linked without exception)

Since CMake introduced Object libraries there is no need to create a static library unless you want to install them permanently on the host. There are also plenty examples on Stack Exchange.

Are you ok with relicensing your CMakeLists.txt as BSD 2-clause?

No problem at all.

@salkinium
Copy link
Member

In modm/CMakeLists.txt you will find add_library(modm OBJECT and it has the same effect. (everything is linked without exception)

Excellent!

I've updated the module docs, however, I'm not an expert so please review @Jasa.
Do you want to add a few lines about how to integrate the modm/CMakeLists.txt into a typical application and what interfaces are available (ie. modm_warnings etc)?

@salkinium salkinium added the ci:hal Triggers the exhaustive HAL compile CI jobs label May 8, 2021
jschultz-dk and others added 2 commits May 8, 2021 04:45
These scripts are intended to be modified by the user for their project
without the need to publish their changes to it.
@salkinium salkinium added ci:hal Triggers the exhaustive HAL compile CI jobs and removed ci:hal Triggers the exhaustive HAL compile CI jobs labels May 8, 2021
@salkinium salkinium merged commit 98b1337 into modm-io:develop May 8, 2021
@jnewcomb
Copy link

Hello @Jasa
Not sure if it's a user error, but I don't see a generated top level CMakeLists.txt file.
To reproduce I edited the nucleo_f429zi\blink example from

<module>modm:build:scons</module>

To:

<module>modm:build:cmake</module>

I ran the 'lbuild build' command and was expecting to see the CMakeLists.txt file in the working directory alongside the project.xml file.

project.xml.log.zip

Using ubuntu 20.04 under windows terminal, Python 3.8.5

image

The modm directory looks complete
image

Any hints welcome.. Thanks..

@salkinium
Copy link
Member

salkinium commented May 10, 2021

You need to set the modm:build:cmake:include_cmakelists option to yes. It's off by default now to force you to write your own, proper CMakeLists.txt like in our example: https://github.com/modm-ext/modm_starter_project

@salkinium salkinium added this to the 2021q2 milestone Jun 20, 2021
@ALSCode
Copy link

ALSCode commented Jan 9, 2022

Hello!
Please, fix option entries
FROM "modm:build:cmake:include_makefile"
TO "modm:build:make:include_makefile"

in README.md https://github.com/modm-ext/modm_starter_project.git

@salkinium
Copy link
Member

Good catch, I've fixed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
advanced 🤯 ci:hal Triggers the exhaustive HAL compile CI jobs lbuild 🏗 toolchain ⚙️
Development

Successfully merging this pull request may close these issues.

cmake issues
5 participants