Page MenuHomePhabricator

build: Enable CMake Policy 0077
ClosedPublic

Authored by compnerd on May 20 2019, 8:51 PM.

Details

Reviewers
beanz
smeenai
Summary

Enable CMake policy 77. This alters the behavior of option. The old behavior would remove the value of the option from the cache and create a new one. The new behavior does not create the variable if it is defined already. This ensures that subsequent reconfigures will behave identically. This seems better than the setting of OLD - the desire is to ensure that it is set to OLD or NEW.

Diff Detail

Repository
rL LLVM

Event Timeline

compnerd created this revision.May 20 2019, 8:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 20 2019, 8:51 PM
Herald added a subscriber: mgorny. · View Herald Transcript

Could you perform a configure before and after this change and confirm that your CMakeCache.txt is the same? If so, this LGTM.

(or if they're different, what the difference is and whether that's desirable)

I think people have learned to overcome the OLD behavior by including CACHE and type in set commands in cache files, but this makes it easier. However, if you want to rerun cmake and change existing values, you'll still need to include CACHE, type, and FORCE, is that right? At least that's how I've been handling it locally. Any advice would be appreciated.

compnerd updated this revision to Diff 200398.May 20 2019, 9:40 PM

@smeenai - not sure what you mean; you mean a clean configuration with the same set of parameters? I'll certainly do that test and expect no differences there. @hintonda - yeah, CACHE ... FORCE should do the trick for that.

@hintonda also, re-running the CMake command line and setting -D... is always a FORCE

This LGTM, but I do want to clarify that your explanation is not correct.

option(...) doesn't remove cached values. If it did, CMake re-generation when you change a build configuration wouldn't work. In the OLD behavior option was implemented roughly as set(${VAR} ${DEFAULT} CACHE BOOL ...). Which if you had a non-cached variable would override it and write it to the cache with the default value. In the NEW behavior if the variable is already set the default value gets replaced with the pre-set value.

The policy documentation explains all the behavior and edge cases pretty well:
https://cmake.org/cmake/help/latest/policy/CMP0077.html

beanz accepted this revision.May 20 2019, 10:09 PM
This revision is now accepted and ready to land.May 20 2019, 10:09 PM
compnerd closed this revision.May 21 2019, 1:26 PM

SVN r361307