[Cmake-commits] CMake branch, master, updated. v3.13.3-1221-g5107a84

Kitware Robot kwrobot at kitware.com
Thu Jan 31 10:53:05 EST 2019


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.king at kitware.com>
AuthorDate: Thu Jan 31 15:50:31 2019 +0000
Commit:     Kitware Robot <kwrobot at 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 <kwrobot at kitware.com>
    Acked-by: Kyle Edwards <kyle.edwards at 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.king at kitware.com>
AuthorDate: Thu Jan 31 15:45:50 2019 +0000
Commit:     Kitware Robot <kwrobot at 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 <kwrobot at kitware.com>
    Merge-request: !2889


https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=0779bc9393efeaf1bb3bbf35619617d317d37308
commit 0779bc9393efeaf1bb3bbf35619617d317d37308
Author:     Vitaly Stakhovsky <vvs31415 at gitlab.org>
AuthorDate: Wed Jan 30 12:19:22 2019 -0500
Commit:     Brad King <brad.king at 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.king at kitware.com>
AuthorDate: Tue Jan 29 12:54:21 2019 -0500
Commit:     Brad King <brad.king at 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.king at kitware.com>
AuthorDate: Tue Jan 29 11:54:46 2019 -0500
Commit:     Brad King <brad.king at 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.king at kitware.com>
AuthorDate: Tue Jan 29 11:54:46 2019 -0500
Commit:     Brad King <brad.king at 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


More information about the Cmake-commits mailing list