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: Add a CMakePresets.json for msvc that handle Debug build with sucess #498

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

OlivierLDff
Copy link
Contributor

@OlivierLDff OlivierLDff commented Mar 30, 2023

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:

QML_IMPORT_PATH=$(SolutionDir)/vcpkg_installed/x64-windows/debug/Qt6/qml
QT_PLUGIN_PATH=$(SolutionDir)/vcpkg_installed/x64-windows/debug/Qt6/plugins
PATH=$(SolutionDir)/vcpkg_installed/x64-windows/debug/bin

or for Release/RelWithDebInfo:

QML_IMPORT_PATH=$(SolutionDir)/vcpkg_installed/x64-windows/Qt6/qml
QT_PLUGIN_PATH=$(SolutionDir)/vcpkg_installed/x64-windows/Qt6/plugins
PATH=$(SolutionDir)/vcpkg_installed/x64-windows/bin

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:
image

image

And for reference mix debugging work out of the box:

debug-msvc

@OlivierLDff OlivierLDff changed the title cmake: Add a CMakePresets.json for msvc that handle Debug build with cmake: Add a CMakePresets.json for msvc that handle Debug build with sucess Mar 30, 2023
CMakePresets.json Outdated Show resolved Hide resolved
@OlivierLDff OlivierLDff force-pushed the cmake-presets branch 2 times, most recently from 8bf6ce7 to c510e01 Compare March 31, 2023 17:43
@OlivierLDff
Copy link
Contributor Author

I've pushed a second proposal based on @ahayzen-kdab CMakeUserPresets.json.
I think that changing the default value of VCPKG based on user OS is bad and highlight dev preference.
I think VCPKG should be defaulted to ON by default, so that default configuration "just build". Same it should be set to ON in CMakePresets.json, since presets are expected to "just work" no matter how the host system is configured.

@Be-ing
Copy link
Contributor

Be-ing commented Apr 1, 2023

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?

if(WIN32 AND (CMAKE_BUILD_TYPE STREQUAL "Debug" OR CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo"))
  set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreadedDLL")
endif()

@OlivierLDff
Copy link
Contributor Author

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

@OlivierLDff
Copy link
Contributor Author

OlivierLDff commented Apr 1, 2023

Ok so I took a drastic modification in the Preset, there is only 2 left.

  • MSVC on windows.
  • Ninja Multi otherwise.

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.

@Be-ing
Copy link
Contributor

Be-ing commented Apr 1, 2023

I am reluctant to recommend use of the MSBuild generators because MSBuild doesn't support ccache/sccache with CMAKE_LANG_COMPILER_LAUNCHER.

@OlivierLDff
Copy link
Contributor Author

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?

@OlivierLDff
Copy link
Contributor Author

In the end the choice is yours, if you are against CMakePresets, then writing CMAKE_MSVC_RUNTIME_LIBRARY is the best workaround. Be aware that CMAKE_BUILD_TYPE doesn't really matter for multi config generator and you should avoid it. Better use generator expression.

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()

@Be-ing
Copy link
Contributor

Be-ing commented Apr 1, 2023

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.

@ahayzen-kdab
Copy link
Collaborator

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.

@OlivierLDff
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants