If a command fails, wait for all running commands to terminate before we do

Previously, if a command fails, the fate of the other child processes
running in parallel was inadequately controlled.  On POSIX platforms,
the processes were orphaned.  Normally they would run to completion,
but were liable to being killed by a SIGPIPE.  On Windows, the child
processes would terminate with the parent.  The cleanup-on-interrupt
patch caused the SubprocessSet and Builder destructors to clean
up after themselves by killing any running child processes and
deleting their output files, making the behaviour more predictable
and consistent across platforms.

If the build is interrupted by the user, this is correct behaviour.
But in the case where the build is stopped by a failed command, this
would be inconsistent with user expectations.  In the latter case,
we now let any remaining child processes run to completion before
leaving the main loop in Builder::Build.

1 files changed, 13 insertions, 14 deletions

diff --git a/src/build.cc b/src/build.cc
index 4c15de9..265c805 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -538,7 +538,7 @@ bool Builder::Build(string* err) {
 
   status_->PlanHasTotalEdges(plan_.command_edge_count());
   int pending_commands = 0;
-  int failures_allowed = config_.swallow_failures;
+  int failures_allowed = config_.failures_allowed;
 
   // This main loop runs the entire build process.
   // It is structured like this:
@@ -549,7 +549,7 @@ bool Builder::Build(string* err) {
   // an error.
   while (plan_.more_to_do()) {
     // See if we can start any more commands.
-    if (command_runner_->CanRunMore()) {
+    if (failures_allowed && command_runner_->CanRunMore()) {
       if (Edge* edge = plan_.FindWork()) {
         if (!StartEdge(edge, err)) {
           status_->BuildFinished();
@@ -576,13 +576,8 @@ bool Builder::Build(string* err) {
         --pending_commands;
         FinishEdge(edge, success, output);
         if (!success) {
-          if (failures_allowed-- == 0) {
-            if (config_.swallow_failures != 0)
-              *err = "subcommands failed";
-            else
-              *err = "subcommand failed";
-            return false;
-          }
+          if (failures_allowed)
+            failures_allowed--;
         }
 
         // We made some progress; start the main loop over.
@@ -605,13 +600,17 @@ bool Builder::Build(string* err) {
 
     // If we get here, we cannot make any more progress.
     status_->BuildFinished();
-    if (failures_allowed < config_.swallow_failures) {
+    if (failures_allowed == 0) {
+      if (config_.failures_allowed > 1)
+        *err = "subcommands failed";
+      else
+        *err = "subcommand failed";
+    } else if (failures_allowed < config_.failures_allowed)
       *err = "cannot make progress due to previous errors";
-      return false;
-    } else {
+    else 
       *err = "stuck [this is a bug]";
-      return false;
-    }
+
+    return false;
   }
 
   status_->BuildFinished();