From bf136e79f2eced9bc000b30e597bdab7fb414d4e Mon Sep 17 00:00:00 2001
From: Josh Holtrop <jholtrop@gmail.com>
Date: Tue, 26 Feb 2019 06:52:43 -0500
Subject: [PATCH] rework showing failed command - close #89

---
 lib/rscons/application.rb |  4 +++-
 lib/rscons/cli.rb         | 14 +++++++++++++
 lib/rscons/environment.rb | 42 +++++++++++----------------------------
 spec/build_tests_spec.rb  | 19 ++++++++++++++++++
 4 files changed, 48 insertions(+), 31 deletions(-)

diff --git a/lib/rscons/application.rb b/lib/rscons/application.rb
index 905e22a..7bef69c 100644
--- a/lib/rscons/application.rb
+++ b/lib/rscons/application.rb
@@ -80,13 +80,14 @@ module Rscons
     #   Exit code.
     def build(options)
       begin
+        Cache.instance["failed_commands"] = []
         @script.build
         Environment.environments.each do |env|
           env.process
         end
         0
       rescue BuildError => be
-        $stderr.puts be
+        Ansi.write($stderr, :red, be.message, :reset, "\n")
         1
       end
     end
@@ -137,6 +138,7 @@ module Rscons
       options[:build_dir] ||= "build"
       options[:prefix] ||= "/usr/local"
       cache = Cache.instance
+      cache["failed_commands"] = []
       cache["configuration_data"] = {}
       if project_name = @script.project_name
         Ansi.write($stdout, "Configuring ", :cyan, project_name, :reset, "...\n")
diff --git a/lib/rscons/cli.rb b/lib/rscons/cli.rb
index 2f7ab43..2bf037a 100644
--- a/lib/rscons/cli.rb
+++ b/lib/rscons/cli.rb
@@ -5,6 +5,7 @@ USAGE = <<EOF
 Usage: #{$0} [global options] [operation] [operation options]
 
 Global options:
+  -F, --show-failure          Show previous failed command(s) and exit
   --version                   Show rscons version and exit
   -h, --help                  Show rscons help and exit
   -r COLOR, --color=COLOR     Set color mode (off, auto, force)
@@ -86,6 +87,11 @@ module Rscons
             rsconscript = f
           end
 
+          opts.on("-F", "--show-failure") do
+            show_failure
+            exit 0
+          end
+
           opts.on("--version") do
             puts "Rscons version #{Rscons::VERSION}"
             exit 0
@@ -157,6 +163,14 @@ module Rscons
         end
       end
 
+      def show_failure
+        failed_commands = Cache.instance["failed_commands"]
+        failed_commands.each_with_index do |command, i|
+          Ansi.write($stdout, :red, "Failed command (#{i + 1}/#{failed_commands.size}):", :reset, "\n")
+          $stdout.puts Util.command_to_s(command)
+        end
+      end
+
     end
   end
 end
diff --git a/lib/rscons/environment.rb b/lib/rscons/environment.rb
index 9e52670..6328a85 100644
--- a/lib/rscons/environment.rb
+++ b/lib/rscons/environment.rb
@@ -261,7 +261,7 @@ module Rscons
       unless Cache.instance["configuration_data"]["configured"]
         raise "Project must be configured before processing an Environment"
       end
-      @process_failure = nil
+      @process_failures = []
       @process_blocking_wait = false
       @process_commands_waiting_to_run = []
       @process_builder_waits = {}
@@ -269,7 +269,7 @@ module Rscons
       begin
         while @builder_set.size > 0 or @threads.size > 0 or @process_commands_waiting_to_run.size > 0
           process_step
-          if @process_failure
+          unless @process_failures.empty?
             # On a build failure, do not start any more builders or commands,
             # but let the threads that have already been started complete.
             @builder_set.clear
@@ -279,8 +279,12 @@ module Rscons
       ensure
         Cache.instance.write
       end
-      if @process_failure
-        raise BuildError.new(@process_failure)
+      unless @process_failures.empty?
+        msg = @process_failures.join("\n")
+        if Cache.instance["failed_commands"].size > 0
+          msg += "\nRun `#{$0} -F` to see the failed command(s)."
+        end
+        raise BuildError.new(msg)
       end
     end
 
@@ -537,29 +541,8 @@ module Rscons
       Ansi.write($stdout, :cyan, message, :reset, "\n") if message
     end
 
-    # Print a failed command.
-    #
-    # @param command [Array<String>]
-    #   Builder command.
-    #
-    # @return [void]
-    def print_failed_command(command)
-      Ansi.write($stdout, :red, "Failed command was: #{Util.command_to_s(command)}", :reset, "\n")
-    end
-
     private
 
-    # Signal a build failure to the {#process} method.
-    #
-    # @param target [String]
-    #   Build target name.
-    #
-    # @return [void]
-    def process_failure(target)
-      @process_failure = "Failed to build #{target}"
-      Ansi.write($stderr, :red, @process_failure, :reset, "\n")
-    end
-
     # Run a builder and process its return value.
     #
     # @param builder [Builder]
@@ -587,7 +570,7 @@ module Rscons
           end
         end
       when false
-        process_failure(builder.target)
+        @process_failures << "Failed to build #{builder.target}."
       when true
         # Register side-effect files as build targets so that a Cache
         # clean operation will remove them.
@@ -644,10 +627,9 @@ module Rscons
           process_remove_wait(completed_command)
           completed_command.status = thread.value
           unless completed_command.status
-            unless @echo == :command
-              print_failed_command(completed_command.command)
-            end
-            return process_failure(builder.target)
+            Cache.instance["failed_commands"] << completed_command.command
+            @process_failures << "Failed to build #{builder.target}."
+            return
           end
         end
       end
diff --git a/spec/build_tests_spec.rb b/spec/build_tests_spec.rb
index f9e3e82..7e58287 100644
--- a/spec/build_tests_spec.rb
+++ b/spec/build_tests_spec.rb
@@ -853,6 +853,25 @@ EOF
     expect(lines(result.stdout)).to include *["MyBuilder foo command"]
   end
 
+  it "stores the failed command for later display with -F command line option" do
+    test_dir("simple")
+
+    File.open("simple.c", "wb") do |fh|
+      fh.write("foo")
+    end
+
+    result = run_rscons
+    expect(result.stderr).to match /Failed to build/
+    expect(result.stderr).to match /^Run.*-F.*to see the failed command/
+    expect(result.status).to_not eq 0
+
+    result = run_rscons(rscons_args: %w[-F])
+    expect(result.stderr).to eq ""
+    expect(result.stdout).to match %r{Failed command \(1/1\):}
+    expect(result.stdout).to match %r{^gcc -}
+    expect(result.status).to eq 0
+  end
+
   context "colored output" do
     it "does not output in color with --color=off" do
       test_dir("simple")