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 CMakePresets with defaults relevant for building CXX-Qt #325

Closed
wants to merge 1 commit into from

Conversation

ahayzen-kdab
Copy link
Collaborator

@ahayzen-kdab ahayzen-kdab commented Oct 18, 2022

This allows us to have presets for building with Qt 5, specifying recommended generators, caching tools, and showing output on failure of tests.

  • Add preset for vcpkg
  • Add preset for sscache (?)
  • Add preset for ninja (?)
  • Potentially use presets for CI too
  • Add RUSTC_WRAPPER to environment for sscache presets
  • Should we have all the combinations or just the simplest + "best" ?

CMakePresets.json Outdated Show resolved Hide resolved
@ahayzen-kdab ahayzen-kdab marked this pull request as draft October 18, 2022 22:01
@ahayzen-kdab ahayzen-kdab changed the title cmake: add CMakePresets with defaults relevant for building CXX-Qt WIP: cmake: add CMakePresets with defaults relevant for building CXX-Qt Oct 18, 2022
@ahayzen-kdab ahayzen-kdab marked this pull request as ready for review October 19, 2022 12:44
@ahayzen-kdab ahayzen-kdab changed the title WIP: cmake: add CMakePresets with defaults relevant for building CXX-Qt cmake: add CMakePresets with defaults relevant for building CXX-Qt Oct 19, 2022
@ahayzen-kdab
Copy link
Collaborator Author

Seems Windows might need the --config Release etc at build time ? So might need buildPresets for all the combinations too 😅

This allows us to have presets for building with Qt 5, specifying
recommended generators, caching tools, and showing output on
failure of tests.
@Be-ing
Copy link
Contributor

Be-ing commented Oct 20, 2022

I don't understand the point of having presets when there's a preset for every combination of options. Why not just specify the CMake options that already exist?

@ahayzen-kdab
Copy link
Collaborator Author

I don't understand the point of having presets when there's a preset for every combination of options. Why not just specify the CMake options that already exist?

Right maybe we don't need all the options but just the main few. But it's still release yes/no, Qt5 yes/no, and vcpkg yes/no, then assume they have ninja and sscache (which removes one set from the matrix).

Note that the CMakePresets are useful for IDEs like vscode as the CMake plugin there uses the presets to decide how to build and setting the options manually is kinda a pain.

And other KDAB projects have various combinations in the CMakePresets https://github.com/KDAB/KDDockWidgets/blob/1.7/CMakePresets.json :-)

And putting some of the options into presets means that developers don't have to hunt around README/CMakeLists.txt finding which options are available, as described in the blog https://www.kdab.com/the-power-of-cmake-presets/

@Be-ing
Copy link
Contributor

Be-ing commented Oct 21, 2022

vscode... setting the options manually is kinda a pain.

That sounds like a VSCode problem. :)

I don't have any use for these because I build in a shell, so I don't really mind what you do with these. @LeonMatthesKDAB, thoughts?

@ahayzen-kdab
Copy link
Collaborator Author

vscode... setting the options manually is kinda a pain.

That sounds like a VSCode problem. :)

Hehe :-) I can keep them as CMakeUserPresets just didn't know if they were useful upstream. Also CI could use them and when building from the CLI they could be used rather than multiple flags.

Or the matrix could be reduced so there are only some options :-)

"configuration": "Release",
"hidden": true
},
{
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do use this we likely also want to set the parallel / execution jobs to something larger than 1

"execution": {
    "jobs": 2
},

@ahayzen-kdab ahayzen-kdab marked this pull request as draft November 1, 2022 16:36
@ahayzen-kdab
Copy link
Collaborator Author

Going to close this for now as there are too many combinations to push into the repo.

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

2 participants