IcePatch bug fixes

3 files changed, 146 insertions, 64 deletions

diff --git a/cpp/src/IcePatch/IcePatchI.cpp b/cpp/src/IcePatch/IcePatchI.cpp
index 5600bbd54de..2aae5148bef 100644
--- a/cpp/src/IcePatch/IcePatchI.cpp
+++ b/cpp/src/IcePatch/IcePatchI.cpp
@@ -55,15 +55,31 @@ IcePatch::FileI::readMD5(const Current& current) const
 	    sync.timedUpgrade(_busyTimeout);
 
 	    infoMD5 = getFileInfo(path + ".md5", false);
-
-	    if(infoMD5.type != FileTypeRegular || infoMD5.time <= info.time)
+	    while(infoMD5.type != FileTypeRegular || infoMD5.time <= info.time)
 	    {
-		createMD5(path);
-		
-		if(_traceLevel > 0)
+		//
+		// Timestamps have only a resolution of one second, so
+		// we sleep if it's likely that the created MD5 file
+		// would have the same time stamp as the source file.
+		//
+		IceUtil::Time diff = IceUtil::Time::seconds(info.time + 1) - IceUtil::Time::now();
+		if(diff > IceUtil::Time())
+		{
+		    IceUtil::ThreadControl::sleep(diff);
+		}
+
+		createMD5(path, _traceLevel > 0 ? _logger : LoggerPtr());
+
+		//
+		// If the source file size has changed after we
+		// created the MD5 file, we try again.
+		//
+		FileInfo oldInfo = info;
+		info = getFileInfo(path, true);
+		infoMD5 = getFileInfo(path + ".md5", true); // Must exist.
+		if(info.size != oldInfo.size)
 		{
-		    Trace out(_logger, "IcePatch");
-		    out << "created MD5 file for `" << path << "'";
+		    info.time = infoMD5.time;
 		}
 	    }
 	}
@@ -122,13 +138,7 @@ IcePatch::DirectoryI::getContents(const Current& current) const
 			equal_range(paths2.begin(), paths2.end(), removeSuffix(*p));
 		    if(r2.first == r2.second)
 		    {
-			removeRecursive(*p);
-
-			if(_traceLevel > 0)
-			{
-			    Trace out(_logger, "IcePatch");
-			    out << "removed orphaned file `" << *p << "'";
-			}
+			removeRecursive(*p, _traceLevel > 0 ? _logger : LoggerPtr());
 		    }
 		}
 	    }
@@ -200,23 +210,21 @@ IcePatch::RegularI::getBZ2Size(const Current& current) const
 	    sync.timedUpgrade(_busyTimeout);
 
 	    infoBZ2 = getFileInfo(path + ".bz2", false);
-
-	    if(infoBZ2.type != FileTypeRegular || infoBZ2.time <= info.time)
+	    while(infoBZ2.type != FileTypeRegular || infoBZ2.time <= info.time)
 	    {
-		createBZ2(path);
-		
-		if(_traceLevel > 0)
-		{
-		    Trace out(_logger, "IcePatch");
-		    out << "created BZ2 file for `" << path << "'";
-		}
+		createBZ2(path, _traceLevel > 0 ? _logger : LoggerPtr());
 		
 		//
-		// Get the .bz2 file info again, so that we can return the
-		// size below. This time the .bz2 file must exist,
-		// otherwise an exception is raised.
+		// If the source file size has changed after we
+		// created the BZ2 file, we try again.
 		//
-		infoBZ2 = getFileInfo(path + ".bz2", true);
+		FileInfo oldInfo = info;
+		info = getFileInfo(path, true);
+		infoBZ2 = getFileInfo(path + ".bz2", true); // Must exist.
+		if(info.size != oldInfo.size)
+		{
+		    info.time = infoBZ2.time;
+		}
 	    }
 	}
 	
@@ -246,15 +254,20 @@ IcePatch::RegularI::getBZ2(Int pos, Int num, const Current& current) const
 	    sync.timedUpgrade(_busyTimeout);
 
 	    infoBZ2 = getFileInfo(path + ".bz2", false);
-
-	    if(infoBZ2.type != FileTypeRegular || infoBZ2.time <= info.time)
+	    while(infoBZ2.type != FileTypeRegular || infoBZ2.time <= info.time)
 	    {
-		createBZ2(path);
-		
-		if(_traceLevel > 0)
+		createBZ2(path, _traceLevel > 0 ? _logger : LoggerPtr());
+
+		//
+		// If the source file size has changed after we
+		// created the BZ2 file, we try again.
+		//
+		FileInfo oldInfo = info;
+		info = getFileInfo(path, true);
+		infoBZ2 = getFileInfo(path + ".bz2", true); // Must exist.
+		if(info.size != oldInfo.size)
 		{
-		    Trace out(_logger, "IcePatch");
-		    out << "created BZ2 file for `" << path << "'";
+		    info.time = infoBZ2.time;
 		}
 	    }
 	}
@@ -285,15 +298,20 @@ IcePatch::RegularI::getBZ2MD5(Int size, const Current& current) const
 	    sync.timedUpgrade(_busyTimeout);
 
 	    infoBZ2 = getFileInfo(path + ".bz2", false);
-
-	    if(infoBZ2.type != FileTypeRegular || infoBZ2.time <= info.time)
+	    while(infoBZ2.type != FileTypeRegular || infoBZ2.time <= info.time)
 	    {
-		createBZ2(path);
-		
-		if(_traceLevel > 0)
+		createBZ2(path, _traceLevel > 0 ? _logger : LoggerPtr());
+
+		//
+		// If the source file size has changed after we
+		// created the BZ2 file, we try again.
+		//
+		FileInfo oldInfo = info;
+		info = getFileInfo(path, true);
+		infoBZ2 = getFileInfo(path + ".bz2", true); // Must exist.
+		if(info.size != oldInfo.size)
 		{
-		    Trace out(_logger, "IcePatch");
-		    out << "created BZ2 file for `" << path << "'";
+		    info.time = infoBZ2.time;
 		}
 	    }
 	}
diff --git a/cpp/src/IcePatch/Server.cpp b/cpp/src/IcePatch/Server.cpp
index 2547ec791ef..54103864ee7 100644
--- a/cpp/src/IcePatch/Server.cpp
+++ b/cpp/src/IcePatch/Server.cpp
@@ -242,14 +242,14 @@ IcePatch::Updater::cleanup(const FileDescSeq& fileDescSeq)
 	{
 	    //
 	    // Force MD5 files to be created and orphaned files to be
-	    // removed.
+	    // removed. Then recurse into subdirectories.
 	    //
 	    cleanup(directoryDesc->dir->getContents());
 
 	    //
-	    // Call describe(), because MD5 files in subdirectories
-	    // might have changed, resulting in a different summary
-	    // MD5 for this directory.
+	    // Call describe(), because BZ2 and MD5 files in the
+	    // directory might have changed, resulting in a different
+	    // summary MD5 for this directory.
 	    //
 	    directoryDesc->dir->describe();
 	}
diff --git a/cpp/src/IcePatch/Util.cpp b/cpp/src/IcePatch/Util.cpp
index d8ff53fb8a3..a413bd55f36 100644
--- a/cpp/src/IcePatch/Util.cpp
+++ b/cpp/src/IcePatch/Util.cpp
@@ -17,6 +17,7 @@
 #include <fstream>
 #include <sys/stat.h>
 #include <openssl/md5.h>
+#include <utime.h>
 #include <bzlib.h>
 
 #ifdef _WIN32
@@ -135,31 +136,76 @@ IcePatch::removeSuffix(const string& path)
 }
 
 FileInfo
-IcePatch::getFileInfo(const string& path, bool exceptionIfNotExist)
+IcePatch::getFileInfo(const string& path, bool exceptionIfNotExist, const Ice::LoggerPtr& logger)
 {
+    FileInfo result;
     struct stat buf;
-    if(::stat(path.c_str(), &buf) == -1)
+
+    while(true)
     {
-	if(!exceptionIfNotExist && errno == ENOENT)
+	if(::stat(path.c_str(), &buf) == -1)
+	{
+	    if(!exceptionIfNotExist && errno == ENOENT)
+	    {
+		FileInfo result;
+		result.size = 0;
+		result.time = 0;
+		result.type = FileTypeNotExist;
+		return result;
+	    }
+	    else
+	    {
+		FileAccessException ex;
+		ex.reason = "cannot stat `" + path + "': " + strerror(errno);
+		throw ex;
+	    }
+	}
+	
+	result.size = buf.st_size;
+	result.time = buf.st_mtime;
+	
+	if(IceUtil::Time::seconds(result.time) <= IceUtil::Time::now())
 	{
-	    FileInfo result;
-	    result.size = 0;
-	    result.time = 0;
-	    result.type = FileTypeNotExist;
-            return result;
+	    break;
 	}
-	else
+	
+	if(logger)
 	{
-	    FileAccessException ex;
-	    ex.reason = "cannot stat `" + path + "': " + strerror(errno);
-	    throw ex;
+	    Trace out(logger, "IcePatch");
+	    out << "modification time for `" << path << "' is in the future\n";
+	    out << "setting modification time to the current time.";
+	}
+	    
+	if(::utime(path.c_str(), 0) == -1)
+	{
+	    if(errno != ENOENT)
+	    {
+		FileAccessException ex;
+		ex.reason = "cannot utime `" + path + "': " + strerror(errno);
+		throw ex;
+	    }
+	}
+	
+	if(!ignoreSuffix(path))
+	{
+	    try
+	    {
+		removeRecursive(path + ".md5");
+	    }
+	    catch(const FileAccessException&)
+	    {
+	    }
+	    
+	    try
+	    {
+		removeRecursive(path + ".bz2");
+	    }
+	    catch(const FileAccessException&)
+	    {
+	    }
 	}
     }
 
-    FileInfo result;
-    result.size = buf.st_size;
-    result.time = buf.st_mtime;
-
     if(S_ISDIR(buf.st_mode))
     {
 	result.type = FileTypeDirectory;
@@ -179,7 +225,7 @@ IcePatch::getFileInfo(const string& path, bool exceptionIfNotExist)
 }
 
 void
-IcePatch::removeRecursive(const string& path)
+IcePatch::removeRecursive(const string& path, const Ice::LoggerPtr& logger)
 {
     if(getFileInfo(path, true).type == FileTypeDirectory)
     {
@@ -209,6 +255,12 @@ IcePatch::removeRecursive(const string& path)
 	    throw ex;
 	}
     }
+
+    if(logger)
+    {
+	Trace out(logger, "IcePatch");
+	out << "removed file `" << path << "'";
+    }
 }
 
 StringSeq
@@ -381,7 +433,7 @@ IcePatch::putMD5(const string& path, const ByteSeq& bytesMD5)
 }
 
 void
-IcePatch::createMD5(const string& path)
+IcePatch::createMD5(const string& path, const LoggerPtr& logger)
 {
     //
     // The current directory is not permissible for MD5 value
@@ -469,6 +521,12 @@ IcePatch::createMD5(const string& path)
     // Save the MD5 hash value.
     //
     putMD5(path, bytesMD5);
+
+    if(logger)
+    {
+	Trace out(logger, "IcePatch");
+	out << "created MD5 file for `" << path << "'";
+    }
 }
 
 ByteSeq
@@ -600,7 +658,7 @@ IcePatch::getBZ2(const string& path, Int pos, Int num)
 }
 
 void
-IcePatch::createBZ2(const string& path)
+IcePatch::createBZ2(const string& path, const Ice::LoggerPtr& logger)
 {
     FileInfo info = getFileInfo(path, true);
     if(info.type == FileTypeDirectory)
@@ -707,4 +765,10 @@ IcePatch::createBZ2(const string& path)
 	ex.reason = "cannot rename `" + pathBZ2Temp + "' to  `" + pathBZ2 + "': " + strerror(errno);
 	throw ex;
     }
+
+    if(logger)
+    {
+	Trace out(logger, "IcePatch");
+	out << "created BZ2 file for `" << path << "'";
+    }
 }