This is an archive of the discontinued LLVM Phabricator instance.

[CMake] On Darwin the LIBCXX_INSTALL_HEADERS and LIBCXX_INSTALL_LIBRARY options should default off
AbandonedPublic

Authored by beanz on Dec 2 2015, 1:29 PM.

Details

Summary

The current implementation doesn't work as intended because of CMake's caching behavior. The CMake set command doesn't globally override cached options.

This change removes LIBCXX_OVERRIDE_DARWIN_INSTALL, and instead defaults the options OFF on Darwin. The end result is the same, if you want to install the libraries or headers on Darwin you need to explicitly set the options.

Diff Detail

Event Timeline

beanz updated this revision to Diff 41665.Dec 2 2015, 1:29 PM
beanz retitled this revision from to [CMake] On Darwin the LIBCXX_INSTALL_HEADERS and LIBCXX_INSTALL_LIBRARY options should default off.
beanz updated this object.
beanz added reviewers: mclow.lists, EricWF.
beanz added a subscriber: cfe-commits.
beanz updated this revision to Diff 41668.Dec 2 2015, 1:40 PM

My patch dropped one of the gating checks. This fixes that. It only needs to apply to Darwin when installing to /usr.

EricWF edited edge metadata.Dec 9 2015, 2:54 PM

The current implementation doesn't work as intended because of CMake's caching behavior. The CMake set command doesn't globally override cached options.

Sort of. It doesn't override the value in the cache but it should mean that the cache value is overridden by the newly set non-cache value. If I am correct the following code will print "VAR = OFF".

option(VAR "description" ON)
set(VAR OFF)
message(STATUS "VAR = ${VAR}")

Could you give me an example of where this breaks? It won't override the values in the cache but it should work correctly.

beanz abandoned this revision.Dec 9 2015, 3:13 PM

Short answer. I was doing something terrible and wrong in out-of-tree code, and this was getting tripped up. I was reading the LIBCXX_INSTALL_* variables from a higher-level CMake file. I've stopped doing that (because it was stupid), so I'll abandon this patch.

For context the CMake behavior I was hitting was this:

CMakeLists.txt:

add_subdirectory(foo)
message(STATUS "VAR = ${VAR}") # prints On

foo/CMakeLists.txt:

option(VAR "description" ON)
set(VAR OFF)
message(STATUS "VAR = ${VAR}") # prints OFF

Overriding a cached variable in CMake only overrides it in the current scope and lower.