ENH: Improve RPATH behavior during installation.

  - If new RPATH is empty then remove the entry completely
  - Preserve file modification time so installation is not repeated
  - If installed file already exists remove it if its RPATH
    does not match that expected

4 files changed, 239 insertions, 35 deletions

diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx
index 68ccbb2..3cf6c0b 100644
--- a/Source/cmFileCommand.cxx
+++ b/Source/cmFileCommand.cxx
@@ -112,9 +112,17 @@ bool cmFileCommand
     {
     return this->HandleInstallCommand(args);
     }
-  else if ( subCommand == "CHRPATH" )
+  else if ( subCommand == "RPATH_CHANGE" || subCommand == "CHRPATH" )
     {
-    return this->HandleChrpathCommand(args);
+    return this->HandleRPathChangeCommand(args);
+    }
+  else if ( subCommand == "RPATH_CHECK" )
+    {
+    return this->HandleRPathCheckCommand(args);
+    }
+  else if ( subCommand == "RPATH_REMOVE" )
+    {
+    return this->HandleRPathRemoveCommand(args);
     }
   else if ( subCommand == "RELATIVE_PATH" )
     {
@@ -1332,7 +1340,8 @@ bool cmFileCommand::HandleInstallDestination(cmFileInstaller& installer,
 }
 
 //----------------------------------------------------------------------------
-bool cmFileCommand::HandleChrpathCommand(std::vector<std::string> const& args)
+bool
+cmFileCommand::HandleRPathChangeCommand(std::vector<std::string> const& args)
 {
   // Evaluate arguments.
   const char* file = 0;
@@ -1372,49 +1381,174 @@ bool cmFileCommand::HandleChrpathCommand(std::vector<std::string> const& args)
     else
       {
       cmOStringStream e;
-      e << "CHRPATH given unknown argument " << args[i];
+      e << "RPATH_CHANGE given unknown argument " << args[i];
       this->SetError(e.str().c_str());
       return false;
       }
     }
   if(!file)
     {
-    this->SetError("CHRPATH not given FILE option.");
+    this->SetError("RPATH_CHANGE not given FILE option.");
     return false;
     }
   if(!oldRPath)
     {
-    this->SetError("CHRPATH not given OLD_RPATH option.");
+    this->SetError("RPATH_CHANGE not given OLD_RPATH option.");
     return false;
     }
   if(!newRPath)
     {
-    this->SetError("CHRPATH not given NEW_RPATH option.");
+    this->SetError("RPATH_CHANGE not given NEW_RPATH option.");
     return false;
     }
   if(!cmSystemTools::FileExists(file, true))
     {
     cmOStringStream e;
-    e << "CHRPATH given FILE \"" << file << "\" that does not exist.";
+    e << "RPATH_CHANGE given FILE \"" << file << "\" that does not exist.";
     this->SetError(e.str().c_str());
     return false;
     }
+  bool success = true;
+  cmSystemToolsFileTime* ft = cmSystemTools::FileTimeNew();
+  bool have_ft = cmSystemTools::FileTimeGet(file, ft);
   std::string emsg;
-  if(cmSystemTools::ChangeRPath(file, oldRPath, newRPath, &emsg))
-    {
-    return true;
-    }
-  else
+  if(!cmSystemTools::ChangeRPath(file, oldRPath, newRPath, &emsg))
     {
     cmOStringStream e;
-    e << "CHRPATH could not write new RPATH:\n"
+    e << "RPATH_CHANGE could not write new RPATH:\n"
       << "  " << newRPath << "\n"
       << "to the file:\n"
       << "  " << file << "\n"
       << emsg;
     this->SetError(e.str().c_str());
+    success = false;
+    }
+  if(success && have_ft)
+    {
+    cmSystemTools::FileTimeSet(file, ft);
+    }
+  cmSystemTools::FileTimeDelete(ft);
+  return success;
+}
+
+//----------------------------------------------------------------------------
+bool
+cmFileCommand::HandleRPathRemoveCommand(std::vector<std::string> const& args)
+{
+  // Evaluate arguments.
+  const char* file = 0;
+  enum Doing { DoingNone, DoingFile };
+  Doing doing = DoingNone;
+  for(unsigned int i=1; i < args.size(); ++i)
+    {
+    if(args[i] == "FILE")
+      {
+      doing = DoingFile;
+      }
+    else if(doing == DoingFile)
+      {
+      file = args[i].c_str();
+      doing = DoingNone;
+      }
+    else
+      {
+      cmOStringStream e;
+      e << "RPATH_REMOVE given unknown argument " << args[i];
+      this->SetError(e.str().c_str());
+      return false;
+      }
+    }
+  if(!file)
+    {
+    this->SetError("RPATH_REMOVE not given FILE option.");
+    return false;
+    }
+  if(!cmSystemTools::FileExists(file, true))
+    {
+    cmOStringStream e;
+    e << "RPATH_REMOVE given FILE \"" << file << "\" that does not exist.";
+    this->SetError(e.str().c_str());
+    return false;
+    }
+  bool success = true;
+  cmSystemToolsFileTime* ft = cmSystemTools::FileTimeNew();
+  bool have_ft = cmSystemTools::FileTimeGet(file, ft);
+  std::string emsg;
+  if(!cmSystemTools::RemoveRPath(file, &emsg))
+    {
+    cmOStringStream e;
+    e << "RPATH_REMOVE could not remove RPATH from file:\n"
+      << "  " << file << "\n"
+      << emsg;
+    this->SetError(e.str().c_str());
+    success = false;
+    }
+  if(success && have_ft)
+    {
+    cmSystemTools::FileTimeSet(file, ft);
+    }
+  cmSystemTools::FileTimeDelete(ft);
+  return success;
+}
+
+//----------------------------------------------------------------------------
+bool
+cmFileCommand::HandleRPathCheckCommand(std::vector<std::string> const& args)
+{
+  // Evaluate arguments.
+  const char* file = 0;
+  const char* rpath = 0;
+  enum Doing { DoingNone, DoingFile, DoingRPath };
+  Doing doing = DoingNone;
+  for(unsigned int i=1; i < args.size(); ++i)
+    {
+    if(args[i] == "RPATH")
+      {
+      doing = DoingRPath;
+      }
+    else if(args[i] == "FILE")
+      {
+      doing = DoingFile;
+      }
+    else if(doing == DoingFile)
+      {
+      file = args[i].c_str();
+      doing = DoingNone;
+      }
+    else if(doing == DoingRPath)
+      {
+      rpath = args[i].c_str();
+      doing = DoingNone;
+      }
+    else
+      {
+      cmOStringStream e;
+      e << "RPATH_CHECK given unknown argument " << args[i];
+      this->SetError(e.str().c_str());
+      return false;
+      }
+    }
+  if(!file)
+    {
+    this->SetError("RPATH_CHECK not given FILE option.");
+    return false;
+    }
+  if(!rpath)
+    {
+    this->SetError("RPATH_CHECK not given RPATH option.");
     return false;
     }
+
+  // If the file exists but does not have the desired RPath then
+  // delete it.  This is used during installation to re-install a file
+  // if its RPath will change.
+  if(cmSystemTools::FileExists(file, true) &&
+     !cmSystemTools::CheckRPath(file, rpath))
+    {
+    cmSystemTools::RemoveFile(file);
+    }
+
+  return true;
 }
 
 //----------------------------------------------------------------------------
diff --git a/Source/cmFileCommand.h b/Source/cmFileCommand.h
index 85bdc32..3f72cd2 100644
--- a/Source/cmFileCommand.h
+++ b/Source/cmFileCommand.h
@@ -171,7 +171,9 @@ protected:
   bool HandleRelativePathCommand(std::vector<std::string> const& args);
   bool HandleCMakePathCommand(std::vector<std::string> const& args,
                               bool nativePath);
-  bool HandleChrpathCommand(std::vector<std::string> const& args);
+  bool HandleRPathChangeCommand(std::vector<std::string> const& args);
+  bool HandleRPathCheckCommand(std::vector<std::string> const& args);
+  bool HandleRPathRemoveCommand(std::vector<std::string> const& args);
 
   // file(INSTALL ...) related functions
   bool HandleInstallCommand(std::vector<std::string> const& args);
diff --git a/Source/cmInstallTargetGenerator.cxx b/Source/cmInstallTargetGenerator.cxx
index 5fd408e..8db0957 100644
--- a/Source/cmInstallTargetGenerator.cxx
+++ b/Source/cmInstallTargetGenerator.cxx
@@ -333,6 +333,32 @@ cmInstallTargetGenerator
     return;
     }
 
+  // Construct the path of the file on disk after installation on
+  // which tweaks may be performed.
+  std::string toDestDirPath = "$ENV{DESTDIR}";
+  if(toInstallPath[0] != '/' && toInstallPath[0] != '$')
+    {
+    toDestDirPath += "/";
+    }
+  toDestDirPath += toInstallPath;
+
+  // Add pre-installation tweaks.
+  if(tweakInstalledFile)
+    {
+    // Collect tweaking rules.
+    cmOStringStream tw;
+    this->AddRPathCheckRule(tw, indent.Next(), config, toDestDirPath);
+    std::string tws = tw.str();
+
+    // Add the rules, if any.
+    if(!tws.empty())
+      {
+      os << indent << "IF(EXISTS \"" << toDestDirPath << "\")\n";
+      os << tws;
+      os << indent << "ENDIF(EXISTS \"" << toDestDirPath << "\")\n";
+      }
+    }
+
   // Write code to install the target file.
   const char* no_dir_permissions = 0;
   const char* no_rename = 0;
@@ -344,25 +370,24 @@ cmInstallTargetGenerator
                        no_rename, literal_args.c_str(),
                        indent);
 
-  // Construct the path of the file on disk after installation on
-  // which tweaks may be performed.
-  std::string toDestDirPath = "$ENV{DESTDIR}";
-  if(toInstallPath[0] != '/' && toInstallPath[0] != '$')
-    {
-    toDestDirPath += "/";
-    }
-  toDestDirPath += toInstallPath;
-
-  // TODO:
-  //   - Skip IF(EXISTS) checks if nothing is done with the installed file
+  // Add post-installation tweaks.
   if(tweakInstalledFile)
     {
-    os << indent << "IF(EXISTS \"" << toDestDirPath << "\")\n";
-    this->AddInstallNamePatchRule(os, indent.Next(), config, toDestDirPath);
-    this->AddChrpathPatchRule(os, indent.Next(), config, toDestDirPath);
-    this->AddRanlibRule(os, indent.Next(), type, toDestDirPath);
-    this->AddStripRule(os, indent.Next(), type, toDestDirPath);
-    os << indent << "ENDIF(EXISTS \"" << toDestDirPath << "\")\n";
+    // Collect tweaking rules.
+    cmOStringStream tw;
+    this->AddInstallNamePatchRule(tw, indent.Next(), config, toDestDirPath);
+    this->AddChrpathPatchRule(tw, indent.Next(), config, toDestDirPath);
+    this->AddRanlibRule(tw, indent.Next(), type, toDestDirPath);
+    this->AddStripRule(tw, indent.Next(), type, toDestDirPath);
+    std::string tws = tw.str();
+
+    // Add the rules, if any.
+    if(!tws.empty())
+      {
+      os << indent << "IF(EXISTS \"" << toDestDirPath << "\")\n";
+      os << tws;
+      os << indent << "ENDIF(EXISTS \"" << toDestDirPath << "\")\n";
+      }
     }
 }
 
@@ -550,6 +575,37 @@ cmInstallTargetGenerator
 //----------------------------------------------------------------------------
 void
 cmInstallTargetGenerator
+::AddRPathCheckRule(std::ostream& os, Indent const& indent,
+                    const char* config, std::string const& toDestDirPath)
+{
+  // Skip the chrpath if the target does not need it.
+  if(this->ImportLibrary || !this->Target->IsChrpathUsed())
+    {
+    return;
+    }
+
+  // Get the link information for this target.
+  // It can provide the RPATH.
+  cmComputeLinkInformation* cli = this->Target->GetLinkInformation(config);
+  if(!cli)
+    {
+    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.
+  os << indent << "FILE(RPATH_CHECK\n"
+     << indent << "     FILE \"" << toDestDirPath << "\"\n"
+     << indent << "     RPATH \"" << newRpath << "\")\n";
+}
+
+//----------------------------------------------------------------------------
+void
+cmInstallTargetGenerator
 ::AddChrpathPatchRule(std::ostream& os, Indent const& indent,
                       const char* config, std::string const& toDestDirPath)
 {
@@ -580,9 +636,18 @@ cmInstallTargetGenerator
     }
 
   // Write a rule to run chrpath to set the install-tree RPATH
-  os << indent << "FILE(CHRPATH FILE \"" << toDestDirPath << "\"\n"
-     << indent << "     OLD_RPATH \"" << oldRpath << "\"\n"
-     << indent << "     NEW_RPATH \"" << newRpath << "\")\n";
+  if(newRpath.empty())
+    {
+    os << indent << "FILE(RPATH_REMOVE\n"
+       << indent << "     FILE \"" << toDestDirPath << "\")\n";
+    }
+  else
+    {
+    os << indent << "FILE(RPATH_CHANGE\n"
+       << indent << "     FILE \"" << toDestDirPath << "\"\n"
+       << indent << "     OLD_RPATH \"" << oldRpath << "\"\n"
+       << indent << "     NEW_RPATH \"" << newRpath << "\")\n";
+    }
 }
 
 //----------------------------------------------------------------------------
diff --git a/Source/cmInstallTargetGenerator.h b/Source/cmInstallTargetGenerator.h
index 95c255d..61357b3 100644
--- a/Source/cmInstallTargetGenerator.h
+++ b/Source/cmInstallTargetGenerator.h
@@ -80,6 +80,9 @@ protected:
   void AddChrpathPatchRule(std::ostream& os, Indent const& indent,
                            const char* config,
                            std::string const& toDestDirPath);
+  void AddRPathCheckRule(std::ostream& os, Indent const& indent,
+                         const char* config,
+                         std::string const& toDestDirPath);
   
   void AddStripRule(std::ostream& os, Indent const& indent,
                     cmTarget::TargetType type,