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

Simplify how to default CMAKE_BUILD_TYPE to Release. #831

Closed
wants to merge 2 commits into from

Conversation

hpkfft
Copy link
Contributor

@hpkfft hpkfft commented Dec 22, 2024

It is simpler to set the VALUE of the existing property to Release.

Note that this cannot be done so easily in nanobind's CMakeLists.txt because the project() specifies LANGUAGES NONE.
Probably, it could be done later in the file, after enable_language(CXX).

See: https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html

@wjakob
Copy link
Owner

wjakob commented Jan 6, 2025

I don't quite understand this change. What is nice with the current incantation (specifying multiple STRINGS) is that it gives a nice drop-down box in MSVC and cmake-gui. It seems to me like this proposed change will lose that feature.

@hpkfft
Copy link
Contributor Author

hpkfft commented Jan 7, 2025

Yeah, I guess you're right.

After project(my_project), the file build/CMakeCache.txt contains:

//Choose the type of build, options are: None Debug Release RelWithDebInfo
// MinSizeRel ...
CMAKE_BUILD_TYPE:STRING=

but cmake-gui ignores the comments. (Though, the comments are useful if one edits the file with a text editor.)

After set(CMAKE_BUILD_TYPE Release CACHE STRING "Choose the type of build." FORCE), it becomes:

//Choose the type of build.
CMAKE_BUILD_TYPE:STRING=Release

Finally, set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug" "Release" "MinSizeRel" "RelWithDebInfo") adds the following towards the bottom of build/CMakeCache.txt:

//STRINGS property for variable: CMAKE_BUILD_TYPE
CMAKE_BUILD_TYPE-STRINGS:INTERNAL=Debug;Release;MinSizeRel;RelWithDebInfo

which is read by cmake-gui to give the nice drop-down menu.

The advice in this PR would change the default value to Release without doing anything else:

//Choose the type of build, options are: None Debug Release RelWithDebInfo
// MinSizeRel ...
CMAKE_BUILD_TYPE:STRING=Release

I suppose one could use two set_property() commands....
I'll make that change to this PR in case you like it better, but honestly, if I had realized that cmake-gui ignores the comments in CMakeCache.txt I would never have made this PR in the first place. Feel free to close this as you like.

@wjakob
Copy link
Owner

wjakob commented Jan 7, 2025

I will close it then. Thanks in any case, I am by no means a CMake expert and it is useful to see other ways of doing things.

@wjakob wjakob closed this Jan 7, 2025
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