BUG: Do not copy permissions of files when making the copy in an install rule. If the source file was read-only, this prevents the subsequent set of the destination file's modification time, making the copied file always different in time-stamp than the original and always installing a new file with a new time stamp (but the same content) causing unnecessary downstream incremental rebuilds. As part of this fix, add an optional copyPermissions parameter to the SystemTools routines CopyFileIfDifferent, CopyFileAlways, CopyAFile and CopyADirectory. The copyPermissions parameter defaults to true to preserve the behavior of these routines for existing callers.

3 files changed, 43 insertions, 22 deletions

diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx
index 032d603..ca8e066 100644
--- a/Source/cmFileCommand.cxx
+++ b/Source/cmFileCommand.cxx
@@ -1049,7 +1049,7 @@ bool cmFileInstaller::InstallFile(const char* fromFile, const char* toFile,
   this->Makefile->DisplayStatus(message.c_str(), -1);
 
   // Copy the file.
-  if(copy && !cmSystemTools::CopyAFile(fromFile, toFile, true))
+  if(copy && !cmSystemTools::CopyAFile(fromFile, toFile, true, false))
     {
     cmOStringStream e;
     e << "INSTALL cannot copy file \"" << fromFile
@@ -1064,7 +1064,13 @@ bool cmFileInstaller::InstallFile(const char* fromFile, const char* toFile,
   // Set the file modification time of the destination file.
   if(copy && !always)
     {
-    cmSystemTools::CopyFileTime(fromFile, toFile);
+    if (!cmSystemTools::CopyFileTime(fromFile, toFile))
+      {
+      cmOStringStream e;
+      e << "Problem setting modification time on file \"" << toFile << "\"";
+      this->FileCommand->SetError(e.str().c_str());
+      return false;
+      }
     }
 
   // Set permissions of the destination file.
diff --git a/Source/kwsys/SystemTools.cxx b/Source/kwsys/SystemTools.cxx
index 72b109c..65f6259 100644
--- a/Source/kwsys/SystemTools.cxx
+++ b/Source/kwsys/SystemTools.cxx
@@ -1611,7 +1611,8 @@ kwsys_stl::string SystemTools::ConvertToWindowsOutputPath(const char* path)
 }
 
 bool SystemTools::CopyFileIfDifferent(const char* source,
-                                      const char* destination)
+                                      const char* destination,
+                                      bool copyPermissions)
 {
   // special check for a destination that is a directory
   // FilesDiffer does not handle file to directory compare
@@ -1624,7 +1625,8 @@ bool SystemTools::CopyFileIfDifferent(const char* source,
     new_destination += SystemTools::GetFilenameName(source_name);
     if(SystemTools::FilesDiffer(source, new_destination.c_str()))
       {
-      return SystemTools::CopyFileAlways(source, destination);
+      return SystemTools::CopyFileAlways(source, destination,
+                                         copyPermissions);
       }
     else
       {
@@ -1637,7 +1639,7 @@ bool SystemTools::CopyFileIfDifferent(const char* source,
   // are different
   if(SystemTools::FilesDiffer(source, destination))
     {
-    return SystemTools::CopyFileAlways(source, destination);
+    return SystemTools::CopyFileAlways(source, destination, copyPermissions);
     }
   // at this point the files must be the same so return true
   return true;
@@ -1718,10 +1720,12 @@ bool SystemTools::FilesDiffer(const char* source,
 }
 
 
+//----------------------------------------------------------------------------
 /**
  * Copy a file named by "source" to the file named by "destination".
  */
-bool SystemTools::CopyFileAlways(const char* source, const char* destination)
+bool SystemTools::CopyFileAlways(const char* source, const char* destination,
+                                 bool copyPermissions)
 {
   // If files are the same do not copy
   if ( SystemTools::SameFile(source, destination) )
@@ -1824,7 +1828,7 @@ bool SystemTools::CopyFileAlways(const char* source, const char* destination)
     {
    return false;
     }
-  if ( perms )
+  if ( copyPermissions && perms )
     {
     if ( !SystemTools::SetPermissions(destination, perm) )
       {
@@ -1836,15 +1840,15 @@ bool SystemTools::CopyFileAlways(const char* source, const char* destination)
 
 //----------------------------------------------------------------------------
 bool SystemTools::CopyAFile(const char* source, const char* destination,
-                            bool always)
+                            bool always, bool copyPermissions)
 {
   if(always)
     {
-    return SystemTools::CopyFileAlways(source, destination);
+    return SystemTools::CopyFileAlways(source, destination, copyPermissions);
     }
   else
     {
-    return SystemTools::CopyFileIfDifferent(source, destination);
+    return SystemTools::CopyFileIfDifferent(source, destination, copyPermissions);
     }
 }
 
@@ -1853,7 +1857,7 @@ bool SystemTools::CopyAFile(const char* source, const char* destination,
  * "destination".
  */
 bool SystemTools::CopyADirectory(const char* source, const char* destination,
-                                 bool always)
+                                 bool always, bool copyPermissions)
 {
   Directory dir;
   dir.Load(source);
@@ -1877,14 +1881,16 @@ bool SystemTools::CopyADirectory(const char* source, const char* destination,
         fullDestPath += dir.GetFile(static_cast<unsigned long>(fileNum));
         if (!SystemTools::CopyADirectory(fullPath.c_str(),
                                          fullDestPath.c_str(),
-                                         always))
+                                         always,
+                                         copyPermissions))
           {
           return false;
           }
         }
       else
         {
-        if(!SystemTools::CopyAFile(fullPath.c_str(), destination, always))
+        if(!SystemTools::CopyAFile(fullPath.c_str(), destination, always,
+                                   copyPermissions))
           {
           return false;
           }
diff --git a/Source/kwsys/SystemTools.hxx.in b/Source/kwsys/SystemTools.hxx.in
index 1484b60..b092764 100644
--- a/Source/kwsys/SystemTools.hxx.in
+++ b/Source/kwsys/SystemTools.hxx.in
@@ -490,33 +490,42 @@ public:
 
   /**
    * Copy the source file to the destination file only
-   * if the two files differ.  
+   * if the two files differ. If the "copyPermissions"
+   * argument is true, the permissions of the copy are
+   * set to be the same as the permissions of the
+   * original.
    */
   static bool CopyFileIfDifferent(const char* source,
-                                  const char* destination);
-  
+                                  const char* destination,
+                                  bool copyPermissions = true);
+
   /**
    * Compare the contents of two files.  Return true if different
    */
   static bool FilesDiffer(const char* source, const char* destination);
-  
+
   /**
    * Return true if the two files are the same file
    */
   static bool SameFile(const char* file1, const char* file2);
 
   /**
-   * Copy a file
+   * Copy a file. If the "copyPermissions" argument is true, the
+   * permissions of the copy are set to be the same as the permissions
+   * of the original.
    */
-  static bool CopyFileAlways(const char* source, const char* destination);
+  static bool CopyFileAlways(const char* source, const char* destination,
+                             bool copyPermissions = true);
 
   /**
    * Copy a file.  If the "always" argument is true the file is always
    * copied.  If it is false, the file is copied only if it is new or
-   * has changed.
+   * has changed. If the "copyPermissions" argument is true, the
+   * permissions of the copy are set to be the same as the permissions
+   * of the original.
    */
   static bool CopyAFile(const char* source, const char* destination,
-                        bool always = true);
+                        bool always = true, bool copyPermissions = true);
 
   /**
    * Copy content directory to another directory with all files and
@@ -525,7 +534,7 @@ public:
    * are new are copied.
    */
   static bool CopyADirectory(const char* source, const char* destination,
-                             bool always = true);
+                             bool always = true, bool copyPermissions = true);
   
   /**
    * Remove a file