cmake_conan_boilerplate_template icon indicating copy to clipboard operation
cmake_conan_boilerplate_template copied to clipboard

Feature/presets enchancements

Open Jason5480 opened this issue 2 years ago • 15 comments

This PR contains several enhancement to the Cmake presets.

  • [x] - Setting "Ninja Multi-Config" as the default generator should have several benefits for your IDEs (see also #15)
  • [x] - Better structure of the hidden presets to facilitate better composability and extensibility!
  • [x] - Due to those changes different modes can be added depending on the use case (developer, user, ci etc.)
  • [ ] - Add integration of the presets with the CI

Currently we support 4 toolchains +1 mode option for msvc (thus 5 non-hidden configure presets) and the corresponding Debug and Release builds (thus 10 non-hidden buildPresets and 10 testPresets)

Since this is getting bigger and bigger with each new configuration option added to the project the configurePresets are doubled in numbers (e.g. msvc + mode option -> msvc Developer Mode and msvc User Mode) and the buildPresets and testPresets are quadrupled. So, at this point I have the following questions.

  1. Shall we consider adding a CMakeUserPresets.json file to that repository that has the default user settings for developing that project? What I have in mind is to let the default build options to the CMakePresets.json and move the user specific options (and modes) to the CMakeUserPresets.json.
  2. Should I also add support for RelWithDebInfo for completeness?
  3. Is it necessary to be able to run tests both in Debug and Release?
  4. Should the mode option be added in all supported toolchains currently it is only in msvc. gcc clang and clang-cl do not have that option
  5. What is the value of the 'ENABLE_DEVELOPER_MODE' if it is not defined?

@ddalcino @aminya @lefticus @oskidan some feedback would be welcome! Also, @oskidan and @ddalcino can you check that those changes are still functional in macOS to make sure I didn't mess with #16

Co-Author: @ClausKlein Thank you for the feedback and your test in macOS that unfortunately cannot do.

Jason5480 avatar Apr 01 '22 12:04 Jason5480

Codecov Report

Merging #19 (ab84504) into main (c77d33f) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #19   +/-   ##
=======================================
  Coverage   77.77%   77.77%           
=======================================
  Files           3        3           
  Lines          36       36           
  Branches       19       19           
=======================================
  Hits           28       28           
  Misses          7        7           
  Partials        1        1           
Flag Coverage Δ
Linux 37.03% <ø> (ø)
Windows 80.00% <ø> (ø)
macOS 38.46% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c77d33f...ab84504. Read the comment docs.

codecov-commenter avatar Apr 01 '22 12:04 codecov-commenter

@ddalcino on gcc Debug build the CI failed to install cmake is anything wrong?

Jason5480 avatar Apr 06 '22 12:04 Jason5480

@lefticus there are still some pending questions about the user/developer mode. And the combinatorial complexity of all the supported platforms, toolchains etc. Also, it was proposed by @ClausKlein to also support mingw and BSD. @oskidan if I am not mistaken you mentioned that BSD should be supported with the current unixlike presets. Am I right?

Jason5480 avatar Apr 06 '22 12:04 Jason5480

@ddalcino on gcc Debug build the CI failed to install cmake is anything wrong?

That looks like an issue with setup-cpp. Looks like it resolved itself though. If you're worried about it, you could file an issue. It's reasonable to expect that setup-cpp would either cache this dependency for us (our cache is turned on) or retry on error.

ddalcino avatar Apr 07 '22 00:04 ddalcino

@lefticus there are still some pending questions about the user/developer mode. And the combinatorial complexity of all the supported platforms, toolchains etc. Also, it was proposed by @ClausKlein to also support mingw and BSD. @oskidan if I am not mistaken you mentioned that BSD should be supported with the current unixlike presets. Am I right?

We are deliberately not supporting mingw due to a long history of really nasty bugs. I thought we had discussed this somewhere else in the cpp_best_practices org, but I can't find it now. Relevant: https://twitter.com/lefticus/status/1484215081806098432

IMHO, we can only claim to support the platforms that we are testing in CI. We should try not to go out of our way to break the project on BSD, but I don't think we need to go out of our way to support it. If you support BSD, where do you draw the line? Solaris? ReactOS? FreeDOS? Redox? Serenity? There's just too many. Please don't forget about our Conan dependencies, many of which do not support any OSes other than win/mac/linux.

Let's not forget, the cmake presets are a convenience. Users can easily do without them, or write their own.

ddalcino avatar Apr 07 '22 01:04 ddalcino

@Jason5480 Hi Nikloas, we should have a separate forum to have such a diskusion.

If you want to use the presets in an IDE, you only need the configpresets, or not?

If you do not know, were the binary directory is, you need the buildpresets too on console.

And with the cmake --build builddirpath --taget test you need no testpreset!

Now I see why the presets are from microsoft

ClausKlein avatar Apr 07 '22 15:04 ClausKlein

If you want to use the presets in an IDE, you only need the configpresets, or not?

Now I see why the presets are from microsoft

2022-04-08 10_33_54-

Currently VSCode and VisualStudio support all three types of presets. CLion supports only build presets QtCreators integration is on its roadmap. For XCode I do not know but the key concept of the presets is that in the future you can have IDE agnostic codebases no need to support *.vsxpjoj files along with *.creator and other IDE specific files. You have one format to produce reproducible builds in all of them! And the command line options should not be excluded from this for development.

Jason5480 avatar Apr 08 '22 07:04 Jason5480

If you do not know, were the binary directory is, you need the buildpresets too on console.

And with the cmake --build builddirpath --taget test you need no testpreset! ...

Command line options override the presets so you can pass your own binary and install dir through the cli. I just put the VSCode/VS defaults there in case you want to open the project that is already build before with another IDE it should be able to find existing build folder and not re-build all of them on its own (different) default folder

Jason5480 avatar Apr 08 '22 08:04 Jason5480

Currently VSCode and VisualStudio support all three types of presets. CLion supports only build presets QtCreators integration is on its roadmap. For XCode I do not know but the key concept of the presets is that in the future you can have IDE agnostic codebases no need to support *.vsxpjoj files along with *.creator and other IDE specific files. You have one format to produce reproducible builds in all of them! And the command line options should not be excluded from this for development.

In my world, I generate a build project (MVS, Xcode, Ninja Multi-Config, Eclipse, ...) from console with CMake. For CI build, we build and test them too from a bash shell.

If needed, I open the generated IDE to develop or debug, ...

So I never need an build preset or a test preset.

In every IDE I ever used, the CMake generated targets all, test, clean, install, ... are available.

see https://discourse.cmake.org/t/cmake-presets-what-is-a-testpreset-for/5404

ClausKlein avatar Apr 08 '22 08:04 ClausKlein

In my world, I generate a build project (MVS, Xcode, Ninja Multi-Config, Eclipse, ...) from console with CMake. For CI build, we build and test them too from a bash shell.

If needed, I open the generated IDE to develop or debug, ...

So I never need an build preset or a test preset.

In every IDE I ever used, the CMake generated targets all, test, clean, install, ... are available.

see https://discourse.cmake.org/t/cmake-presets-what-is-a-testpreset-for/5404

Yes, this is the "old" way (better than the manual) of supporting several IDEs. With the presets the IDEs are getting closer to the CMake and not the other way around. Also, vendor specific options enable some customization points for the IDE without affecting others. Having different generators for each IDE is also an option but how do you make sure that project for Eclipse and VS have been generated/configured with the same cmake variables and have the same behavior? Well, you can inspect the cmake variables but when using presets the author makes sure that the correct variables are passed. For IDEs I think that test presets are optional but I added them for ease of use and completeness. It is convenient to just click a Run Ctest button and get the results inside your IDE. Now for the command line all presets are optional you can still pass -DENABLE_CLANG_TIDY_DEFAULT=false -DCMAKE_C_COMPILER=cl -DCMAKE_CXX_COMPILER=cl -DENABLE_DEVELOPER_MODE=OFF manually. My point is that the minimum requirements are defined by the IDE integration and not by the cli usage. Otherwise, we could claim that presets are useless, then remove the preset file and we all use the command line.

Jason5480 avatar Apr 08 '22 09:04 Jason5480

My point is that the minimum requirements are defined by the IDE integration and not by the cli usage. Otherwise, we could claim that presets are useless, then remove the preset file and we all use the command line.

I love presets, at least the configurePrests, but not more!

ClausKlein avatar Apr 08 '22 11:04 ClausKlein

My point is that the minimum requirements are defined by the IDE integration and not by the cli usage. Otherwise, we could claim that presets are useless, then remove the preset file and we all use the command line.

I love presets, at least the configurePrests, but not more!

Well I agree that it is way simpler to just pass --build and --test option in the cli but I would also like to have the full integration in my IDE. What really bugs me though is that I have to write 200 additional lines to achieve this and the duplication in build and test presets is so obvious. What I really want (and I should be able to write) is two buildPresets (Relese/Debug) that applies to all config presets and one testPreset. The limitation here is that in the current schema the "configurePreset" variable cannot take a list. If that was possible then I could make the presets file way more concise. Since you searched a little bit more about the schema is it possible to change the limitation of the "configurePreset" of build and test Presets in a way that can be understood by the IDEs?

Jason5480 avatar Apr 09 '22 10:04 Jason5480

The only thing that this PR is really lacking is automated testing. I think it would be reasonable to add a separate build step to the .github/workflows/ci.yml file that runs cmake configure, build, and test using the appropriate presets. I don’t think anybody wants to test the presets manually on 3 different OSs over and over again. An automated CLI test should give us a really good idea of whether or not the tests will work on VS Code on a given platform. ...

Nice idea to add those presets to the CI I will try it!

Jason5480 avatar Apr 18 '22 07:04 Jason5480

This is looking pretty good so far. I, like you, am a little disappointed that this couldn’t be shorter, but I don’t see a reasonable way to make that happen (other that generating this file with a script, and I’m not convinced we want that).

The only thing that this PR is really lacking is automated testing. I think it would be reasonable to add a separate build step to the .github/workflows/ci.yml file that runs cmake configure, build, and test using the appropriate presets. I don’t think anybody wants to test the presets manually on 3 different OSs over and over again. An automated CLI test should give us a really good idea of whether or not the tests will work on VS Code on a given platform.

With automated tests, I think this is an easy approval from me.

It's been a long time since we discussed this but I was not able to pass the presets in the ci.yml and action.yml. The problem here is that the compiler, generator, build_type, and developer_mode settings are defined in the ci where this info should be read from the CMakePresets.json file instead if we want to support CMake 3.16. Maybe @aminya or @lefticus can help with that. Otherwise, I will revert the yml changes and I will open another issue for the CI part.

Jason5480 avatar Dec 29 '22 11:12 Jason5480

CMake v3.25 supports Workflow Presets, is this supported b this PR too?

see https://github.com/aminya/cpp_vcpkg_project/pull/20

ClausKlein avatar Feb 05 '23 12:02 ClausKlein