Merge topic 'escape-install-rpath'

4caefbb423 cmInstallTargetGenerator: Add tests for the RPATH_CHANGE rule
749ce48eb5 cmInstallTargetGenerator: Escape generated OLD_RPATH argument
9e84c7c5e8 cmInstallTargetGenerator: Introduce CMP0095

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !3383

3 files changed, 76 insertions, 11 deletions

diff --git a/Source/cmInstallTargetGenerator.cxx b/Source/cmInstallTargetGenerator.cxx
index 7c5a55b..0774436 100644
--- a/Source/cmInstallTargetGenerator.cxx
+++ b/Source/cmInstallTargetGenerator.cxx
@@ -16,6 +16,8 @@
 #include "cmLocalGenerator.h"
 #include "cmMakefile.h"
 #include "cmMessageType.h"
+#include "cmOutputConverter.h"
+#include "cmPolicies.h"
 #include "cmStateTypes.h"
 #include "cmSystemTools.h"
 #include "cmTarget.h"
@@ -632,17 +634,34 @@ void cmInstallTargetGenerator::AddRPathCheckRule(
     return;
   }
 
-  // Get the install RPATH from the link information.
-  std::string newRpath = cli->GetChrpathString();
-
   // Write a rule to remove the installed file if its rpath is not the
   // new rpath.  This is needed for existing build/install trees when
   // the installed rpath changes but the file is not rebuilt.
-  /* clang-format off */
   os << indent << "file(RPATH_CHECK\n"
-     << indent << "     FILE \"" << toDestDirPath << "\"\n"
-     << indent << "     RPATH \"" << newRpath << "\")\n";
-  /* clang-format on */
+     << indent << "     FILE \"" << toDestDirPath << "\"\n";
+
+  // CMP0095: ``RPATH`` entries are properly escaped in the intermediary
+  // CMake install script.
+  switch (this->Target->GetPolicyStatusCMP0095()) {
+    case cmPolicies::WARN:
+      // No author warning needed here, we warn later in
+      // cmInstallTargetGenerator::AddChrpathPatchRule().
+      CM_FALLTHROUGH;
+    case cmPolicies::OLD: {
+      // Get the install RPATH from the link information.
+      std::string newRpath = cli->GetChrpathString();
+      os << indent << "     RPATH \"" << newRpath << "\")\n";
+      break;
+    }
+    default: {
+      // Get the install RPATH from the link information and
+      // escape any CMake syntax in the install RPATH.
+      std::string escapedNewRpath =
+        cmOutputConverter::EscapeForCMake(cli->GetChrpathString());
+      os << indent << "     RPATH " << escapedNewRpath << ")\n";
+      break;
+    }
+  }
 }
 
 void cmInstallTargetGenerator::AddChrpathPatchRule(
@@ -731,11 +750,28 @@ void cmInstallTargetGenerator::AddChrpathPatchRule(
       return;
     }
 
+    // Escape any CMake syntax in the RPATHs.
+    std::string escapedOldRpath = cmOutputConverter::EscapeForCMake(oldRpath);
+    std::string escapedNewRpath = cmOutputConverter::EscapeForCMake(newRpath);
+
     // Write a rule to run chrpath to set the install-tree RPATH
     os << indent << "file(RPATH_CHANGE\n"
        << indent << "     FILE \"" << toDestDirPath << "\"\n"
-       << indent << "     OLD_RPATH \"" << oldRpath << "\"\n"
-       << indent << "     NEW_RPATH \"" << newRpath << "\")\n";
+       << indent << "     OLD_RPATH " << escapedOldRpath << "\n";
+
+    // CMP0095: ``RPATH`` entries are properly escaped in the intermediary
+    // CMake install script.
+    switch (this->Target->GetPolicyStatusCMP0095()) {
+      case cmPolicies::WARN:
+        this->IssueCMP0095Warning(newRpath);
+        CM_FALLTHROUGH;
+      case cmPolicies::OLD:
+        os << indent << "     NEW_RPATH \"" << newRpath << "\")\n";
+        break;
+      default:
+        os << indent << "     NEW_RPATH " << escapedNewRpath << ")\n";
+        break;
+    }
   }
 }
 
@@ -838,3 +874,26 @@ void cmInstallTargetGenerator::AddUniversalInstallRule(
      << "\"" << this->Target->Target->GetName() << "\" "
      << "\"" << toDestDirPath << "\")\n";
 }
+
+void cmInstallTargetGenerator::IssueCMP0095Warning(
+  const std::string& unescapedRpath)
+{
+  // Reduce warning noise to cases where used RPATHs may actually be affected
+  // by CMP0095. This filter is meant to skip warnings in cases when
+  // non-curly-braces syntax (e.g. $ORIGIN) or no keyword is used which has
+  // worked already before CMP0095. We intend to issue a warning in all cases
+  // with curly-braces syntax, even if the workaround of double-escaping is in
+  // place, since we deprecate the need for it with CMP0095.
+  const bool potentially_affected(unescapedRpath.find("${") !=
+                                  std::string::npos);
+
+  if (potentially_affected) {
+    std::ostringstream w;
+    w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0095) << "\n";
+    w << "RPATH entries for target '" << this->Target->GetName() << "' "
+      << "will not be escaped in the intermediary "
+      << "cmake_install.cmake script.";
+    this->Target->GetGlobalGenerator()->GetCMakeInstance()->IssueMessage(
+      MessageType::AUTHOR_WARNING, w.str(), this->GetBacktrace());
+  }
+}
diff --git a/Source/cmInstallTargetGenerator.h b/Source/cmInstallTargetGenerator.h
index ed3ab52..9ccad63 100644
--- a/Source/cmInstallTargetGenerator.h
+++ b/Source/cmInstallTargetGenerator.h
@@ -104,6 +104,7 @@ protected:
                      const std::string& toDestDirPath);
   void AddUniversalInstallRule(std::ostream& os, Indent indent,
                                const std::string& toDestDirPath);
+  void IssueCMP0095Warning(const std::string& unescapedRpath);
 
   std::string TargetName;
   cmGeneratorTarget* Target;
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index b705119..c16a46f 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -279,7 +279,11 @@ class cmMakefile;
   SELECT(POLICY, CMP0094,                                                     \
          "FindPython3,  FindPython2 and FindPyton use "                       \
          "LOCATION for lookup strategy.",                                     \
-         3, 15, 0, cmPolicies::WARN)
+         3, 15, 0, cmPolicies::WARN)                                          \
+  SELECT(POLICY, CMP0095,                                                     \
+         "RPATH entries are properly escaped in the intermediary CMake "      \
+         "install script.",                                                   \
+         3, 16, 0, cmPolicies::WARN)
 
 #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
 #define CM_FOR_EACH_POLICY_ID(POLICY)                                         \
@@ -307,7 +311,8 @@ class cmMakefile;
   F(CMP0073)                                                                  \
   F(CMP0076)                                                                  \
   F(CMP0081)                                                                  \
-  F(CMP0083)
+  F(CMP0083)                                                                  \
+  F(CMP0095)
 
 /** \class cmPolicies
  * \brief Handles changes in CMake behavior and policies