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

Set default build type to DebugRelease #5202

Merged
merged 1 commit into from Jul 8, 2023

Conversation

gassmoeller
Copy link
Member

This sets the ASPECT default build type to DebugRelease (just like deal.II). This creates two executables in the build directory. The main reason I think this is more useful because most users will need both executables eventually (debug for testing, release for actually running models) and I keep encountering users that either:

  • never run debug mode, because they dont want to recompile before running the actual model
  • or dont know that they are running debug mode (because it is the default) and are unaware there is a faster option available.

I hope with this change we can reduce these confusions (at the cost of slower compile time).

Comment on lines +41 to +49
#
# Setup CMAKE_BUILD_TYPE:
#

SET(CMAKE_BUILD_TYPE
"DebugRelease"
CACHE STRING
"Choose the type of build, options are: Debug, Release and DebugRelease."
)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a variable in the cmake namespace. Isn't it defined by default?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is, but the default documentation says that only 'Debug' and 'Release' are allowed options. This way I overwrite the documentation with the additional option. I copied this code from https://github.com/dealii/dealii/blob/master/cmake/setup_cached_variables.cmake do you want me to make the same change there too?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. I trust @tamiko that it is ok to overwrite this variable.

And yes, it would of course be fantastic if you could make the same changes there as well!

CMakeLists.txt Outdated
NOT "${CMAKE_BUILD_TYPE}" STREQUAL "Debug" AND
NOT "${CMAKE_BUILD_TYPE}" STREQUAL "DebugRelease" )
MESSAGE(FATAL_ERROR
"CMAKE_BUILD_TYPE does neither match Release, Debug, nor DebugRelease!"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"CMAKE_BUILD_TYPE does neither match Release, Debug, nor DebugRelease!"
"CMAKE_BUILD_TYPE must either be 'Release', 'Debug', nor 'DebugRelease', but is set to '${CMAKE_BUILD_TYPE}'."

Comment on lines 8 to 9
(Rene Gassmoeller, Timo Heister, 2023/07/07)

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
(Rene Gassmoeller, Timo Heister, 2023/07/07)
(Rene Gassmoeller, Timo Heister, 2023/07/07)

@gassmoeller
Copy link
Member Author

I included the suggestions.

@tjhei tjhei merged commit 62d846c into geodynamics:main Jul 8, 2023
6 checks passed
@tjhei
Copy link
Member

tjhei commented Jul 8, 2023

Thank you. I agree and made the change on my machine since I introduced it.

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

3 participants