ENH: Re-implemented cmGeneratedFileStream to look like a real stream and replace the destination file atomically. This will avoid problems with the process being terminated while generating a file.

Source/CMakeLists.txt

@@ -16,6 +16,7 @@ cmCacheManager.cxx
 cmSourceGroup.cxx
 cmListFileCache.cxx
 cmListFileLexer.c
+cmGeneratedFileStream.cxx
 cmGlob.cxx
 cmGlobalGenerator.cxx
 cmGlobalUnixMakefileGenerator.cxx

Source/cmExportLibraryDependencies.cxx

@@ -73,9 +73,9 @@ void cmExportLibraryDependenciesCommand::FinalPass()
   else
     {
     std::auto_ptr<cmGeneratedFileStream> ap(
-      new cmGeneratedFileStream(fname.c_str()));
+      new cmGeneratedFileStream(fname.c_str(), true, true));
     foutNew = ap;
-    foutPtr = &foutNew->GetStream();
+    foutPtr = foutNew.get();
     }
   std::ostream& fout = *foutPtr;
 

Source/cmGeneratedFileStream.cxx

@@ -0,0 +1,140 @@
+/*=========================================================================
+
+  Program:   CMake - Cross-Platform Makefile Generator
+  Module:    $RCSfile$
+  Language:  C++
+  Date:      $Date$
+  Version:   $Revision$
+
+  Copyright (c) 2002 Kitware, Inc., Insight Consortium.  All rights reserved.
+  See Copyright.txt or http://www.cmake.org/HTML/Copyright.html for details.
+
+     This software is distributed WITHOUT ANY WARRANTY; without even
+     the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+     PURPOSE.  See the above copyright notices for more information.
+
+=========================================================================*/
+#include "cmGeneratedFileStream.h"
+
+#include "cmSystemTools.h"
+
+// Includes needed for implementation of RenameFile.  This is not in
+// system tools because it is not implemented robustly enough to move
+// files across directories.
+#ifdef _WIN32
+# include <windows.h>
+# include <sys/stat.h>
+#endif
+
+//----------------------------------------------------------------------------
+cmGeneratedFileStream::cmGeneratedFileStream(const char* name,
+                                             bool copy_if_different,
+                                             bool quiet):
+  cmGeneratedFileStreamBase(name, copy_if_different),
+  std::ofstream(m_TempName.c_str())
+{
+  // Check if the file opened.
+  if(!*this && !quiet)
+    {
+    cmSystemTools::Error("Cannot open file for write: ", m_TempName.c_str());
+    cmSystemTools::ReportLastSystemError("");
+    }
+}
+
+//----------------------------------------------------------------------------
+cmGeneratedFileStream::~cmGeneratedFileStream()
+{
+  // This is the first destructor called.  Check the status of the
+  // stream and give the information to the private base.  Next the
+  // stream will be destroyed which will close the temporary file.
+  // Finally the base destructor will be called to replace the
+  // destination file.
+  m_Okay = *this;
+}
+
+//----------------------------------------------------------------------------
+cmGeneratedFileStreamBase::cmGeneratedFileStreamBase(const char* name,
+                                                     bool copy_if_different):
+  m_Name(name),
+  m_TempName(name),
+  m_CopyIfDifferent(copy_if_different),
+  m_Okay(false)
+{
+  // Create the name of the temporary file.
+  m_TempName += ".tmp";
+
+  // Make sure the temporary file that will be used is not present.
+  cmSystemTools::RemoveFile(m_TempName.c_str());
+}
+
+//----------------------------------------------------------------------------
+cmGeneratedFileStreamBase::~cmGeneratedFileStreamBase()
+{
+  // Only consider replacing the destination file if no error
+  // occurred.
+  if(m_Okay &&
+     (!m_CopyIfDifferent ||
+      cmSystemTools::FilesDiffer(m_TempName.c_str(), m_Name.c_str())))
+    {
+    // The destination is to be replaced.  Rename the temporary to the
+    // destination atomically.
+    this->RenameFile(m_TempName.c_str(), m_Name.c_str());
+    }
+  else
+    {
+    // The destination was not replaced.  Just delete the temporary
+    // file.
+    cmSystemTools::RemoveFile(m_TempName.c_str());
+    }
+}
+
+//----------------------------------------------------------------------------
+int cmGeneratedFileStreamBase::RenameFile(const char* oldname,
+                                          const char* newname)
+{
+#ifdef _WIN32
+  /* On Windows the move functions will not replace existing files.
+     Check if the destination exists.  */
+  struct stat newFile;
+  if(stat(newname, &newFile) == 0)
+    {
+    /* The destination exists.  We have to replace it carefully.  The
+       MoveFileEx function does what we need but is not available on
+       Win9x.  */
+    OSVERSIONINFO osv;
+    DWORD attrs;
+
+    /* Make sure the destination is not read only.  */
+    attrs = GetFileAttributes(newname);
+    if(attrs & FILE_ATTRIBUTE_READONLY)
+      {
+      SetFileAttributes(newname, attrs & ~FILE_ATTRIBUTE_READONLY);
+      }
+
+    /* Check the windows version number.  */
+    osv.dwOSVersionInfoSize = sizeof(osv);
+    GetVersionEx(&osv);
+    if(osv.dwPlatformId == VER_PLATFORM_WIN32_WINDOWS)
+      {
+      /* This is Win9x.  There is no MoveFileEx implementation.  We
+         cannot quite rename the file atomically.  Just delete the
+         destination and then move the file.  */
+      DeleteFile(newname);
+      return MoveFile(oldname, newname);
+      }
+    else
+      {
+      /* This is not Win9x.  Use the MoveFileEx implementation.  */
+      return MoveFileEx(oldname, newname, MOVEFILE_REPLACE_EXISTING);
+      }
+    }
+  else
+    {
+    /* The destination does not exist.  Just move the file.  */
+    return MoveFile(oldname, newname);
+    }
+#else
+  /* On UNIX we have an OS-provided call to do this atomically.  */
+  return rename(oldname, newname) == 0;
+#endif
+}

Source/cmGeneratedFileStream.h

@@ -18,122 +18,65 @@
 #define cmGeneratedFileStream_h
 
 #include "cmStandardIncludes.h"
-#include "cmSystemTools.h"
+
+// This is the first base class of cmGeneratedFileStream.  It will be
+// created before and destroyed after the ofstream portion and can
+// therefore be used to manage the temporary file.
+class cmGeneratedFileStreamBase
+{
+protected:
+  // The constructor prepares the temporary output file.
+  cmGeneratedFileStreamBase(const char* name, bool copy_if_different);
+
+  // The destructor renames the temporary output file to the real name.
+  ~cmGeneratedFileStreamBase();
+
+  // Internal file replacement implementation.
+  int RenameFile(const char* oldname, const char* newname);
+
+  // The name of the final destination file for the output.
+  std::string m_Name;
+
+  // The name of the temporary file.
+  std::string m_TempName;
+
+  // Whether to do a copy-if-different.
+  bool m_CopyIfDifferent;
+
+  // Whether the destination file should be replaced.
+  bool m_Okay;
+};
 
 /** \class cmGeneratedFileStream
- * \brief Output stream for generated files that does copy-if-different.
+ * \brief Output stream for generated files.
  *
- * Many files generated by CMake don't change each time they are generated.
- * This class can be used in place of std::ofstream to open a file and
- * write output to it.  The class will automatically write output to a
- * temporary file and copy it over an existing file only if the generated
- * file has changed.
+ * File generation should be atomic so that if CMake is killed then a
+ * generated file is either the original version or the complete new
+ * version.  This stream is used to make sure file generation is
+ * atomic.  Optionally the output file is only replaced if its
+ * contents have changed to prevent the file modification time from
+ * being updated.
  */
-class cmGeneratedFileStream
+class cmGeneratedFileStream: private cmGeneratedFileStreamBase,
+                             public std::ofstream
 {
 public:
   /**
    * The constructor takes the name of the file to be generated.  It
-   * automatically generates a name for the temporary file.
+   * automatically generates a name for the temporary file.  The
+   * second argument specifies whether the copy-if-different check
+   * should be done.  If the file cannot be opened an error message is
+   * produced unless the third argument is set to true.
    */
-  cmGeneratedFileStream(const char* name):
-    m_Name(name),
-    m_TempName(m_Name+".tmp"),
-    m_Stream(m_TempName.c_str()),
-    m_Copied(false),
-    m_AlwaysCopy(false)
-    {}
+  cmGeneratedFileStream(const char* name, bool copy_if_different,
+                        bool quiet = false);
 
   /**
-   * The destructor ensures that the file has been closed and copied if
-   * it has changed.
+   * The destructor checks the stream status to be sure the temporary
+   * file was successfully written before allowing the original to be
+   * replaced.
    */
-  ~cmGeneratedFileStream() { this->DoCopy(); }
-  
-  /**
-   * Get the real output stream.
-   */
-  std::ostream& GetStream() { return m_Stream; }
-  
-  /**
-   * Allow a test for whether the file is open.
-   */
-  operator bool() { return m_Stream.good(); }
-  
-  /**
-   * Close the file stream.  This will cause the copy-if-different to the
-   * real file name to occur.
-   */
-  void close() { this->DoCopy(); }
-  /**
-   * If always copy is true, then copy the file all the time without
-   *  checking for differences.  The default is false.
-   */
-  bool SetAlwaysCopy(bool v) { m_AlwaysCopy = v; return v;}
-private:
-  /**
-   * The name of the real file where output will be copied if it has changed.
-   */
-  std::string m_Name;
-  
-  /**
-   * The name of the temporary file.
-   */
-  std::string m_TempName;
-  
-  /**
-   * The real output stream used to write to the file.
-   */
-  std::ofstream m_Stream;
-  
-  /**
-   * Whether the temporary file has already been copied to the real file.
-   */
-  bool m_Copied;
-
-  /**
-   *  If always copy is true, then copy the file all the time without
-   *  checking for differences.  The default is false.
-   */
-  bool m_AlwaysCopy;
-  
-  /**
-   * Closes the temporary file and does the copy-if-different to the
-   * real file.
-   */
-  void DoCopy()
-    {
-    if(!m_Copied)
-      {
-      m_Stream.close();
-      if(m_AlwaysCopy)
-        {
-        cmSystemTools::cmCopyFile(m_TempName.c_str(), m_Name.c_str());
-        }
-      else
-        {
-        cmSystemTools::CopyFileIfDifferent(m_TempName.c_str(),
-                                           m_Name.c_str());
-        }
-      cmSystemTools::RemoveFile(m_TempName.c_str());
-      m_Copied = true;
-      }
-    }
+  ~cmGeneratedFileStream();
 };
 
-
-/**
- * Allow a cmGeneratedFileStream to be used just as a real std::ostream
- * would be.
- */
-template <class T>
-std::ostream& operator << (cmGeneratedFileStream& l, const T& r)
-{
-  std::ostream& os = l.GetStream();
-  os << r;
-  return os;
-}
-
-
-
 #endif

Source/cmLocalGenerator.cxx

@@ -92,8 +92,7 @@ void cmLocalGenerator::GenerateInstallRules()
     toplevel_install = 1;
     }
   file += "/cmake_install.cmake";
-  cmGeneratedFileStream tempFile(file.c_str());
-  std::ostream&  fout = tempFile.GetStream();
+  cmGeneratedFileStream fout(file.c_str(), true);
 
   fout << "# Install script for directory: " << m_Makefile->GetCurrentDirectory() 
     << std::endl << std::endl;

Source/cmLocalKdevelopGenerator.cxx

@@ -120,12 +120,9 @@ void cmLocalKdevelopGenerator::MergeProjectFiles(const std::string& outputDir,
     }
   oldProjectFile.close();
 
-  cmGeneratedFileStream tempFile(filename.c_str());
-  tempFile.SetAlwaysCopy(true);
-  std::ostream&  fout = tempFile.GetStream();
+  cmGeneratedFileStream fout(filename.c_str(), false);
   if(!fout)
     {
-    cmSystemTools::Error("Error can not open for write: ", filename.c_str());
     return;
     }
 
@@ -174,13 +171,9 @@ void cmLocalKdevelopGenerator::CreateNewProjectFile(const std::string& outputDir
                                                     const std::string& cmakeFilePattern)
 {
 
-  cmGeneratedFileStream tempFile(filename.c_str());
-  tempFile.SetAlwaysCopy(true);
-
-  std::ostream&  fout = tempFile.GetStream();
+  cmGeneratedFileStream fout(filename.c_str(), false);
   if(!fout)
     {
-    cmSystemTools::Error("Error can not open for write: ", filename.c_str());
     return;
     }
 
@@ -371,12 +364,9 @@ bool cmLocalKdevelopGenerator::CreateFilelistFile(const std::string& outputDir,
     }
 
   //now write the new filename
-  cmGeneratedFileStream tempFile(filename.c_str());
-  tempFile.SetAlwaysCopy(true);
-  std::ostream&  fout = tempFile.GetStream();
+  cmGeneratedFileStream fout(filename.c_str(), false);
   if(!fout)
     {
-    cmSystemTools::Error("Error can not open for write: ", filename.c_str());
     return false;
     }
    

Source/cmLocalUnixMakefileGenerator.cxx

@@ -186,13 +186,9 @@ void cmLocalUnixMakefileGenerator::OutputMakefile(const char* file,
   // Create a stream that writes to a temporary file
   // then does a copy at the end.   This is to allow users
   // to hit control-c during the make of the makefile
-  cmGeneratedFileStream tempFile(file);
-  tempFile.SetAlwaysCopy(true);
-  std::ostream&  fout = tempFile.GetStream();
+  cmGeneratedFileStream fout(file, false);
   if(!fout)
     {
-    cmSystemTools::Error("Error can not open for write: ", file);
-    cmSystemTools::ReportLastSystemError("");
     return;
     }
   fout << "# CMAKE generated Makefile, DO NOT EDIT!\n"

Source/cmLocalUnixMakefileGenerator2.cxx

@@ -237,12 +237,9 @@ cmLocalUnixMakefileGenerator2
   // Open the rule file.  This should be copy-if-different because the
   // rules may depend on this file itself.
   std::string ruleFileNameFull = this->ConvertToFullPath(ruleFileName);
-  cmGeneratedFileStream ruleFile(ruleFileNameFull.c_str());
-  std::ostream& ruleFileStream = ruleFile.GetStream();
+  cmGeneratedFileStream ruleFileStream(ruleFileNameFull.c_str(), true);
   if(!ruleFileStream)
     {
-    // TODO: Produce error message that accounts for generated stream
-    // .tmp.
     return;
     }
   this->WriteDisclaimer(ruleFileStream);
@@ -354,12 +351,9 @@ cmLocalUnixMakefileGenerator2
   std::string ruleFileName = obj;
   ruleFileName += ".make";
   std::string ruleFileNameFull = this->ConvertToFullPath(ruleFileName);
-  cmGeneratedFileStream ruleFile(ruleFileNameFull.c_str());
-  std::ostream& ruleFileStream = ruleFile.GetStream();
+  cmGeneratedFileStream ruleFileStream(ruleFileNameFull.c_str(), true);
   if(!ruleFileStream)
     {
-    // TODO: Produce error message that accounts for generated stream
-    // .tmp.
     return;
     }
   this->WriteDisclaimer(ruleFileStream);
@@ -532,12 +526,9 @@ cmLocalUnixMakefileGenerator2
   // Open the rule file.  This should be copy-if-different because the
   // rules may depend on this file itself.
   std::string ruleFileNameFull = this->ConvertToFullPath(ruleFileName);
-  cmGeneratedFileStream ruleFile(ruleFileNameFull.c_str());
-  std::ostream& ruleFileStream = ruleFile.GetStream();
+  cmGeneratedFileStream ruleFileStream(ruleFileNameFull.c_str(), true);
   if(!ruleFileStream)
     {
-    // TODO: Produce error message that accounts for generated stream
-    // .tmp.
     return;
     }
   this->WriteDisclaimer(ruleFileStream);

Source/cmVTKMakeInstantiatorCommand.cxx

@@ -127,16 +127,16 @@ cmVTKMakeInstantiatorCommand
   std::string fullName = headerPath+"/"+fileName;
   
   // Generate the output file with copy-if-different.
-  cmGeneratedFileStream fout(fullName.c_str());
+  cmGeneratedFileStream fout(fullName.c_str(), true);
   
   // Actually generate the code in the file.
   if(!oldVersion)
     {
-    this->GenerateHeaderFile(fout.GetStream());
+    this->GenerateHeaderFile(fout);
     }
   else
     {
-    this->OldGenerateHeaderFile(fout.GetStream());
+    this->OldGenerateHeaderFile(fout);
     }
   }
   
@@ -147,16 +147,16 @@ cmVTKMakeInstantiatorCommand
   
   // Generate the output file with copy-if-different.
   {
-  cmGeneratedFileStream fout(fullName.c_str());
+  cmGeneratedFileStream fout(fullName.c_str(), true);
   
   // Actually generate the code in the file.
   if(!oldVersion)
     {
-    this->GenerateImplementationFile(fout.GetStream());
+    this->GenerateImplementationFile(fout);
     }
   else
     {
-    this->OldGenerateImplementationFile(fout.GetStream());
+    this->OldGenerateImplementationFile(fout);
     }
   }
   
@@ -188,13 +188,13 @@ cmVTKMakeInstantiatorCommand
     
       // Generate the output file with copy-if-different.
       {
-      cmGeneratedFileStream fout(fullName.c_str());
+      cmGeneratedFileStream fout(fullName.c_str(), true);
     
       size_t thisBlockSize =
         (block < numFullBlocks)? groupSize:lastBlockSize;
     
       // Actually generate the code in the file.
-      this->OldGenerateCreationFile(fout.GetStream(),
+      this->OldGenerateCreationFile(fout,
                                     block*groupSize, 
                                     static_cast<int>(thisBlockSize));
       }

bootstrap

@@ -38,6 +38,7 @@ CMAKE_CXX_SOURCES="\
   cmakemain \
   cmMakeDepend \
   cmMakefile \
+  cmGeneratedFileStream \
   cmGlobalGenerator \
   cmLocalGenerator \
   cmSourceFile \