-
Notifications
You must be signed in to change notification settings - Fork 77
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: Add a CMakePresets.json for msvc that handle Debug build with sucess #498
base: main
Are you sure you want to change the base?
Conversation
8bf6ce7
to
c510e01
Compare
I've pushed a second proposal based on @ahayzen-kdab CMakeUserPresets.json. |
c510e01
to
0d1748e
Compare
Ehh, this is turning into lots of different presets. which brings up the same question as before. Could we hack around the Windows debug build issue another way by adding this to CMakeLists.txt?
|
It's only 5 presets. It could be trim to 2 ninja multi config and msvc. The fact I included all configuration to inherit is so it is simple to use from cmakeuserpresets.json |
Ok so I took a drastic modification in the Preset, there is only 2 left.
I think it is the 2 sane default that should work anywhere. Leaving Ninja on windows, if clang++ is available it will use that instead of msvc, leading to conflict with Qt version used from vcpkg. |
I am reluctant to recommend use of the MSBuild generators because MSBuild doesn't support ccache/sccache with CMAKE_LANG_COMPILER_LAUNCHER. |
Does it even matter for this project to use ccache/sccache ? Most of the code compiled is rust, so it doesn't really matter what build system is used or c++ compiler? |
In the end the choice is yours, if you are against CMakePresets, then writing if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
# rust cc::Build always use MultiThreadedDLL, so for c++ code handled from CMake to correctly link with generated code by Cargo, this variable is globally set on all CMake target.
# This is a workaround, and is not optimal.
set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreadedDLL")
endif() |
I think adding that little bit to make debug builds work on Windows regardless of presets would be helpful. I personally don't really understand the value of adding this CMakePresets.json, but if it helps some people then sure. So I'll neither merge nor block this and leave the decision to others. |
I think for now lets put this in the CMakeLists so that it solves the issue for both users of CMakePresets and users who use CMake manually. Then presets can be a separate investigation / task whether we want to include one. Then i guess the CMakeLists workaround should be documented / visible in the code snippets somewhere so that people who are using CXX-Qt as a library know to put this in their CMakeLists too. |
Another alternative that I saw in corrosion: https://github.com/corrosion-rs/corrosion/blob/e516737bf4987d000353045133c2f82b23553490/test/cxxbridge/cxxbridge_rust2cpp/CMakeLists.txt#L13-L16 |
Related to #495 .
It's nice if user want to hack around this repo and the example to be able to build with Debug mode in windows.
Maybe the CMakePresets should be mentioned in Readme, but then a variant for most common generator should be added (like Ninja, Xcode I guess)
What is important here for windows is
"CMAKE_MSVC_RUNTIME_LIBRARY": "MultiThreadedDLL"
.Then by settings for Debug build with environment:
or for Release/RelWithDebInfo:
I'm able to correctly start the app from visual studio. (Not familiar enough with
launch.json
of vscode, but I guess the same can be done).Screenshot for reference:
And for reference mix debugging work out of the box: