Fix --no-exec handling of cache. (Kevin Quick)

4 files changed, 47 insertions, 7 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index dc1922b..61fafae 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -145,6 +145,9 @@ RELEASE 0.97 - XXX
   - Fix handling of src_suffix values that aren't extensions (don't
     begin with a '.').
 
+  - Don't retrieve files from a CacheDir, but report what would happen,
+    when the -n option is used.
+
   From Christoph Wiedemann:
 
   - Add an Environment.SetDefault() method that only sets values if
diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py
index 5015c84..be4770f 100644
--- a/src/engine/SCons/Node/FS.py
+++ b/src/engine/SCons/Node/FS.py
@@ -202,9 +202,10 @@ def CacheRetrieveFunc(target, source, env):
     fs = t.fs
     cachedir, cachefile = t.cachepath()
     if fs.exists(cachefile):
-        fs.copy2(cachefile, t.path)
-        st = fs.stat(cachefile)
-        fs.chmod(t.path, stat.S_IMODE(st[stat.ST_MODE]) | stat.S_IWRITE)
+        if SCons.Action.execute_actions:
+            fs.copy2(cachefile, t.path)
+            st = fs.stat(cachefile)
+            fs.chmod(t.path, stat.S_IMODE(st[stat.ST_MODE]) | stat.S_IWRITE)
         return 0
     return 1
 
@@ -1553,6 +1554,25 @@ class File(Base):
         so only do thread safe stuff here. Do thread unsafe stuff in
         built().
 
+        Note that there's a special trick here with the execute flag
+        (one that's not normally done for other actions).  Basically
+        if the user requested a noexec (-n) build, then
+        SCons.Action.execute_actions is set to 0 and when any action
+        is called, it does its showing but then just returns zero
+        instead of actually calling the action execution operation.
+        The problem for caching is that if the file does NOT exist in
+        cache then the CacheRetrieveString won't return anything to
+        show for the task, but the Action.__call__ won't call
+        CacheRetrieveFunc; instead it just returns zero, which makes
+        the code below think that the file *was* successfully
+        retrieved from the cache, therefore it doesn't do any
+        subsequent building.  However, the CacheRetrieveString didn't
+        print anything because it didn't actually exist in the cache,
+        and no more build actions will be performed, so the user just
+        sees nothing.  The fix is to tell Action.__call__ to always
+        execute the CacheRetrieveFunc and then have the latter
+        explicitly check SCons.Action.execute_actions itself.
+
         Returns true iff the node was successfully retrieved.
         """
         b = self.is_derived()
@@ -1560,10 +1580,10 @@ class File(Base):
             return None
         if b and self.fs.CachePath:
             if self.fs.cache_show:
-                if CacheRetrieveSilent(self, [], None) == 0:
+                if CacheRetrieveSilent(self, [], None, execute=1) == 0:
                     self.build(presub=0, execute=0)
                     return 1
-            elif CacheRetrieve(self, [], None) == 0:
+            elif CacheRetrieve(self, [], None, execute=1) == 0:
                 return 1
         return None
 
diff --git a/src/engine/SCons/Node/FSTests.py b/src/engine/SCons/Node/FSTests.py
index 4c086dd..6bee431 100644
--- a/src/engine/SCons/Node/FSTests.py
+++ b/src/engine/SCons/Node/FSTests.py
@@ -1552,10 +1552,10 @@ class CacheDirTestCase(unittest.TestCase):
 
         save_CacheRetrieve = SCons.Node.FS.CacheRetrieve
         self.retrieved = []
-        def retrieve_succeed(target, source, env, self=self):
+        def retrieve_succeed(target, source, env, self=self, execute=1):
             self.retrieved.append(target)
             return 0
-        def retrieve_fail(target, source, env, self=self):
+        def retrieve_fail(target, source, env, self=self, execute=1):
             self.retrieved.append(target)
             return 1
 
diff --git a/test/CacheDir.py b/test/CacheDir.py
index ae3f76e..67d3ed4 100644
--- a/test/CacheDir.py
+++ b/test/CacheDir.py
@@ -65,6 +65,23 @@ test.write(['src', 'bbb.in'], "bbb.in\n")
 test.write(['src', 'ccc.in'], "ccc.in\n")
 
 #############################################################################
+
+# Verify that building with -n and an empty cache reports that proper
+# build operations would be taken, but that nothing is actually built
+# and that the cache is still empty.
+test.run(chdir = 'src', arguments = '-n .', stdout = test.wrap_stdout("""\
+cat(["aaa.out"], ["aaa.in"])
+cat(["bbb.out"], ["bbb.in"])
+cat(["ccc.out"], ["ccc.in"])
+cat(["all"], ["aaa.out", "bbb.out", "ccc.out"])
+"""))
+
+test.must_not_exist(test.workpath('src', 'aaa.out'))
+test.must_not_exist(test.workpath('src', 'bbb.out'))
+test.must_not_exist(test.workpath('src', 'ccc.out'))
+test.must_not_exist(test.workpath('src', 'all'))
+test.fail_test(len(os.listdir(test.workpath('cache1'))))
+
 # Verify that a normal build works correctly, and clean up.
 # This should populate the cache with our derived files.
 test.run(chdir = 'src', arguments = '.')