Create CMake preset by wusatosi · Pull Request #44 · bemanproject/exemplar

wusatosi · 2024-10-04T22:10:52Z

closes #17 .

This is a work in process seeking feedback, currently only included gcc related workflows.
Needs feedback on which compiler and what flags I should include/ exclude.
Let's keep the list of presets short (#17 (comment)) so we don't strain the CI system too much.

I omitted -Werror in #5 as this flag isn't indicated elsewhere (e.g. README).

There's two workflow presets:

gcc-debug, (-g with all non-conflicting sanitizer included, see: Add CMake preset support #17 (comment))
gcc-release, (-g with -O3)

Assumes C++ 20 per: #38 (comment) , but will monitor the progress happening at #38 . Please forward minimum version discussion to that thread unless it should be specific to a workflow.

CI test is only meant to test if the workflow config is functional, main testing should be done in the main build & test matrix, thus there's no cross-platform test. We could merge the two but that will need future discussion.

This is also a good peak into docker-less build & test for CI.

Please test this locally, I have experienced the test process hanging, possibly due to the excessive amount of sanitizer enabled. We may need to include a warning in the README to advise turning off/ splitting specific performance-impacting sanitizer on bigger projects.

Related pr: #7

bretbrownjr

You can use variables like CMAKE_CXX_FLAGS_Debug_INIT to tune what a Debug or Release build entails, for what it's worth. It's maybe a little nicer to use those variables instead of CMAKE_CXX_FLAGS. But not a huge deal.

https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_FLAGS_CONFIG_INIT.html

bretbrownjr · 2024-10-14T16:32:04Z

It could be a separate PR, but I expect that especially for introducing a new, friendly workflow, we'll want to make sure we have the workflow documented for potential contributors and users.

camio

I tried this on an ubuntu machine and it's a good improvement.

In testing this I used cmake --list-presets, cmake --workflow --preset gcc-release, and cmake --workflow --preset gcc-debug. All worked as expected on an Ubuntu VM.

I tried this on MacOS and got an error. The reason is that MacOS has a gcc executable in the path that is actually clang under the hood...one that doesn't support the -fsanitize=leak option. Clang presets will help mitigate this issue, but it's good to be aware of in case we want to attempt better error messages in the future.

wusatosi · 2024-10-14T19:03:00Z

I tried this on an ubuntu machine and it's a good improvement.

In testing this I used cmake --list-presets, cmake --workflow --preset gcc-release, and cmake --workflow --preset gcc-debug. All worked as expected on an Ubuntu VM.

I tried this on MacOS and got an error. The reason is that MacOS has a gcc executable in the path that is actually clang under the hood...one that doesn't support the -fsanitize=leak option. Clang presets will help mitigate this issue, but it's good to be aware of in case we want to attempt better error messages in the future.

Thank you for looking over this PR. Should I include this in the documentation?

camio · 2024-10-14T19:35:05Z

I tried this on MacOS and got an error.

Should I include this in the documentation?

I don't think that's necessary. Let's see if others run into the same issue first. It feels kinda niche to me, especially if we include a clang preset.

wusatosi · 2024-10-15T00:33:55Z

It could be a separate PR, but I expect that especially for introducing a new, friendly workflow, we'll want to make sure we have the workflow documented for potential contributors and users.

What would that workflow be different from gcc-debug?

wusatosi · 2024-10-15T00:36:54Z

[It's not necessary to document failure to run gcc-debug on macos due to symlinking gcc with clang]

Noted! I've updated the documentation.
This pr is ready to merge if only this two presets is desired.

I could add clang-debug/release to this pr too, or I could also do this via another PR.

steve-downey · 2024-10-15T01:19:08Z

+    {
+      "name": "gcc-debug",
+      "inherits": "_test_base",
+      "configurePreset": "gcc-debug"


Not for this PR, but it just occurred to me that we might leverage the ninja-multiconfig generator to keep the list of configurePresets small? That is have one that configures Debug, Release, and possibly various sanitizer flavors? This might help keep the combinatorics down.

camio · 2024-10-15T20:20:30Z

+cmake --workflow --preset gcc-debug
 cmake --install build --prefix /opt/beman.exemplar
 ```



Note that the "verbose logs" sections below need to be updated as well.

On the other hand, I'd be happier to see them removed entirely. I've used beman.exemplar for a few different projects now and it's a pain to have to update those sections.

The "Disable tests build" section also needs to be updated.

Thank you for double checking! Sorry I missed those sections. I will have them updated later today. !!

Updated cmake command and output in 8430517 .

Co-authored-by: David Sankel <[email protected]>

wusatosi · 2024-10-19T19:43:12Z

In the README, we includes support for turning off build test. However, cmake does not support passing additional parameters to workflows till (late) 3.25.

How should I update the documentations?
Should I

Remove the section entirely
Add a no build test variant to the preset
Indicate that workflow will not support this, and include commands to manually build the project with BUILD_TEST flag?

@camio , cc @bretbrownjr as that paragraph was introduced in #15 .

JeffGarland · 2024-10-21T16:50:32Z

This looks like it's ready to close?

wusatosi · 2024-10-21T16:56:22Z

This looks like it's ready to close?

There's a pending documentation change, see above comment.

JeffGarland · 2024-10-21T17:14:16Z

There's a lot of PR's open here so hopefully we can close them soon :)

wusatosi · 2024-10-21T17:17:51Z

There's a lot of PR's open here so hopefully we can close them soon :)

#48 should be ready, can you review and merge it?

camio · 2024-10-21T18:03:38Z

I think this is ready to merge.

How should I update the documentations?
Should I

Remove the section entirely

I think this is a good idea. Can be done in a different PR.

Add a no build test variant to the preset

I don't think we need a preset for this as it is a specialized need.

Indicate that workflow will not support this, and include commands to manually build the project with BUILD_TEST flag?

I think this only needs to be discussed in a "how to incorporate in another project" section which needs more work anyway (e.g. a FetchContent example)

wusatosi added 2 commits October 4, 2024 17:28

Introduce basic CMakePreset

d581a23

update CI

af0660e

wusatosi requested review from bretbrownjr, camio, dietmarkuehl, neatudarius and steve-downey as code owners October 4, 2024 22:10

wusatosi mentioned this pull request Oct 4, 2024

Add CMake preset support #17

Closed

wusatosi changed the title ~~Cmake preset~~ Create CMake preset Oct 5, 2024

bretbrownjr approved these changes Oct 14, 2024

View reviewed changes

Comment thread CMakePresets.json Outdated

Comment thread CMakePresets.json Outdated

remove redundant flags

c46ee33

camio approved these changes Oct 14, 2024

View reviewed changes

wusatosi added 2 commits October 14, 2024 20:32

minor CI format update

a42d679

update README

6b92c73

steve-downey reviewed Oct 15, 2024

View reviewed changes

wusatosi mentioned this pull request Oct 15, 2024

Add vcpkg configuration files #7

Closed

wusatosi commented Oct 15, 2024

View reviewed changes

Comment thread CMakePresets.json

camio reviewed Oct 15, 2024

View reviewed changes

wusatosi and others added 4 commits October 18, 2024 22:17

Update README.md

85d5c39

Co-authored-by: David Sankel <[email protected]>

update README

8430517

Merge branch 'main' into cmake-preset

a887289

fix lint

d102176

camio approved these changes Oct 21, 2024

View reviewed changes

camio merged commit 10cd0a5 into bemanproject:main Oct 21, 2024

This was referenced Nov 8, 2024

clang++: error: unsupported option '-fsanitize=leak' for target 'x86_64-apple-darwin23.6.0' #65

Closed

Introduce CMake toolchain #76

Merged

Conversation

wusatosi commented Oct 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bretbrownjr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bretbrownjr commented Oct 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

camio left a comment

Choose a reason for hiding this comment

Uh oh!

wusatosi commented Oct 14, 2024

Uh oh!

camio commented Oct 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wusatosi commented Oct 15, 2024

Uh oh!

wusatosi commented Oct 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steve-downey Oct 15, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

camio Oct 15, 2024

Choose a reason for hiding this comment

Uh oh!

wusatosi Oct 17, 2024

Choose a reason for hiding this comment

Uh oh!

wusatosi Oct 19, 2024

Choose a reason for hiding this comment

Uh oh!

wusatosi commented Oct 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JeffGarland commented Oct 21, 2024

Uh oh!

wusatosi commented Oct 21, 2024

Uh oh!

JeffGarland commented Oct 21, 2024

Uh oh!

wusatosi commented Oct 21, 2024

Uh oh!

camio commented Oct 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wusatosi commented Oct 4, 2024 •

edited

Loading

bretbrownjr commented Oct 14, 2024 •

edited

Loading

camio commented Oct 14, 2024 •

edited

Loading

wusatosi commented Oct 15, 2024 •

edited

Loading

wusatosi commented Oct 19, 2024 •

edited

Loading