This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Properly set CMAKE_BUILD_TYPE to Debug by default
ClosedPublic

Authored by bruening on Feb 18 2016, 10:22 AM.

Details

Summary

PR26666: CMAKE_BUILD_TYPE was previously being reset to blank.
By setting it after project() we ensure that it is not blank.

Diff Detail

Event Timeline

bruening updated this revision to Diff 48353.Feb 18 2016, 10:22 AM
bruening retitled this revision from to [CMake] Properly set CMAKE_BUILD_TYPE to Debug by default.
bruening updated this object.
bruening added reviewers: beanz, rnk.
bruening added a subscriber: llvm-commits.
beanz requested changes to this revision.Feb 18 2016, 10:31 AM
beanz edited edge metadata.

Actually I don't think this is the right solution.

The CMAKE_BUILD_TYPE variable needs to be cached. Setting it can stay where it is as long as the set command caches.
Changing:

set(CMAKE_BUILD_TYPE "Debug")

-to-

set(CMAKE_BUILD_TYPE "Debug" CACHE STRING "Build type (default Debug)")

Should fix the bug.

This revision now requires changes to proceed.Feb 18 2016, 10:31 AM

Agreed, it may be best if it's cached: other projects have hit issues with having to force-cache it for Ninja generators (https://github.com/DynamoRIO/dynamorio/issues/919). For some reason in this other project we're not caching it for non-Ninja but it is not clear why: https://github.com/DynamoRIO/dynamorio/blob/master/CMakeLists.txt#L132

bruening updated this revision to Diff 48362.Feb 18 2016, 10:55 AM
bruening edited edge metadata.

Switch to caching CMAKE_BUILD_TYPE.

beanz accepted this revision.Feb 18 2016, 11:26 AM
beanz edited edge metadata.

LGTM!

Thanks for investigating and fixing this!

This revision is now accepted and ready to land.Feb 18 2016, 11:26 AM

I am not yet a committer: could you commit the patch? Thank you.

This revision was automatically updated to reflect the committed changes.
beanz added a subscriber: beanz.Feb 18 2016, 3:12 PM

Committed in r261273.

Thanks,
-Chris