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: Enable configuration of CMAKE_BUILD_TYPE #239

Merged
merged 1 commit into from
Dec 24, 2024
Merged

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Dec 19, 2024

msh3 used to hardcode the value of CMAKE_BUILD_TYPE to Release, in the middle of CMakeLists.txt. This is unexpected and a source of problems.

  • In general, CMAKE_BUILD_TYPE is a variable which is meant to be controlled by the user. Also package managers like vcpkg make use of this as a general input variable.
  • Setting a normal variable to a hard coded variable in the middle of the build script will hide the user input from this point. But users may easily overlook the trigger for behavioral inconsistencies they observe for different parts of the build.
  • CMake encourages to use a different value than the default empty string.
    The natural way to implement defaults for user controlled variables is using cache variables.

This PR aligns msh3 with common practice: It sets a default early in CMakeLists.txt, as a cache variable subject to user control.

@nibanks
Copy link
Owner

nibanks commented Dec 19, 2024

Can you please provide more description as to why this is necessary?

@dg0yt
Copy link
Contributor Author

dg0yt commented Dec 24, 2024

Updated description.

@nibanks nibanks merged commit 28a6cd5 into nibanks:main Dec 24, 2024
21 checks passed
@dg0yt dg0yt deleted the patch-2 branch December 25, 2024 09:14
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.

2 participants