Fix two bugs in CacheDir handling. (Jeff Petkau)

git-svn-id: http://scons.tigris.org/svn/scons/trunk@615 fdb21ef1-2011-0410-befe-b5e4ea1792b1

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index ca480deb3524c45f0dadc0dfd43998d15723efd7..70ac65546d085a8100339ba7aeae14ea6666cfe8 100644 (file)
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -84,6 +84,13 @@ RELEASE 0.12 - XXX
 
   - Fix WhereIs() to return normalized paths.
 
+  From Jeff Petkau:
+
+  - Don't copy a built file to a CacheDir() if it's already there.
+
+  - Avoid partial copies of built files in a CacheDir() by copying
+    to a temporary file and renaming.
+
 
 
 RELEASE 0.11 - Tue, 11 Feb 2003 05:24:33 -0600

diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py
index b0bc3e4306e6b0d6b9323e7caebdeef61984a1ef..e5182cd884ae23dfede003a17a67a09ba086f9bc 100644 (file)
--- a/src/engine/SCons/Node/FS.py
+++ b/src/engine/SCons/Node/FS.py
@@ -130,11 +130,27 @@ CacheRetrieveSilent = SCons.Action.Action(CacheRetrieveFunc, None)
 def CachePushFunc(target, source, env):
     t = target[0]
     cachedir, cachefile = t.cachepath()
+    if os.path.exists(cachefile):
+        # Don't bother copying it if it's already there.
+        return
+
     if not os.path.isdir(cachedir):
         os.mkdir(cachedir)
-    shutil.copy2(t.path, cachefile)
-    st = os.stat(t.path)
-    os.chmod(cachefile, stat.S_IMODE(st[stat.ST_MODE]) | stat.S_IWRITE)
+
+    tempfile = cachefile+'.tmp'
+    try:
+        shutil.copy2(t.path, tempfile)
+        os.rename(tempfile, cachefile)
+        st = os.stat(t.path)
+        os.chmod(cachefile, stat.S_IMODE(st[stat.ST_MODE]) | stat.S_IWRITE)
+    except OSError:
+        # It's possible someone else tried writing the file at the same
+        # time we did.  Print a warning but don't stop the build, since
+        # it doesn't affect the correctness of the build.
+        SCons.Warnings.warn(SCons.Warnings.CacheWriteErrorWarning,
+                            "Unable to copy %s to cache. Cache file is %s"
+                                % (str(target), cachefile))
+        return
 
 CachePush = SCons.Action.Action(CachePushFunc, None)
 

diff --git a/src/engine/SCons/Node/FSTests.py b/src/engine/SCons/Node/FSTests.py
index 3a862ebe1e5633d2b40b9ef08549f4d508e3b323..294a4bb51eb6b5a19bc583dfcfda0d33e3cabaca 100644 (file)
--- a/src/engine/SCons/Node/FSTests.py
+++ b/src/engine/SCons/Node/FSTests.py
@@ -29,12 +29,14 @@ import string
 import sys
 import time
 import unittest
-import SCons.Node.FS
 from TestCmd import TestCmd
-import SCons.Errors
 import shutil
 import stat
 
+import SCons.Errors
+import SCons.Node.FS
+import SCons.Warnings
+
 built_it = None
 
 # This will be built-in in 2.3.  For now fake it.
@@ -1276,6 +1278,8 @@ class SConstructTestCase(unittest.TestCase):
 class CacheDirTestCase(unittest.TestCase):
     def runTest(self):
         """Test CacheDir functionality"""
+        test = TestCmd(workdir='')
+
         global built_it
 
         fs = SCons.Node.FS.FS()
@@ -1350,8 +1354,6 @@ class CacheDirTestCase(unittest.TestCase):
         SCons.Node.FS.CachePush = push
 
         try:
-            test = TestCmd(workdir='')
-
             self.pushed = []
 
             cd_f3 = test.workpath("cd.f3")
@@ -1377,6 +1379,8 @@ class CacheDirTestCase(unittest.TestCase):
         finally:
             SCons.Node.FS.CachePush = save_CachePush
 
+        # Verify how the cachepath() method determines the name
+        # of the file in cache.
         f5 = fs.File("cd.f5")
         f5.set_bsig('a_fake_bsig')
         cp = f5.cachepath()
@@ -1384,15 +1388,46 @@ class CacheDirTestCase(unittest.TestCase):
         filename = os.path.join(dirname, 'a_fake_bsig')
         assert cp == (dirname, filename), cp
 
+        # Verify that no bsig raises an InternalERror
         f6 = fs.File("cd.f6")
-        f5.set_bsig(None)
+        f6.set_bsig(None)
         exc_caught = 0
         try:
-            cp = f5.cachepath()
+            cp = f6.cachepath()
         except SCons.Errors.InternalError:
             exc_caught = 1
         assert exc_caught
 
+        # Verify that we raise a warning if we can't copy a file to cache.
+        save_copy2 = shutil.copy2
+        def copy2(src, dst):
+            raise OSError
+        shutil.copy2 = copy2
+        save_mkdir = os.mkdir
+        def mkdir(dir):
+            pass
+        os.mkdir = mkdir
+        old_warn_exceptions = SCons.Warnings.warningAsException(1)
+        SCons.Warnings.enableWarningClass(SCons.Warnings.CacheWriteErrorWarning)
+
+        try:
+            cd_f7 = test.workpath("cd.f7")
+            test.write(cd_f7, "cd.f7\n")
+            f7 = fs.File(cd_f7)
+            f7.set_bsig('f7_bsig')
+
+            warn_caught = 0
+            try:
+                f7.built()
+            except SCons.Warnings.CacheWriteErrorWarning:
+                warn_caught = 1
+            assert warn_caught
+        finally:
+            shutil.copy2 = save_copy2
+            os.mkdir = save_mkdir
+            SCons.Warnings.warningAsException(old_warn_exceptions)
+            SCons.Warnings.suppressWarningClass(SCons.Warnings.CacheWriteErrorWarning)
+
 
 
 if __name__ == "__main__":

diff --git a/src/engine/SCons/Warnings.py b/src/engine/SCons/Warnings.py
index 6d75c0628d78ceb878df30fe2efb3bad81331a45..436e66332f82367a8d1ba67aea382c86f4fd23d7 100644 (file)
--- a/src/engine/SCons/Warnings.py
+++ b/src/engine/SCons/Warnings.py
@@ -46,6 +46,9 @@ class CorruptSConsignWarning(Warning):
 class ReservedVariableWarning(Warning):
     pass
 
+class CacheWriteErrorWarning(Warning):
+    pass
+
 _warningAsException = 0
 
 # The below is a list of 2-tuples.  The first element is a class object.