From 95210d027a03192532fcf2c71c98c79fd4cc0fd1 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 29 Jan 2019 11:54:46 -0500 Subject: macOS: Restore compatibility for setting FRAMEWORK after install() The `FRAMEWORK` target property affects the way the `install()` command treats the target and so should be set first. Our implementation assumed that this was always the case and led to an assertion failure. Prior to CMake 3.12 this was visible only when using an explicit `LIBRARY ... NAMELINK_ONLY` option, but commit 0212d7c762 (install: add NAMELINK_COMPONENT argument, 2018-04-18, v3.12.0-rc1~139^2~3) made it possible with a simple `LIBRARY DESTINATION`. Fully supporting out-of-order specification will require non-trivial refactoring to defer install generator creation to generate time. For now simply restore the old behavior of installing the framework to the library destination and warn about the case. Fixes: #18848 --- Source/cmInstallTargetGenerator.cxx | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) (limited to 'Source/cmInstallTargetGenerator.cxx') diff --git a/Source/cmInstallTargetGenerator.cxx b/Source/cmInstallTargetGenerator.cxx index d1d431653f..61e5979b73 100644 --- a/Source/cmInstallTargetGenerator.cxx +++ b/Source/cmInstallTargetGenerator.cxx @@ -19,6 +19,7 @@ #include "cmStateTypes.h" #include "cmSystemTools.h" #include "cmTarget.h" +#include "cmake.h" cmInstallTargetGenerator::cmInstallTargetGenerator( std::string targetName, const char* dest, bool implib, @@ -211,8 +212,34 @@ void cmInstallTargetGenerator::GenerateScriptForConfig( // An import library looks like a static library. type = cmInstallType_STATIC_LIBRARY; } else if (this->Target->IsFrameworkOnApple()) { - // There is a bug in cmInstallCommand if this fails. - assert(this->NamelinkMode == NamelinkModeNone); + // FIXME: In principle we should be able to + // assert(this->NamelinkMode == NamelinkModeNone); + // but since the current install() command implementation checks + // the FRAMEWORK property immediately instead of delaying until + // generate time, it is possible for project code to set the + // property after calling install(). In such a case, the install() + // command will use the LIBRARY code path and create two install + // generators, one for the namelink component (NamelinkModeOnly) + // and one for the primary artifact component (NamelinkModeSkip). + // Historically this was not diagnosed and resulted in silent + // installation of a framework to the LIBRARY destination. + // Retain that behavior and warn about the case. + switch (this->NamelinkMode) { + case NamelinkModeNone: + // Normal case. + break; + case NamelinkModeOnly: + // Assume the NamelinkModeSkip instance will warn and install. + return; + case NamelinkModeSkip: { + std::string e = "Target '" + this->Target->GetName() + + "' was changed to a FRAMEWORK sometime after install(). " + "This may result in the wrong install DESTINATION. " + "Set the FRAMEWORK property earlier."; + this->Target->GetGlobalGenerator()->GetCMakeInstance()->IssueMessage( + MessageType::AUTHOR_WARNING, e, this->GetBacktrace()); + } break; + } // Install the whole framework directory. type = cmInstallType_DIRECTORY; -- cgit v1.2.1