ENH: Cleanup builtin chrpath support

  - Move computation of extended build-tree rpath
    to cmComputeLinkInformation
  - Only enable the extended build-tree rpath if
    the target will be installed
  - Generalize the interface of file(CHRPATH)
  - When changing the rpath on installation only
    replace the part generated by CMake because
    the native tools (ex SunCC on Linux) might have
    added their own part to the rpath

7 files changed, 131 insertions, 32 deletions

diff --git a/Source/cmComputeLinkInformation.cxx b/Source/cmComputeLinkInformation.cxx
index 554d8de..a3f8bab 100644
--- a/Source/cmComputeLinkInformation.cxx
+++ b/Source/cmComputeLinkInformation.cxx
@@ -1463,6 +1463,18 @@ std::string cmComputeLinkInformation::GetRPathString(bool for_install)
     // Add this path.
     rpath += *ri;
     }
+
+  // If the rpath will be replaced at install time make sure it is
+  // long enough now.
+  if(!for_install && this->RuntimeUseChrpath)
+    {
+    std::string::size_type minLength = this->GetChrpathString().length();
+    while(rpath.length() < minLength)
+      {
+      rpath += this->GetRuntimeSep();
+      }
+    }
+
   return rpath;
 }
 
diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx
index 0128135..8868812 100644
--- a/Source/cmFileCommand.cxx
+++ b/Source/cmFileCommand.cxx
@@ -1333,25 +1333,81 @@ bool cmFileCommand::HandleInstallDestination(cmFileInstaller& installer,
 //----------------------------------------------------------------------------
 bool cmFileCommand::HandleChrpathCommand(std::vector<std::string> const& args)
 {
-  if(args.size() != 3)
+  // Evaluate arguments.
+  const char* file = 0;
+  const char* oldRPath = 0;
+  const char* newRPath = 0;
+  enum Doing { DoingNone, DoingFile, DoingOld, DoingNew };
+  Doing doing = DoingNone;
+  for(unsigned int i=1; i < args.size(); ++i)
+    {
+    if(args[i] == "OLD_RPATH")
+      {
+      doing = DoingOld;
+      }
+    else if(args[i] == "NEW_RPATH")
+      {
+      doing = DoingNew;
+      }
+    else if(args[i] == "FILE")
+      {
+      doing = DoingFile;
+      }
+    else if(doing == DoingFile)
+      {
+      file = args[i].c_str();
+      doing = DoingNone;
+      }
+    else if(doing == DoingOld)
+      {
+      oldRPath = args[i].c_str();
+      doing = DoingNone;
+      }
+    else if(doing == DoingNew)
+      {
+      newRPath = args[i].c_str();
+      doing = DoingNone;
+      }
+    else
+      {
+      cmOStringStream e;
+      e << "CHRPATH given unknown argument " << args[i];
+      this->SetError(e.str().c_str());
+      return false;
+      }
+    }
+  if(!file)
+    {
+    this->SetError("CHRPATH not given FILE option.");
+    return false;
+    }
+  if(!oldRPath)
     {
-    this->SetError("CHRPATH must be given a file and a new rpath.");
+    this->SetError("CHRPATH not given OLD_RPATH option.");
     return false;
     }
-  if(!cmSystemTools::FileExists(args[1].c_str(), true))
+  if(!newRPath)
     {
-    this->SetError("CHRPATH given file that does not exist.");
+    this->SetError("CHRPATH not given NEW_RPATH option.");
+    return false;
+    }
+  if(!cmSystemTools::FileExists(file, true))
+    {
+    cmOStringStream e;
+    e << "CHRPATH given FILE \"" << file << "\" that does not exist.";
+    this->SetError(e.str().c_str());
     return false;
     }
   std::string emsg;
-  if(cmSystemTools::ChangeRPath(args[1], args[2], &emsg))
+  if(cmSystemTools::ChangeRPath(file, oldRPath, newRPath, &emsg))
     {
     return true;
     }
   else
     {
     cmOStringStream e;
-    e << "CHRPATH could not write new RPATH to the file: "
+    e << "CHRPATH could not write new RPATH \""
+      << newRPath << "\" to the file \"" << file << "\": "
       << emsg;
     this->SetError(e.str().c_str());
     return false;
diff --git a/Source/cmInstallTargetGenerator.cxx b/Source/cmInstallTargetGenerator.cxx
index c2d606e..9ecf346 100644
--- a/Source/cmInstallTargetGenerator.cxx
+++ b/Source/cmInstallTargetGenerator.cxx
@@ -552,15 +552,8 @@ cmInstallTargetGenerator
 ::AddChrpathPatchRule(std::ostream& os, Indent const& indent,
                       const char* config, std::string const& toDestDirPath)
 {
-  if(this->ImportLibrary ||
-     !(this->Target->GetType() == cmTarget::SHARED_LIBRARY ||
-       this->Target->GetType() == cmTarget::MODULE_LIBRARY ||
-       this->Target->GetType() == cmTarget::EXECUTABLE))
-    {
-    return;
-    }
-
-  if(!this->Target->IsChrpathUsed())
+  // Skip the chrpath if the target does not need it.
+  if(this->ImportLibrary || !this->Target->IsChrpathUsed())
     {
     return;
     }
@@ -573,12 +566,16 @@ cmInstallTargetGenerator
     return;
     }
 
+  // Construct the original rpath string to be replaced.
+  std::string oldRpath = cli->GetRPathString(false);
+
   // Get the install RPATH from the link information.
   std::string newRpath = cli->GetChrpathString();
 
   // Write a rule to run chrpath to set the install-tree RPATH
-  os << indent
-     << "FILE(CHRPATH \"" << toDestDirPath << "\" \"" << newRpath << "\")\n";
+  os << indent << "FILE(CHRPATH FILE \"" << toDestDirPath << "\"\n"
+     << indent << "     OLD_RPATH \"" << oldRpath << "\"\n"
+     << indent << "     NEW_RPATH \"" << newRpath << "\")\n";
 }
 
 //----------------------------------------------------------------------------
diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx
index 5fca1f3..f96704f 100644
--- a/Source/cmLocalGenerator.cxx
+++ b/Source/cmLocalGenerator.cxx
@@ -1591,17 +1591,6 @@ void cmLocalGenerator::OutputLinkLibraries(std::ostream& fout,
     // All rpath entries are combined ("-Wl,-rpath,a:b:c").
     std::string rpath = cli.GetRPathString(relink);
 
-    // If not relinking, make sure the rpath string is long enough to
-    // support a subsequent chrpath on installation.
-    if(!relink)
-      {
-      std::string::size_type minLength = cli.GetChrpathString().size();
-      while(rpath.size() < minLength)
-        {
-        rpath += cli.GetRuntimeSep();
-        }
-      }
-
     // Store the rpath option in the stream.
     if(!rpath.empty())
       {
diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx
index fb41834..dbcf775 100644
--- a/Source/cmSystemTools.cxx
+++ b/Source/cmSystemTools.cxx
@@ -2198,18 +2198,41 @@ bool cmSystemTools::GuessLibrarySOName(std::string const& fullPath,
 
 //----------------------------------------------------------------------------
 bool cmSystemTools::ChangeRPath(std::string const& file,
+                                std::string const& oldRPath,
                                 std::string const& newRPath,
                                 std::string* emsg)
 {
 #if defined(CMAKE_USE_ELF_PARSER)
   unsigned long rpathPosition = 0;
   unsigned long rpathSize = 0;
+  std::string rpathSuffix;
   {
   cmELF elf(file.c_str());
   if(cmELF::StringEntry const* se = elf.GetRPath())
     {
+    // Make sure the current rpath begins with the old rpath.
+    if(se->Value.length() < oldRPath.length() ||
+       se->Value.substr(0, oldRPath.length()) != oldRPath)
+      {
+      // If it begins with the new rpath instead then it is okay.
+      if(se->Value.length() >= newRPath.length() &&
+         se->Value.substr(0, newRPath.length()) == newRPath)
+        {
+        return true;
+        }
+      if(emsg)
+        {
+        *emsg = "The current RPATH does not begin with that specified.";
+        }
+      return false;
+      }
+
+    // Store information about the entry.
     rpathPosition = se->Position;
     rpathSize = se->Size;
+
+    // Store the part of the path we must preserve.
+    rpathSuffix = se->Value.substr(oldRPath.length(), oldRPath.npos);
     }
   else if(newRPath.empty())
     {
@@ -2221,14 +2244,19 @@ bool cmSystemTools::ChangeRPath(std::string const& file,
     {
     if(emsg)
       {
-      *emsg = "No valid ELF RPATH entry exists in the file.";
+      *emsg = "No valid ELF RPATH entry exists in the file; ";
+      *emsg += elf.GetErrorMessage();
       }
     return false;
     }
   }
+  // Compute the full new rpath.
+  std::string rpath = newRPath;
+  rpath += rpathSuffix;
+
   // Make sure there is enough room to store the new rpath and at
   // least one null terminator.
-  if(rpathSize < newRPath.length()+1)
+  if(rpathSize < rpath.length()+1)
     {
     if(emsg)
       {
@@ -2259,8 +2287,8 @@ bool cmSystemTools::ChangeRPath(std::string const& file,
 
   // Write the new rpath.  Follow it with enough null terminators to
   // fill the string table entry.
-  f << newRPath;
-  for(unsigned long i=newRPath.length(); i < rpathSize; ++i)
+  f << rpath;
+  for(unsigned long i=rpath.length(); i < rpathSize; ++i)
     {
     f << '\0';
     }
@@ -2280,6 +2308,7 @@ bool cmSystemTools::ChangeRPath(std::string const& file,
     }
 #else
   (void)file;
+  (void)oldRPath;
   (void)newRPath;
   (void)emsg;
   return false;
diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h
index 42282d3..a7abc5f 100644
--- a/Source/cmSystemTools.h
+++ b/Source/cmSystemTools.h
@@ -383,6 +383,7 @@ public:
 
   /** Try to set the RPATH in an ELF binary.  */
   static bool ChangeRPath(std::string const& file,
+                          std::string const& oldRPath,
                           std::string const& newRPath,
                           std::string* emsg = 0);
 
diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx
index c14e527..55947e2 100644
--- a/Source/cmTarget.cxx
+++ b/Source/cmTarget.cxx
@@ -3007,6 +3007,21 @@ void cmTarget::GetLanguages(std::set<cmStdString>& languages) const
 bool cmTarget::IsChrpathUsed()
 {
 #if defined(CMAKE_USE_ELF_PARSER)
+  // Only certain target types have an rpath.
+  if(!(this->GetType() == cmTarget::SHARED_LIBRARY ||
+       this->GetType() == cmTarget::MODULE_LIBRARY ||
+       this->GetType() == cmTarget::EXECUTABLE))
+    {
+    return false;
+    }
+
+  // If the target will not be installed we do not need to change its
+  // rpath.
+  if(!this->GetHaveInstallRule())
+    {
+    return false;
+    }
+
   // Skip chrpath if skipping rpath altogether.
   if(this->Makefile->IsOn("CMAKE_SKIP_RPATH"))
     {