Fix -k sometimes trying to link executables even when library builds fail.

5 files changed, 94 insertions, 21 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index be3522b..7a6ef02 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -40,6 +40,15 @@ RELEASE 0.97 - XXX
     interrupting builds by writing to a temporary file and renaming,
     not writing the file directly.
 
+  - Fix a 0.96 regression where when running with -k, targets built from
+    walking dependencies later on the command line would not realize
+    that a dependency had failed an earlier build attempt, and would
+    try to rebuild the dependent targets.
+
+  - Change the final messages when using -k and errors occur from
+    "{building,cleaning} terminated because of errors" to "done
+    {building,cleaning} targets (errors occurred during {build,clean})."
+
   From Elliot Murphy:
 
   - Enhance the tests to guarantee persistence of ListOption
diff --git a/src/engine/SCons/Script/__init__.py b/src/engine/SCons/Script/__init__.py
index 5dd907c..dfee767 100644
--- a/src/engine/SCons/Script/__init__.py
+++ b/src/engine/SCons/Script/__init__.py
@@ -1008,7 +1008,10 @@ def _main(args, parser):
     task_class = BuildTask	# default action is to build targets
     opening_message = "Building targets ..."
     closing_message = "done building targets."
-    failure_message = "building terminated because of errors."
+    if keep_going_on_error:
+        failure_message = "done building targets (errors occurred during build)."
+    else:
+        failure_message = "building terminated because of errors."
     if options.question:
         task_class = QuestionTask
     try:
@@ -1016,7 +1019,10 @@ def _main(args, parser):
             task_class = CleanTask
             opening_message = "Cleaning targets ..."
             closing_message = "done cleaning targets."
-            failure_message = "cleaning terminated because of errors."
+            if keep_going_on_error:
+                closing_message = "done cleaning targets (errors occurred during clean)."
+            else:
+                failure_message = "cleaning terminated because of errors."
     except AttributeError:
         pass
 
diff --git a/src/engine/SCons/Taskmaster.py b/src/engine/SCons/Taskmaster.py
index 5bfa3d6..8f9839e 100644
--- a/src/engine/SCons/Taskmaster.py
+++ b/src/engine/SCons/Taskmaster.py
@@ -282,6 +282,16 @@ class Taskmaster:
                 self.ready = node
                 break
 
+            # Skip this node if any of its children have failed.  This
+            # catches the case where we're descending a top-level target
+            # and one of our children failed while trying to be built
+            # by a *previous* descent of an earlier top-level target.
+            def failed(node): return node.get_state() == SCons.Node.failed
+            if filter(failed, children):
+                node.set_state(SCons.Node.failed)
+                self.candidates.pop()
+                continue
+
             # Detect dependency cycles:
             def in_stack(node): return node.get_state() == SCons.Node.stack
             cycle = filter(in_stack, children)
diff --git a/src/engine/SCons/TaskmasterTests.py b/src/engine/SCons/TaskmasterTests.py
index 040d52a..ef7f51a 100644
--- a/src/engine/SCons/TaskmasterTests.py
+++ b/src/engine/SCons/TaskmasterTests.py
@@ -422,6 +422,12 @@ class TaskmasterTestCase(unittest.TestCase):
         assert t.get_target() == n7
         t.executed()
 
+        n1 = Node("n1")
+        n2 = Node("n2", [n1])
+        n1.set_state(SCons.Node.failed)
+        tm = SCons.Taskmaster.Taskmaster([n2])
+        assert tm.next_task() is None
+
 
     def test_make_ready_out_of_date(self):
         """Test the Task.make_ready() method's list of out-of-date Nodes
diff --git a/test/option-k.py b/test/option-k.py
index 8950f5f..45c31d2 100644
--- a/test/option-k.py
+++ b/test/option-k.py
@@ -32,6 +32,10 @@ python = TestSCons.python
 
 test = TestSCons.TestSCons()
 
+test.subdir('work1', 'work2')
+
+
+
 test.write('succeed.py', r"""
 import sys
 file = open(sys.argv[1], 'wb')
@@ -45,43 +49,81 @@ import sys
 sys.exit(1)
 """)
 
-test.write('SConstruct', """
-Succeed = Builder(action = r'%s succeed.py $TARGETS')
-Fail = Builder(action = r'%s fail.py $TARGETS')
+test.write(['work1', 'SConstruct'], """\
+Succeed = Builder(action = r'%s ../succeed.py $TARGETS')
+Fail = Builder(action = r'%s ../fail.py $TARGETS')
 env = Environment(BUILDERS = { 'Succeed' : Succeed, 'Fail' : Fail })
 env.Fail(target = 'aaa.1', source = 'aaa.in')
 env.Succeed(target = 'aaa.out', source = 'aaa.1')
 env.Succeed(target = 'bbb.out', source = 'bbb.in')
 """ % (python, python))
 
-test.write('aaa.in', "aaa.in\n")
-test.write('bbb.in', "bbb.in\n")
+test.write(['work1', 'aaa.in'], "aaa.in\n")
+test.write(['work1', 'bbb.in'], "bbb.in\n")
 
-test.run(arguments = 'aaa.out bbb.out',
+test.run(chdir = 'work1',
+         arguments = 'aaa.out bbb.out',
          stderr = 'scons: *** [aaa.1] Error 1\n',
          status = 2)
 
-test.fail_test(os.path.exists(test.workpath('aaa.1')))
-test.fail_test(os.path.exists(test.workpath('aaa.out')))
-test.fail_test(os.path.exists(test.workpath('bbb.out')))
+test.must_not_exist(test.workpath('work1', 'aaa.1'))
+test.must_not_exist(test.workpath('work1', 'aaa.out'))
+test.must_not_exist(test.workpath('work1', 'bbb.out'))
 
-test.run(arguments = '-k aaa.out bbb.out',
+test.run(chdir = 'work1',
+         arguments = '-k aaa.out bbb.out',
          stderr = 'scons: *** [aaa.1] Error 1\n',
          status = 2)
 
-test.fail_test(os.path.exists(test.workpath('aaa.1')))
-test.fail_test(os.path.exists(test.workpath('aaa.out')))
-test.fail_test(test.read('bbb.out') != "succeed.py: bbb.out\n")
+test.must_not_exist(test.workpath('work1', 'aaa.1'))
+test.must_not_exist(test.workpath('work1', 'aaa.out'))
+test.must_match(['work1', 'bbb.out'], "succeed.py: bbb.out\n")
 
-test.unlink("bbb.out")
+test.unlink(['work1', 'bbb.out'])
 
-test.run(arguments = '--keep-going aaa.out bbb.out',
+test.run(chdir = 'work1',
+         arguments = '--keep-going aaa.out bbb.out',
          stderr = 'scons: *** [aaa.1] Error 1\n',
          status = 2)
 
-test.fail_test(os.path.exists(test.workpath('aaa.1')))
-test.fail_test(os.path.exists(test.workpath('aaa.out')))
-test.fail_test(test.read('bbb.out') != "succeed.py: bbb.out\n")
+test.must_not_exist(test.workpath('work1', 'aaa.1'))
+test.must_not_exist(test.workpath('work1', 'aaa.out'))
+test.must_match(['work1', 'bbb.out'], "succeed.py: bbb.out\n")
+
+
+
+test.write(['work2', 'SConstruct'], """\
+Succeed = Builder(action = r'%s ../succeed.py $TARGETS')
+Fail = Builder(action = r'%s ../fail.py $TARGETS')
+env = Environment(BUILDERS = { 'Succeed' : Succeed, 'Fail' : Fail })
+env.Fail('aaa.out', 'aaa.in')
+env.Succeed('bbb.out', 'aaa.out')
+env.Succeed('ccc.out', 'ccc.in')
+env.Succeed('ddd.out', 'ccc.in')
+""" % (python, python))
+
+test.write(['work2', 'aaa.in'], "aaa.in\n")
+test.write(['work2', 'ccc.in'], "ccc.in\n")
+
+test.run(chdir = 'work2',
+         arguments = '-k .',
+         status = 2,
+         stderr = None,
+         stdout = """\
+scons: Reading SConscript files ...
+scons: done reading SConscript files.
+scons: Building targets ...
+%s ../fail.py aaa.out
+%s ../succeed.py ccc.out
+%s ../succeed.py ddd.out
+scons: done building targets (errors occurred during build).
+""" % (python, python, python))
+
+test.must_not_exist(['work2', 'aaa.out'])
+test.must_not_exist(['work2', 'bbb.out'])
+test.must_match(['work2', 'ccc.out'], "succeed.py: ccc.out\n")
+test.must_match(['work2', 'ddd.out'], "succeed.py: ddd.out\n")
+
+
 
 test.pass_test()
-