This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "CMake".
The branch, master has been updated via 5107a84d46ad245221843adbef092118649e6dc7 (commit) via 9329e1e30ef4de637d0e71950933f1725f28b475 (commit) via 0779bc9393efeaf1bb3bbf35619617d317d37308 (commit) via f64099cf5e5a659340cf221dd32a6cfa8bfe47d3 (commit) via 95210d027a03192532fcf2c71c98c79fd4cc0fd1 (commit) via d9dd68cb60ed09e32cbda8bbd3e31c86a8778e66 (commit) from 8ea30a44d38057fed134532f6d3b6e8626a1ccff (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=5107a84d46ad245221843adbef092118649e6dc7 commit 5107a84d46ad245221843adbef092118649e6dc7 Merge: 9329e1e f64099c Author: Brad King <brad.k...@kitware.com> AuthorDate: Thu Jan 31 15:50:31 2019 +0000 Commit: Kitware Robot <kwro...@kitware.com> CommitDate: Thu Jan 31 10:50:38 2019 -0500 Merge topic 'restore-install-late-framework' f64099cf5e Merge branch 'backport-restore-install-late-framework' 95210d027a macOS: Restore compatibility for setting FRAMEWORK after install() d9dd68cb60 macOS: Restore compatibility for setting FRAMEWORK after install() Acked-by: Kitware Robot <kwro...@kitware.com> Acked-by: Kyle Edwards <kyle.edwa...@kitware.com> Merge-request: !2878 https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=9329e1e30ef4de637d0e71950933f1725f28b475 commit 9329e1e30ef4de637d0e71950933f1725f28b475 Merge: 8ea30a4 0779bc9 Author: Brad King <brad.k...@kitware.com> AuthorDate: Thu Jan 31 15:45:50 2019 +0000 Commit: Kitware Robot <kwro...@kitware.com> CommitDate: Thu Jan 31 10:45:56 2019 -0500 Merge topic 'readlistfile-stdstring' 0779bc9393 ReadListFile: Accept std::string argument Acked-by: Kitware Robot <kwro...@kitware.com> Merge-request: !2889 https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=0779bc9393efeaf1bb3bbf35619617d317d37308 commit 0779bc9393efeaf1bb3bbf35619617d317d37308 Author: Vitaly Stakhovsky <vvs31...@gitlab.org> AuthorDate: Wed Jan 30 12:19:22 2019 -0500 Commit: Brad King <brad.k...@kitware.com> CommitDate: Thu Jan 31 09:27:54 2019 -0500 ReadListFile: Accept std::string argument diff --git a/Source/cmFindPackageCommand.cxx b/Source/cmFindPackageCommand.cxx index f9ac310..c2e0712 100644 --- a/Source/cmFindPackageCommand.cxx +++ b/Source/cmFindPackageCommand.cxx @@ -693,7 +693,7 @@ bool cmFindPackageCommand::FindModule(bool& found) std::string var = this->Name; var += "_FIND_MODULE"; this->Makefile->AddDefinition(var, "1"); - bool result = this->ReadListFile(mfile.c_str(), DoPolicyScope); + bool result = this->ReadListFile(mfile, DoPolicyScope); this->Makefile->RemoveDefinition(var); return result; } @@ -776,7 +776,7 @@ bool cmFindPackageCommand::HandlePackageMode() this->StoreVersionFound(); // Parse the configuration file. - if (this->ReadListFile(this->FileFound.c_str(), DoPolicyScope)) { + if (this->ReadListFile(this->FileFound, DoPolicyScope)) { // The package has been found. found = true; @@ -1036,7 +1036,8 @@ bool cmFindPackageCommand::FindAppBundleConfig() return false; } -bool cmFindPackageCommand::ReadListFile(const char* f, PolicyScopeRule psr) +bool cmFindPackageCommand::ReadListFile(const std::string& f, + PolicyScopeRule psr) { const bool noPolicyScope = !this->PolicyScope || psr == NoPolicyScope; if (this->Makefile->ReadDependentFile(f, noPolicyScope)) { @@ -1590,7 +1591,7 @@ bool cmFindPackageCommand::CheckVersionFile(std::string const& version_file, // Load the version check file. Pass NoPolicyScope because we do // our own policy push/pop independent of CMP0011. bool suitable = false; - if (this->ReadListFile(version_file.c_str(), NoPolicyScope)) { + if (this->ReadListFile(version_file, NoPolicyScope)) { // Check the output variables. bool okay = this->Makefile->IsOn("PACKAGE_VERSION_EXACT"); bool unsuitable = this->Makefile->IsOn("PACKAGE_VERSION_UNSUITABLE"); diff --git a/Source/cmFindPackageCommand.h b/Source/cmFindPackageCommand.h index 83d8431..a11d253 100644 --- a/Source/cmFindPackageCommand.h +++ b/Source/cmFindPackageCommand.h @@ -110,7 +110,7 @@ private: NoPolicyScope, DoPolicyScope }; - bool ReadListFile(const char* f, PolicyScopeRule psr); + bool ReadListFile(const std::string& f, PolicyScopeRule psr); void StoreVersionFound(); void ComputePrefixes(); diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 2b627ff..8023298 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -437,7 +437,7 @@ bool cmake::SetCacheArgs(const std::vector<std::string>& args) } } std::cout << "loading initial cache file " << path << "\n"; - this->ReadListFile(args, path.c_str()); + this->ReadListFile(args, path); } else if (arg.find("-P", 0) == 0) { i++; if (i >= args.size()) { @@ -451,7 +451,7 @@ bool cmake::SetCacheArgs(const std::vector<std::string>& args) } // Register fake project commands that hint misuse in script mode. GetProjectCommandsInScriptMode(this->State); - this->ReadListFile(args, path.c_str()); + this->ReadListFile(args, path); } else if (arg.find("--find-package", 0) == 0) { findPackageMode = true; } @@ -465,7 +465,7 @@ bool cmake::SetCacheArgs(const std::vector<std::string>& args) } void cmake::ReadListFile(const std::vector<std::string>& args, - const char* path) + const std::string& path) { // if a generator was not yet created, temporarily create one cmGlobalGenerator* gg = this->GetGlobalGenerator(); @@ -478,7 +478,7 @@ void cmake::ReadListFile(const std::vector<std::string>& args, } // read in the list file to fill the cache - if (path) { + if (!path.empty()) { this->CurrentSnapshot = this->State->Reset(); std::string homeDir = this->GetHomeDirectory(); std::string homeOutputDir = this->GetHomeOutputDirectory(); @@ -499,7 +499,7 @@ void cmake::ReadListFile(const std::vector<std::string>& args, mf.SetArgcArgv(args); } if (!mf.ReadListFile(path)) { - cmSystemTools::Error("Error processing file: ", path); + cmSystemTools::Error("Error processing file: " + path); } this->SetHomeDirectory(homeDir); this->SetHomeOutputDirectory(homeOutputDir); @@ -1606,14 +1606,14 @@ void cmake::PreLoadCMakeFiles() if (!pre_load.empty()) { pre_load += "/PreLoad.cmake"; if (cmSystemTools::FileExists(pre_load)) { - this->ReadListFile(args, pre_load.c_str()); + this->ReadListFile(args, pre_load); } } pre_load = this->GetHomeOutputDirectory(); if (!pre_load.empty()) { pre_load += "/PreLoad.cmake"; if (cmSystemTools::FileExists(pre_load)) { - this->ReadListFile(args, pre_load.c_str()); + this->ReadListFile(args, pre_load); } } } @@ -2613,7 +2613,7 @@ int cmake::Build(int jobs, const std::string& dir, const std::string& target, cachePath + "/" + "CMakeFiles/" + "VerifyGlobs.cmake"; if (cmSystemTools::FileExists(globVerifyScript)) { std::vector<std::string> args; - this->ReadListFile(args, globVerifyScript.c_str()); + this->ReadListFile(args, globVerifyScript); } } diff --git a/Source/cmake.h b/Source/cmake.h index af51616..53d44f1 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -463,7 +463,8 @@ protected: std::string GeneratorToolset; ///! read in a cmake list file to initialize the cache - void ReadListFile(const std::vector<std::string>& args, const char* path); + void ReadListFile(const std::vector<std::string>& args, + const std::string& path); bool FindPackage(const std::vector<std::string>& args); ///! Check if CMAKE_CACHEFILE_DIR is set. If it is not, delete the log file. https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=f64099cf5e5a659340cf221dd32a6cfa8bfe47d3 commit f64099cf5e5a659340cf221dd32a6cfa8bfe47d3 Merge: 95210d0 d9dd68c Author: Brad King <brad.k...@kitware.com> AuthorDate: Tue Jan 29 12:54:21 2019 -0500 Commit: Brad King <brad.k...@kitware.com> CommitDate: Wed Jan 30 08:00:23 2019 -0500 Merge branch 'backport-restore-install-late-framework' https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=95210d027a03192532fcf2c71c98c79fd4cc0fd1 commit 95210d027a03192532fcf2c71c98c79fd4cc0fd1 Author: Brad King <brad.k...@kitware.com> AuthorDate: Tue Jan 29 11:54:46 2019 -0500 Commit: Brad King <brad.k...@kitware.com> CommitDate: Wed Jan 30 08:00:06 2019 -0500 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 diff --git a/Source/cmInstallTargetGenerator.cxx b/Source/cmInstallTargetGenerator.cxx index d1d4316..61e5979 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; diff --git a/Tests/RunCMake/Framework/InstallBeforeFramework-stderr.txt b/Tests/RunCMake/Framework/InstallBeforeFramework-stderr.txt new file mode 100644 index 0000000..a3a7c6c --- /dev/null +++ b/Tests/RunCMake/Framework/InstallBeforeFramework-stderr.txt @@ -0,0 +1,7 @@ +^CMake Warning \(dev\) at InstallBeforeFramework.cmake:4 \(install\): + Target 'foo' was changed to a FRAMEWORK sometime after install\(\). This may + result in the wrong install DESTINATION. Set the FRAMEWORK property + earlier. +Call Stack \(most recent call first\): + CMakeLists.txt:[0-9]+ \(include\) +This warning is for project developers. Use -Wno-dev to suppress it. diff --git a/Tests/RunCMake/Framework/InstallBeforeFramework.cmake b/Tests/RunCMake/Framework/InstallBeforeFramework.cmake new file mode 100644 index 0000000..3791dac --- /dev/null +++ b/Tests/RunCMake/Framework/InstallBeforeFramework.cmake @@ -0,0 +1,5 @@ +enable_language(C) + +add_library(foo SHARED foo.c) +install(TARGETS foo LIBRARY DESTINATION lib) +set_property(TARGET foo PROPERTY FRAMEWORK TRUE) diff --git a/Tests/RunCMake/Framework/RunCMakeTest.cmake b/Tests/RunCMake/Framework/RunCMakeTest.cmake index 4fc83f8..e705a31 100644 --- a/Tests/RunCMake/Framework/RunCMakeTest.cmake +++ b/Tests/RunCMake/Framework/RunCMakeTest.cmake @@ -1,5 +1,7 @@ include(RunCMake) +run_cmake(InstallBeforeFramework) + function(framework_layout_test Name Toolchain Type) set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${Toolchain}${Type}FrameworkLayout-build) set(RunCMake_TEST_NO_CLEAN 1) https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=d9dd68cb60ed09e32cbda8bbd3e31c86a8778e66 commit d9dd68cb60ed09e32cbda8bbd3e31c86a8778e66 Author: Brad King <brad.k...@kitware.com> AuthorDate: Tue Jan 29 11:54:46 2019 -0500 Commit: Brad King <brad.k...@kitware.com> CommitDate: Tue Jan 29 12:49:53 2019 -0500 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. Fixes: #18848 diff --git a/Source/cmInstallTargetGenerator.cxx b/Source/cmInstallTargetGenerator.cxx index 8b8f79b..bf0217a 100644 --- a/Source/cmInstallTargetGenerator.cxx +++ b/Source/cmInstallTargetGenerator.cxx @@ -210,8 +210,29 @@ 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. + switch (this->NamelinkMode) { + case NamelinkModeNone: + // Normal case. + break; + case NamelinkModeOnly: + // Assume the NamelinkModeSkip instance will install. + return; + case NamelinkModeSkip: { + // Proceed to install in the LIBRARY destination for compatibility. + } break; + } // Install the whole framework directory. type = cmInstallType_DIRECTORY; diff --git a/Tests/RunCMake/Framework/InstallBeforeFramework.cmake b/Tests/RunCMake/Framework/InstallBeforeFramework.cmake new file mode 100644 index 0000000..3791dac --- /dev/null +++ b/Tests/RunCMake/Framework/InstallBeforeFramework.cmake @@ -0,0 +1,5 @@ +enable_language(C) + +add_library(foo SHARED foo.c) +install(TARGETS foo LIBRARY DESTINATION lib) +set_property(TARGET foo PROPERTY FRAMEWORK TRUE) diff --git a/Tests/RunCMake/Framework/RunCMakeTest.cmake b/Tests/RunCMake/Framework/RunCMakeTest.cmake index 4fc83f8..e705a31 100644 --- a/Tests/RunCMake/Framework/RunCMakeTest.cmake +++ b/Tests/RunCMake/Framework/RunCMakeTest.cmake @@ -1,5 +1,7 @@ include(RunCMake) +run_cmake(InstallBeforeFramework) + function(framework_layout_test Name Toolchain Type) set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/${Toolchain}${Type}FrameworkLayout-build) set(RunCMake_TEST_NO_CLEAN 1) ----------------------------------------------------------------------- Summary of changes: Source/cmFindPackageCommand.cxx | 9 ++++--- Source/cmFindPackageCommand.h | 2 +- Source/cmInstallTargetGenerator.cxx | 31 ++++++++++++++++++++-- Source/cmake.cxx | 16 +++++------ Source/cmake.h | 3 ++- .../Framework/InstallBeforeFramework-stderr.txt | 7 +++++ .../Framework/InstallBeforeFramework.cmake | 5 ++++ Tests/RunCMake/Framework/RunCMakeTest.cmake | 2 ++ 8 files changed, 59 insertions(+), 16 deletions(-) create mode 100644 Tests/RunCMake/Framework/InstallBeforeFramework-stderr.txt create mode 100644 Tests/RunCMake/Framework/InstallBeforeFramework.cmake hooks/post-receive -- CMake _______________________________________________ Cmake-commits mailing list Cmake-commits@cmake.org https://cmake.org/mailman/listinfo/cmake-commits