- added simple test for #2720 (execute with cached targets)

- fixed two typos
- updated list of changes

4 files changed, 101 insertions, 3 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index fbe4f250..b34350fa 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -4,6 +4,18 @@
 
                             Change Log
 
+RELEASE 2.X.X - 
+
+  From Alexey Klimkin:
+  
+    - Fixed the Taskmaster, curing spurious build failures in
+      multi-threaded runs (#2720).
+    
+  From Dirk Baechle:
+  
+    - Improved documentation of command-line variables (#2809).
+    - Fixed scons-doc.py to properly convert main XML files (#2812).
+
 
 RELEASE 2.1.0 - Mon, 09 Sep 2011 20:54:57 -0700
 
diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py
index f106d46d..f31ca831 100644
--- a/src/engine/SCons/Node/FS.py
+++ b/src/engine/SCons/Node/FS.py
@@ -2715,7 +2715,7 @@ class File(Base):
         so only do thread safe stuff here. Do thread unsafe stuff in
         built().
 
-        Returns true iff the node was successfully retrieved.
+        Returns true if the node was successfully retrieved.
         """
         if self.nocache:
             return None
diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py
index 69d28983..992284dc 100644
--- a/src/engine/SCons/Node/__init__.py
+++ b/src/engine/SCons/Node/__init__.py
@@ -307,7 +307,7 @@ class Node(object):
         so only do thread safe stuff here. Do thread unsafe stuff in
         built().
 
-        Returns true iff the node was successfully retrieved.
+        Returns true if the node was successfully retrieved.
         """
         return 0
 
diff --git a/src/engine/SCons/TaskmasterTests.py b/src/engine/SCons/TaskmasterTests.py
index 4c565384..85ade8df 100644
--- a/src/engine/SCons/TaskmasterTests.py
+++ b/src/engine/SCons/TaskmasterTests.py
@@ -86,6 +86,7 @@ class Node(object):
 
     def prepare(self):
         self.prepared = 1
+        self.get_binfo()        
 
     def build(self):
         global built_text
@@ -94,11 +95,52 @@ class Node(object):
     def remove(self):
         pass
 
+    # The following four methods new_binfo(), del_binfo(),
+    # get_binfo(), clear() as well as its calls have been added
+    # to support the cached_execute() test (issue #2720).
+    # They are full copies (or snippets) of their actual
+    # counterparts in the Node class...
+    def new_binfo(self):
+        binfo = "binfo"
+        return binfo
+
+    def del_binfo(self):
+        """Delete the build info from this node."""
+        try:
+            delattr(self, 'binfo')
+        except AttributeError:
+            pass
+
+    def get_binfo(self):
+        """Fetch a node's build information."""
+        try:
+            return self.binfo
+        except AttributeError:
+            pass
+
+        binfo = self.new_binfo()
+        self.binfo = binfo
+
+        return binfo
+    
+    def clear(self):
+        # The del_binfo() call here isn't necessary for normal execution,
+        # but is for interactive mode, where we might rebuild the same
+        # target and need to start from scratch.
+        self.del_binfo()
+
     def built(self):
         global built_text
         if not self.cached:
             built_text = built_text + " really"
-
+            
+        # Clear the implicit dependency caches of any Nodes
+        # waiting for this Node to be built.
+        for parent in self.waiting_parents:
+            parent.implicit = None
+
+        self.clear()
+        
     def has_builder(self):
         return not self.builder is None
 
@@ -957,6 +999,50 @@ class TaskmasterTestCase(unittest.TestCase):
         assert built_text is None, built_text
         assert cache_text == ["n7 retrieved", "n8 retrieved"], cache_text
 
+    def test_cached_execute(self):
+        """Test executing a task with cached targets
+        """
+        # In issue #2720 Alexei Klimkin detected that the previous
+        # workflow for execute() led to problems in a multithreaded build.
+        # We have:
+        #    task.prepare()
+        #    task.execute()
+        #    task.executed()
+        #        -> node.visited()
+        # for the Serial flow, but
+        #    - Parallel -           - Worker -
+        #      task.prepare()
+        #      requestQueue.put(task)
+        #                           task = requestQueue.get()
+        #                           task.execute()
+        #                           resultQueue.put(task)
+        #      task = resultQueue.get()
+        #      task.executed()
+        #        ->node.visited()
+        # in parallel. Since execute() used to call built() when a target
+        # was cached, it could unblock dependent nodes before the binfo got
+        # restored again in visited(). This resulted in spurious
+        # "file not found" build errors, because files fetched from cache would
+        # be seen as not up to date and wouldn't be scanned for implicit
+        # dependencies.
+        #
+        # The following test ensures that execute() only marks targets as cached,
+        # but the actual call to built() happens in executed() only.
+        # Like this, the binfo should still be intact after calling execute()...
+        global cache_text
+
+        n1 = Node("n1")
+        # Mark the node as being cached
+        n1.cached = 1
+        tm = SCons.Taskmaster.Taskmaster([n1])
+        t = tm.next_task()
+        t.prepare()
+        t.execute()
+        assert cache_text == ["n1 retrieved"], cache_text
+        # If no binfo exists anymore, something has gone wrong...
+        has_binfo = hasattr(n1, 'binfo')
+        assert has_binfo == True, has_binfo
+
     def test_exception(self):
         """Test generic Taskmaster exception handling