-
Notifications
You must be signed in to change notification settings - Fork 18
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
Split app.cmake into two parts to fix CMake deprecation warning #421
Conversation
One point about GCC finding. I noticed the VS Code uses GCC 11.3.1 every time and this even though I have GCC 12 and 13 installed. Then I found it uses the one from STM32 package especially from STM32CubeCLT. This probably caused because it is first in the list and rest are probably ignored. My list:
The fact about it is working just with STM32CubeCLT is cool because you save another space but this can be issue for someone who want to use latest GCC or a specific one for some compatibility reason. Maybe we should make a not in wiki about this. Or any idea how to solve this? |
I choose the gcc version manually, in the VSCode Cmake extension you can select the kit you want. |
'Scan for kits' needs the gcc in the search path, then it is added to the list. The list can be edited also manually, somewhere hidden in the VSCode settings |
I ofc know about the list and possibility to choose but how the wiki says, it should stay unspecified because CMake will find and choose it automatically because of another functionality. BTW that is what we both did wrong. Some new users reported something is wrong and we said "No, everything is ok" but that was because we are choosing the kit manually, however the wiki say something else. So we did it wrong and these new users were right. |
Do you know what is different by using unspecified? |
It is written on the picture above.
When I made videos for video guides for "how to start" that i recognize the behaviour was different and lefebvresam was right in one of his issue. |
Hmm, I have a build dir per target and build variation because it is defined in settings.json to use the environment variables. |
No, I like just basic project structure so I have it exactly how is described - https://github.com/mbed-ce/mbed-os/wiki/Project-Setup:-VS-Code#loading-your-project |
That is exactly what I’m using also. |
@JohnK1987 Yeah probably it's because the STM32CubeCLT one is first on your PATH. Moving the other one ahead on your PATH and then doing a fresh build should (TM) fix this issue, I'd hope |
I have tried this commit, removed mbed_finalize_build(), but left old include app.cmake. It did work also, So the result in this case is only getting some warnings? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested and it worked
Summary of changes
Up until now, CMake projects using Mbed had to include
app.cmake
near the top of their CMakeLists.txt files. This worked fine for a while, but it did have the side effect of calling enable_languages() before project(), which now triggers a warning in CMake 3.31 (apparently you weren't supposed to ever do it that way... who knew).Unfortunately, there was no way to fix this without breaking changes for projects using Mbed, as the existing app.cmake file had to be split up. However, I did what I could to try and ease the pain:
Impact of changes
Should not actually break any projects, but adds a new, preferred way to structure CMakeLists.txt and adds several deprecation warnings if you are using the old way.
Migration actions required
Up until now, a basic CMakeLists.txt looked like:
Going forward, this should be changed to:
Basically:
include(mbed-os/tools/cmake/app.cmake)
replaced withinclude(mbed-os/tools/cmake/mbed_toolchain_setup.cmake)
project()
call must be immediately afterinclude(mbed-os/tools/cmake/mbed_toolchain_setup.cmake)
include(mbed_project_setup)
must be immediately afterproject()
.Documentation
Will update docs to use new structure when merged
Pull request type
Test results