Move the set_state(EXECUTED) call associated with pulling things

out of a CacheDir from CacheDir.py to Taskmaster.py.  Ripple effects
include adding a Node.FS.push_to_cache() method, so that logic can
be invoked separately from the Node.built() method.

8 files changed, 53 insertions, 34 deletions

diff --git a/src/engine/SCons/CacheDir.py b/src/engine/SCons/CacheDir.py
index c5ec14e..73bfe6b 100644
--- a/src/engine/SCons/CacheDir.py
+++ b/src/engine/SCons/CacheDir.py
@@ -190,22 +190,16 @@ class CacheDir:
         if not self.is_enabled():
             return False
 
-        retrieved = False
-
+        env = node.get_build_env()
         if cache_show:
-            if CacheRetrieveSilent(node, [], node.get_build_env(), execute=1) == 0:
+            if CacheRetrieveSilent(node, [], env, execute=1) == 0:
                 node.build(presub=0, execute=0)
-                retrieved = 1
+                return True
         else:
-            if CacheRetrieve(node, [], node.get_build_env(), execute=1) == 0:
-                retrieved = 1
-        if retrieved:
-            # Record build signature information, but don't
-            # push it out to cache.  (We just got it from there!)
-            node.set_state(SCons.Node.executed)
-            SCons.Node.Node.built(node)
-
-        return retrieved
+            if CacheRetrieve(node, [], env, execute=1) == 0:
+                return True
+
+        return False
 
     def push(self, node):
         if not self.is_enabled():
diff --git a/src/engine/SCons/CacheDirTests.py b/src/engine/SCons/CacheDirTests.py
index fd36cce..5e63548 100644
--- a/src/engine/SCons/CacheDirTests.py
+++ b/src/engine/SCons/CacheDirTests.py
@@ -194,10 +194,10 @@ class FileTestCase(BaseTestCase):
 
             cd_f3 = self.test.workpath("cd.f3")
             f3 = self.File(cd_f3)
-            f3.built()
+            f3.push_to_cache()
             assert self.pushed == [], self.pushed
             self.test.write(cd_f3, "cd.f3\n")
-            f3.built()
+            f3.push_to_cache()
             assert self.pushed == [f3], self.pushed
 
             self.pushed = []
@@ -240,7 +240,7 @@ class FileTestCase(BaseTestCase):
 
             warn_caught = 0
             try:
-                f7.built()
+                f7.push_to_cache()
             except SCons.Errors.BuildError, e:
                 assert e.exc_info[0] == SCons.Warnings.CacheWriteErrorWarning
                 warn_caught = 1
diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py
index 62d39dd..a4036ab 100644
--- a/src/engine/SCons/Node/FS.py
+++ b/src/engine/SCons/Node/FS.py
@@ -2562,6 +2562,22 @@ class File(Base):
         # created.
         self.dir._create()
 
+    def push_to_cache(self):
+        """Try to push the node into a cache
+        """
+        # This should get called before the Nodes' .built() method is
+        # called, which would clear the build signature if the file has
+        # a source scanner.
+        #
+        # We have to clear the local memoized values *before* we push
+        # the node to cache so that the memoization of the self.exists()
+        # return value doesn't interfere.
+        if self.nocache:
+            return
+        self.clear_memoized_values()
+        if self.exists():
+            self.get_build_env().get_CacheDir().push(self)
+
     def retrieve_from_cache(self):
         """Try to retrieve the node's content from a cache
 
@@ -2577,22 +2593,6 @@ class File(Base):
             return None
         return self.get_build_env().get_CacheDir().retrieve(self)
 
-    def built(self):
-        """
-        Called just after this node is successfully built.
-        """
-        # Push this file out to cache before the superclass Node.built()
-        # method has a chance to clear the build signature, which it
-        # will do if this file has a source scanner.
-        #
-        # We have to clear the memoized values *before* we push it to
-        # cache so that the memoization of the self.exists() return
-        # value doesn't interfere.
-        self.clear_memoized_values()
-        if self.exists():
-            self.get_build_env().get_CacheDir().push(self)
-        SCons.Node.Node.built(self)
-
     def visited(self):
         if self.exists():
             self.get_build_env().get_CacheDir().push_if_forced(self)
diff --git a/src/engine/SCons/Node/NodeTests.py b/src/engine/SCons/Node/NodeTests.py
index ce56d5a..4612d3f 100644
--- a/src/engine/SCons/Node/NodeTests.py
+++ b/src/engine/SCons/Node/NodeTests.py
@@ -435,6 +435,12 @@ class NodeTestCase(unittest.TestCase):
         assert n.cleared, n.cleared
         assert n.ninfo.updated, n.ninfo.cleared
 
+    def test_push_to_cache(self):
+        """Test the base push_to_cache() method"""
+        n = SCons.Node.Node()
+        r = n.push_to_cache()
+        assert r is None, r
+
     def test_retrieve_from_cache(self):
         """Test the base retrieve_from_cache() method"""
         n = SCons.Node.Node()
diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py
index 6852f69..ee9083c 100644
--- a/src/engine/SCons/Node/__init__.py
+++ b/src/engine/SCons/Node/__init__.py
@@ -297,6 +297,11 @@ class Node:
         except AttributeError:
             pass
 
+    def push_to_cache(self):
+        """Try to push a node into a cache
+        """
+        pass
+
     def retrieve_from_cache(self):
         """Try to retrieve the node's content from a cache
 
diff --git a/src/engine/SCons/SConfTests.py b/src/engine/SCons/SConfTests.py
index 24cfb8d..2fa173d 100644
--- a/src/engine/SCons/SConfTests.py
+++ b/src/engine/SCons/SConfTests.py
@@ -207,6 +207,8 @@ class SConfTestCase(unittest.TestCase):
                         return None
                     def prepare(self):
                         pass
+                    def push_to_cache(self):
+                        pass
                     def retrieve_from_cache(self):
                         return 0
                     def build(self, **kw):
diff --git a/src/engine/SCons/Taskmaster.py b/src/engine/SCons/Taskmaster.py
index 8e71be9..91eac35 100644
--- a/src/engine/SCons/Taskmaster.py
+++ b/src/engine/SCons/Taskmaster.py
@@ -223,7 +223,14 @@ class Task:
         try:
             everything_was_cached = 1
             for t in self.targets:
-                if not t.retrieve_from_cache():
+                if t.retrieve_from_cache():
+                    # Call the .built() method without calling the
+                    # .push_to_cache() method, since we just got the
+                    # target from the cache and don't need to push
+                    # it back there.
+                    t.set_state(NODE_EXECUTED)
+                    t.built()
+                else:
                     everything_was_cached = 0
                     break
             if not everything_was_cached:
@@ -279,6 +286,7 @@ class Task:
                 for side_effect in t.side_effects:
                     side_effect.set_state(NODE_NO_STATE)
                 t.set_state(NODE_EXECUTED)
+                t.push_to_cache()
                 t.built()
             t.visited()
 
diff --git a/src/engine/SCons/TaskmasterTests.py b/src/engine/SCons/TaskmasterTests.py
index 282f4df..6c80230 100644
--- a/src/engine/SCons/TaskmasterTests.py
+++ b/src/engine/SCons/TaskmasterTests.py
@@ -69,6 +69,9 @@ class Node:
     def disambiguate(self):
         return self
 
+    def push_to_cache(self):
+        pass
+
     def retrieve_from_cache(self):
         global cache_text
         if self.cached:
@@ -87,7 +90,8 @@ class Node:
 
     def built(self):
         global built_text
-        built_text = built_text + " really"
+        if not self.cached:
+            built_text = built_text + " really"
 
     def has_builder(self):
         return not self.builder is None