Issue 2265: Suppress messages about spurious dependency cycles.

(Jason Kenny)

6 files changed, 66 insertions, 47 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 0afce48..d415509 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -34,6 +34,11 @@ RELEASE 1.X - XXX
 
     - Handle Java inner classes declared within a method.
 
+  From Jason Kenny:
+
+    - Suppress mistaken reports of a dependency cycle when a child
+      left on the pending list is a single Node in EXECUTED state.
+
   From Steven Knight:
 
     - Fix label placement by the "scons-time.py func" subcommand
diff --git a/src/engine/SCons/Script/Interactive.py b/src/engine/SCons/Script/Interactive.py
index 4158f99..98a312b 100644
--- a/src/engine/SCons/Script/Interactive.py
+++ b/src/engine/SCons/Script/Interactive.py
@@ -258,14 +258,12 @@ class SConsInteractiveCmd(cmd.Cmd):
             # node.set_state() to reset it manually
             node.set_state(SCons.Node.no_state)
             node.implicit = None
-            # Make sure Taskmaster reference counts are reset to zero.
-            #
-            # TODO: Look for a way to avoid having to reset this here
-            # by making sure the Taskmaster will always end up reverting
-            # every Node's ref_count to 0 before terminating.  That may
-            # provide clues about intermittent phantom cycles that have
-            # been reported (e.g. issue 2265 at tigris.org).
-            node.ref_count = 0
+
+            # Debug:  Uncomment to verify that all Taskmaster reference
+            # counts have been reset to zero.
+            #if node.ref_count != 0:
+            #    from SCons.Debug import Trace
+            #    Trace('node %s, ref_count %s !!!\n' % (node, node.ref_count))
 
         SCons.SConsign.Reset()
         SCons.Script.Main.progress_display("scons: done clearing node information.")
diff --git a/src/engine/SCons/Taskmaster.py b/src/engine/SCons/Taskmaster.py
index 766d894..eefe56b 100644
--- a/src/engine/SCons/Taskmaster.py
+++ b/src/engine/SCons/Taskmaster.py
@@ -441,7 +441,7 @@ class Task:
             p.ref_count = p.ref_count - subtract
             if T: T.write(self.trace_message('Task.postprocess()',
                                              p,
-                                             'adjusting parent ref count'))
+                                             'adjusted parent ref count'))
             if p.ref_count == 0:
                 self.tm.candidates.append(p)
 
@@ -820,7 +820,7 @@ class Taskmaster:
                     # count so we can be put back on the list for
                     # re-evaluation when they've all finished.
                     node.ref_count =  node.ref_count + child.add_to_waiting_parents(node)
-                    if T: T.write(self.trace_message('     adjusting ref count: %s, child %s' %
+                    if T: T.write(self.trace_message('     adjusted ref count: %s, child %s' %
                                   (self.trace_node(node), repr(str(child)))))
 
                 if T:
@@ -908,7 +908,7 @@ class Taskmaster:
 
         if T:
             for n in nodes:
-                T.write(self.trace_message('       removing %s from the pending children set\n' %
+                T.write(self.trace_message('       removing node %s from the pending children set\n' %
                         self.trace_node(n)))
         try:
             while 1:
@@ -932,10 +932,10 @@ class Taskmaster:
                 to_visit = to_visit | parents
                 pending_children = pending_children - parents
 
-                if T:
-                    for p in parents:
-                        T.write(self.trace_message('       removing %s from the pending children set\n' %
-                                self.trace_node(p)))
+                for p in parents:
+                    p.ref_count = p.ref_count - 1
+                    if T: T.write(self.trace_message('       removing parent %s from the pending children set\n' %
+                                  self.trace_node(p)))
         except KeyError:
             # The container to_visit has been emptied.
             pass
@@ -955,15 +955,31 @@ class Taskmaster:
         """
         Check for dependency cycles.
         """
-        if self.pending_children:
-            desc = 'Found dependency cycle(s):\n'
-            for node in self.pending_children:
-                cycle = find_cycle([node], set())
-                if cycle:
-                    desc = desc + "  " + string.join(map(str, cycle), " -> ") + "\n"
-                else:
-                    desc = desc + \
-                        "  Internal Error: no cycle found for node %s (%s) in state %s\n" %  \
-                        (node, repr(node), StateString[node.get_state()])
-
-            raise SCons.Errors.UserError, desc
+        if not self.pending_children:
+            return
+
+        # TODO(1.5)
+        #nclist = [ (n, find_cycle([n], set())) for n in self.pending_children ]
+        nclist = map(lambda n: (n, find_cycle([n], set())), self.pending_children)
+
+        # TODO(1.5)
+        #genuine_cycles = [
+        #    node for node, cycle in nclist
+        #             if cycle or node.get_state() != NODE_EXECUTED
+        #]
+        genuine_cycles = filter(lambda t: t[1] or t[0].get_state() != NODE_EXECUTED, nclist)
+        if not genuine_cycles:
+            # All of the "cycles" found were single nodes in EXECUTED state,
+            # which is to say, they really weren't cycles.  Just return.
+            return
+
+        desc = 'Found dependency cycle(s):\n'
+        for node, cycle in nclist:
+            if cycle:
+                desc = desc + "  " + string.join(map(str, cycle), " -> ") + "\n"
+            else:
+                desc = desc + \
+                    "  Internal Error: no cycle found for node %s (%s) in state %s\n" %  \
+                    (node, repr(node), StateString[node.get_state()])
+
+        raise SCons.Errors.UserError, desc
diff --git a/src/engine/SCons/TaskmasterTests.py b/src/engine/SCons/TaskmasterTests.py
index 2031d23..b36e4aa 100644
--- a/src/engine/SCons/TaskmasterTests.py
+++ b/src/engine/SCons/TaskmasterTests.py
@@ -1067,7 +1067,7 @@ Taskmaster:        already handled (executed)
 Taskmaster:     Considering node <no_state   0   'n3'> and its children:
 Taskmaster:        <executed   0   'n1'>
 Taskmaster:        <no_state   0   'n2'>
-Taskmaster:      adjusting ref count: <pending    1   'n3'>, child 'n2'
+Taskmaster:      adjusted ref count: <pending    1   'n3'>, child 'n2'
 Taskmaster:     Considering node <no_state   0   'n2'> and its children:
 Taskmaster: Evaluating <pending    0   'n2'>
 
@@ -1076,7 +1076,7 @@ Task.prepare():      node <executing  0   'n2'>
 Task.execute():      node <executing  0   'n2'>
 Task.postprocess():  node <executing  0   'n2'>
 Task.postprocess():  removing <executing  0   'n2'>
-Task.postprocess():  adjusting parent ref count <pending    0   'n3'>
+Task.postprocess():  adjusted parent ref count <pending    0   'n3'>
 
 Taskmaster: Looking for a node to evaluate
 Taskmaster:     Considering node <pending    0   'n3'> and its children:
diff --git a/test/Interactive/taskmastertrace.py b/test/Interactive/taskmastertrace.py
index b274c65..4ccf46b 100644
--- a/test/Interactive/taskmastertrace.py
+++ b/test/Interactive/taskmastertrace.py
@@ -75,7 +75,7 @@ scons>>>
 Taskmaster: Looking for a node to evaluate
 Taskmaster:     Considering node <no_state   0   'foo.out'> and its children:
 Taskmaster:        <no_state   0   'foo.in'>
-Taskmaster:      adjusting ref count: <pending    1   'foo.out'>, child 'foo.in'
+Taskmaster:      adjusted ref count: <pending    1   'foo.out'>, child 'foo.in'
 Taskmaster:     Considering node <no_state   0   'foo.in'> and its children:
 Taskmaster: Evaluating <pending    0   'foo.in'>
 
@@ -84,7 +84,7 @@ Task.prepare():      node <up_to_date 0   'foo.in'>
 Task.executed_with_callbacks(): node <up_to_date 0   'foo.in'>
 Task.postprocess():  node <up_to_date 0   'foo.in'>
 Task.postprocess():  removing <up_to_date 0   'foo.in'>
-Task.postprocess():  adjusting parent ref count <pending    0   'foo.out'>
+Task.postprocess():  adjusted parent ref count <pending    0   'foo.out'>
 
 Taskmaster: Looking for a node to evaluate
 Taskmaster:     Considering node <pending    0   'foo.out'> and its children:
diff --git a/test/option/taskmastertrace.py b/test/option/taskmastertrace.py
index cd6e09d..23545c1 100644
--- a/test/option/taskmastertrace.py
+++ b/test/option/taskmastertrace.py
@@ -53,10 +53,10 @@ Taskmaster:        <no_state   0   'SConstruct'>
 Taskmaster:        <no_state   0   'Tfile.in'>
 Taskmaster:        <no_state   0   'Tfile.mid'>
 Taskmaster:        <no_state   0   'Tfile.out'>
-Taskmaster:      adjusting ref count: <pending    1   '.'>, child 'SConstruct'
-Taskmaster:      adjusting ref count: <pending    2   '.'>, child 'Tfile.in'
-Taskmaster:      adjusting ref count: <pending    3   '.'>, child 'Tfile.mid'
-Taskmaster:      adjusting ref count: <pending    4   '.'>, child 'Tfile.out'
+Taskmaster:      adjusted ref count: <pending    1   '.'>, child 'SConstruct'
+Taskmaster:      adjusted ref count: <pending    2   '.'>, child 'Tfile.in'
+Taskmaster:      adjusted ref count: <pending    3   '.'>, child 'Tfile.mid'
+Taskmaster:      adjusted ref count: <pending    4   '.'>, child 'Tfile.out'
 Taskmaster:     Considering node <no_state   0   'SConstruct'> and its children:
 Taskmaster: Evaluating <pending    0   'SConstruct'>
 
@@ -65,7 +65,7 @@ Task.prepare():      node <up_to_date 0   'SConstruct'>
 Task.executed_with_callbacks(): node <up_to_date 0   'SConstruct'>
 Task.postprocess():  node <up_to_date 0   'SConstruct'>
 Task.postprocess():  removing <up_to_date 0   'SConstruct'>
-Task.postprocess():  adjusting parent ref count <pending    3   '.'>
+Task.postprocess():  adjusted parent ref count <pending    3   '.'>
 
 Taskmaster: Looking for a node to evaluate
 Taskmaster:     Considering node <no_state   0   'Tfile.in'> and its children:
@@ -76,7 +76,7 @@ Task.prepare():      node <up_to_date 0   'Tfile.in'>
 Task.executed_with_callbacks(): node <up_to_date 0   'Tfile.in'>
 Task.postprocess():  node <up_to_date 0   'Tfile.in'>
 Task.postprocess():  removing <up_to_date 0   'Tfile.in'>
-Task.postprocess():  adjusting parent ref count <pending    2   '.'>
+Task.postprocess():  adjusted parent ref count <pending    2   '.'>
 
 Taskmaster: Looking for a node to evaluate
 Taskmaster:     Considering node <no_state   0   'Tfile.mid'> and its children:
@@ -90,7 +90,7 @@ Copy("Tfile.mid", "Tfile.in")
 Task.executed_with_callbacks(): node <executing  0   'Tfile.mid'>
 Task.postprocess():  node <executed   0   'Tfile.mid'>
 Task.postprocess():  removing <executed   0   'Tfile.mid'>
-Task.postprocess():  adjusting parent ref count <pending    1   '.'>
+Task.postprocess():  adjusted parent ref count <pending    1   '.'>
 
 Taskmaster: Looking for a node to evaluate
 Taskmaster:     Considering node <no_state   0   'Tfile.out'> and its children:
@@ -104,7 +104,7 @@ Copy("Tfile.out", "Tfile.mid")
 Task.executed_with_callbacks(): node <executing  0   'Tfile.out'>
 Task.postprocess():  node <executed   0   'Tfile.out'>
 Task.postprocess():  removing <executed   0   'Tfile.out'>
-Task.postprocess():  adjusting parent ref count <pending    0   '.'>
+Task.postprocess():  adjusted parent ref count <pending    0   '.'>
 
 Taskmaster: Looking for a node to evaluate
 Taskmaster:     Considering node <pending    0   '.'> and its children:
@@ -148,10 +148,10 @@ Taskmaster:        <no_state   0   'SConstruct'>
 Taskmaster:        <no_state   0   'Tfile.in'>
 Taskmaster:        <no_state   0   'Tfile.mid'>
 Taskmaster:        <no_state   0   'Tfile.out'>
-Taskmaster:      adjusting ref count: <pending    1   '.'>, child 'SConstruct'
-Taskmaster:      adjusting ref count: <pending    2   '.'>, child 'Tfile.in'
-Taskmaster:      adjusting ref count: <pending    3   '.'>, child 'Tfile.mid'
-Taskmaster:      adjusting ref count: <pending    4   '.'>, child 'Tfile.out'
+Taskmaster:      adjusted ref count: <pending    1   '.'>, child 'SConstruct'
+Taskmaster:      adjusted ref count: <pending    2   '.'>, child 'Tfile.in'
+Taskmaster:      adjusted ref count: <pending    3   '.'>, child 'Tfile.mid'
+Taskmaster:      adjusted ref count: <pending    4   '.'>, child 'Tfile.out'
 Taskmaster:     Considering node <no_state   0   'SConstruct'> and its children:
 Taskmaster: Evaluating <pending    0   'SConstruct'>
 
@@ -160,7 +160,7 @@ Task.prepare():      node <up_to_date 0   'SConstruct'>
 Task.executed_with_callbacks(): node <up_to_date 0   'SConstruct'>
 Task.postprocess():  node <up_to_date 0   'SConstruct'>
 Task.postprocess():  removing <up_to_date 0   'SConstruct'>
-Task.postprocess():  adjusting parent ref count <pending    3   '.'>
+Task.postprocess():  adjusted parent ref count <pending    3   '.'>
 
 Taskmaster: Looking for a node to evaluate
 Taskmaster:     Considering node <no_state   0   'Tfile.in'> and its children:
@@ -171,7 +171,7 @@ Task.prepare():      node <up_to_date 0   'Tfile.in'>
 Task.executed_with_callbacks(): node <up_to_date 0   'Tfile.in'>
 Task.postprocess():  node <up_to_date 0   'Tfile.in'>
 Task.postprocess():  removing <up_to_date 0   'Tfile.in'>
-Task.postprocess():  adjusting parent ref count <pending    2   '.'>
+Task.postprocess():  adjusted parent ref count <pending    2   '.'>
 
 Taskmaster: Looking for a node to evaluate
 Taskmaster:     Considering node <no_state   0   'Tfile.mid'> and its children:
@@ -184,7 +184,7 @@ Task.execute():      node <executing  0   'Tfile.mid'>
 Task.executed_with_callbacks(): node <executing  0   'Tfile.mid'>
 Task.postprocess():  node <executed   0   'Tfile.mid'>
 Task.postprocess():  removing <executed   0   'Tfile.mid'>
-Task.postprocess():  adjusting parent ref count <pending    1   '.'>
+Task.postprocess():  adjusted parent ref count <pending    1   '.'>
 
 Taskmaster: Looking for a node to evaluate
 Taskmaster:     Considering node <no_state   0   'Tfile.out'> and its children:
@@ -197,7 +197,7 @@ Task.execute():      node <executing  0   'Tfile.out'>
 Task.executed_with_callbacks(): node <executing  0   'Tfile.out'>
 Task.postprocess():  node <executed   0   'Tfile.out'>
 Task.postprocess():  removing <executed   0   'Tfile.out'>
-Task.postprocess():  adjusting parent ref count <pending    0   '.'>
+Task.postprocess():  adjusted parent ref count <pending    0   '.'>
 
 Taskmaster: Looking for a node to evaluate
 Taskmaster:     Considering node <pending    0   '.'> and its children: