Fix a problem with the new Parallel job support when a command fails. (J.T. Conklin)

4 files changed, 90 insertions, 24 deletions

diff --git a/src/engine/SCons/Job.py b/src/engine/SCons/Job.py
index 202f86f..0c01b85 100644
--- a/src/engine/SCons/Job.py
+++ b/src/engine/SCons/Job.py
@@ -147,12 +147,7 @@ class ThreadPool:
 
     def __init__(self, num):
         """Create the request and reply queues, and 'num' worker threads."""
-        # Ideally we wouldn't have to artificially limit the number of
-        # tasks that can be posted to the request queue.  But this can
-        # result in a large number of pending tasks, which at the time
-        # of this writing causes the taskmaster's next_task method to
-        # take a very long time.
-        self.requestQueue = Queue.Queue(num)
+        self.requestQueue = Queue.Queue()
         self.resultsQueue = Queue.Queue()
 
         # Create worker threads
@@ -200,6 +195,9 @@ class Parallel:
         self.taskmaster = taskmaster
         self.tp = ThreadPool(num)
 
+        self.jobs = 0
+        self.maxjobs = num
+
     def start(self):
         """Start the job. This will begin pulling tasks from the
         taskmaster and executing them, and return when there are no
@@ -207,31 +205,34 @@ class Parallel:
         an exception), then the job will stop."""
 
         while 1:
-            task = self.taskmaster.next_task()
-            if task is None:
-                break
+            if self.jobs < self.maxjobs:
+                task = self.taskmaster.next_task()
+                if task is None:
+                    break
 
-            # prepare task for execution
-            try:
-                task.prepare()
-            except KeyboardInterrupt:
-                raise
-            except:
-                # Let the failed() callback function arrange for the
-                # build to stop if that's appropriate.
-                task.failed()
+                # prepare task for execution
+                try:
+                    task.prepare()
+                except KeyboardInterrupt:
+                    raise
+                except:
+                    # Let the failed() callback function arrange for the
+                    # build to stop if that's appropriate.
+                    task.failed()
 
-            # dispatch task
-            self.tp.put(task)
+                # dispatch task
+                self.tp.put(task)
+                self.jobs = self.jobs + 1
 
             while 1:
                 try:
                     task, ok = self.tp.get_nowait()
                 except Queue.Empty:
-                    if not self.taskmaster.is_blocked():
+                    if not (self.jobs is self.maxjobs or self.taskmaster.is_blocked()):
                         break
                     task, ok = self.tp.get()
 
+                self.jobs = self.jobs - 1
                 if ok:
                     task.executed()
                 else:
diff --git a/src/engine/SCons/Taskmaster.py b/src/engine/SCons/Taskmaster.py
index 7760cfe..6eda8c1 100644
--- a/src/engine/SCons/Taskmaster.py
+++ b/src/engine/SCons/Taskmaster.py
@@ -140,6 +140,7 @@ class Task:
         """Explicit stop-the-build failure."""
         for t in self.targets:
             t.set_state(SCons.Node.failed)
+        self.tm.failed(self.node)
         self.tm.stop()
 
     def fail_continue(self):
@@ -366,6 +367,16 @@ class Taskmaster:
         self.ready = None
         self.pending = []
 
+    def failed(self, node):
+        try:
+            tlist = node.builder.targets(node)
+        except AttributeError:
+            tlist = [node]
+        for t in tlist:
+            self.executing.remove(t)
+        for side_effect in node.side_effects:
+            self.executing.remove(side_effect)
+
     def executed(self, node):
         try:
             tlist = node.builder.targets(node)
diff --git a/src/engine/SCons/TaskmasterTests.py b/src/engine/SCons/TaskmasterTests.py
index a394151..fd21891 100644
--- a/src/engine/SCons/TaskmasterTests.py
+++ b/src/engine/SCons/TaskmasterTests.py
@@ -535,6 +535,16 @@ class TaskmasterTestCase(unittest.TestCase):
         assert built_text == "MyTM.stop()"
         assert tm.next_task() is None
 
+    def test_failed(self):
+        """Test when a task has failed
+        """
+        n1 = Node("n1")
+        tm = SCons.Taskmaster.Taskmaster([n1])
+        t = tm.next_task()
+        assert tm.executing == [n1], tm.executing
+        tm.failed(n1)
+        assert tm.executing == [], tm.executing
+
     def test_executed(self):
         """Test when a task has been executed
         """
diff --git a/test/option-j.py b/test/option-j.py
index cf41930..9af8179 100644
--- a/test/option-j.py
+++ b/test/option-j.py
@@ -29,6 +29,7 @@ SConscript settable option.
 
 __revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
 
+import os.path
 import string
 import sys
 import TestSCons
@@ -75,7 +76,9 @@ def copyn(env, target, source):
     for t in target:
         shutil.copy(str(source[0]), str(t))
 
-t = env.Command(target=['foo/foo1.out', 'foo/foo2.out'], source='foo/foo.in', action=copyn)
+t = env.Command(target=['foo/foo1.out', 'foo/foo2.out'],
+                source='foo/foo.in',
+                action=copyn)
 env.Install('out', t)
 """ % python)
 
@@ -114,9 +117,13 @@ start2, finish1 = RunTest('f1 f2', "second")
 test.fail_test(start2 < finish1)
 
 # Make sure that a parallel build using a list builder
-# succeedes.
+# succeeds.
 test.run(arguments='-j 2 out')
 
+
+# Test that a failed build with -j works properly.
+
+
 # Test SetJobs() with no -j:
 test.write('SConstruct', """
 MyBuild = Builder(action = r'%s build.py $TARGETS')
@@ -174,5 +181,42 @@ start2, finish1 = RunTest('-j 1 f1 f2', "fourth")
 test.fail_test(start2 < finish1)
 
 
+# Test that a failed build with -j works properly.
+
+test.write('copy.py', r"""\
+import sys
+import time
+time.sleep(1)
+open(sys.argv[1], 'w').write(open(sys.argv[2], 'r').read())
+""")
+
+test.write('fail.py', r"""\
+import sys
+sys.exit(1)
+""")
+
+test.write('SConstruct', """
+MyCopy = Builder(action = r'%s copy.py $TARGET $SOURCE')
+Fail = Builder(action = r'%s fail.py $TARGETS $SOURCE')
+env = Environment(BUILDERS = { 'MyCopy' : MyCopy, 'Fail' : Fail })
+env.Fail(target = 'f3', source = 'f3.in')
+env.MyCopy(target = 'f4', source = 'f4.in')
+env.MyCopy(target = 'f5', source = 'f5.in')
+env.MyCopy(target = 'f6', source = 'f6.in')
+""" % (python, python))
+
+test.write('f3.in', "f3.in\n")
+test.write('f4.in', "f4.in\n")
+test.write('f5.in', "f5.in\n")
+test.write('f6.in', "f6.in\n")
+
+test.run(arguments = '-j 2 .',
+         status = 2,
+         stderr = "scons: *** [f3] Error 1\n")
+
+test.fail_test(os.path.exists(test.workpath('f3')))
+test.fail_test(test.read(test.workpath('f4')) != 'f4.in\n')
+test.fail_test(os.path.exists(test.workpath('f5'))) 
+test.fail_test(os.path.exists(test.workpath('f6'))) 
+
 test.pass_test()
-