Merge pull request #16098 from reitermarkus/odebug-system-command

Improve `odebug`/`SystemCommand` debugging output.

Library/Homebrew/context.rb

@@ -33,27 +33,33 @@ module Context
       @verbose = verbose
     end
 
+    sig { returns(T::Boolean) }
     def debug?
       @debug == true
     end
 
+    sig { returns(T::Boolean) }
     def quiet?
       @quiet == true
     end
 
+    sig { returns(T::Boolean) }
     def verbose?
       @verbose == true
     end
   end
 
+  sig { returns(T::Boolean) }
   def debug?
     Context.current.debug?
   end
 
+  sig { returns(T::Boolean) }
   def quiet?
     Context.current.quiet?
   end
 
+  sig { returns(T::Boolean) }
   def verbose?
     Context.current.verbose?
   end
@@ -69,8 +75,10 @@ module Context
 
     Thread.current[:context] = new_context
 
-    yield
-  ensure
-    Thread.current[:context] = old_context
+    begin
+      yield
+    ensure
+      Thread.current[:context] = old_context
+    end
   end
 end

Library/Homebrew/extend/kernel.rb

@@ -39,8 +39,8 @@ module Kernel
 
     return if !debug && !always_display
 
-    puts Formatter.headline(title, color: :magenta)
-    puts sput unless sput.empty?
+    $stderr.puts Formatter.headline(title, color: :magenta)
+    $stderr.puts sput unless sput.empty?
   end
 
   def oh1_title(title, truncate: :auto)

Library/Homebrew/system_command.rb

@@ -47,10 +47,20 @@ class SystemCommand
     each_output_line do |type, line|
       case type
       when :stdout
-        $stdout << redact_secrets(line, @secrets) if print_stdout?
+        case @print_stdout
+        when true
+          $stdout << redact_secrets(line, @secrets)
+        when :debug
+          $stderr << redact_secrets(line, @secrets) if debug?
+        end
         @output << [:stdout, line]
       when :stderr
-        $stderr << redact_secrets(line, @secrets) if print_stderr?
+        case @print_stderr
+        when true
+          $stderr << redact_secrets(line, @secrets)
+        when :debug
+          $stderr << redact_secrets(line, @secrets) if debug?
+        end
         @output << [:stderr, line]
       end
     end
@@ -69,8 +79,8 @@ class SystemCommand
       env:          T::Hash[String, String],
       input:        T.any(String, T::Array[String]),
       must_succeed: T::Boolean,
-      print_stdout: T::Boolean,
-      print_stderr: T::Boolean,
+      print_stdout: T.any(T::Boolean, Symbol),
+      print_stderr: T.any(T::Boolean, Symbol),
       debug:        T.nilable(T::Boolean),
       verbose:      T.nilable(T::Boolean),
       secrets:      T.any(String, T::Array[String]),
@@ -98,7 +108,14 @@ class SystemCommand
     @executable = executable
     @args = args
 
-    raise ArgumentError, "sudo_as_root cannot be set if sudo is false" if !sudo && sudo_as_root
+    raise ArgumentError, "`sudo_as_root` cannot be set if sudo is false" if !sudo && sudo_as_root
+
+    if print_stdout.is_a?(Symbol) && print_stdout != :debug
+      raise ArgumentError, "`print_stdout` is not a valid symbol"
+    end
+    if print_stderr.is_a?(Symbol) && print_stderr != :debug
+      raise ArgumentError, "`print_stderr` is not a valid symbol"
+    end
 
     @sudo = sudo
     @sudo_as_root = sudo_as_root
@@ -128,7 +145,7 @@ class SystemCommand
 
   attr_reader :executable, :args, :input, :chdir, :env
 
-  attr_predicate :sudo?, :sudo_as_root?, :print_stdout?, :print_stderr?, :must_succeed?
+  attr_predicate :sudo?, :sudo_as_root?, :must_succeed?
 
   sig { returns(T::Boolean) }
   def debug?
@@ -227,9 +244,13 @@ class SystemCommand
     write_input_to(raw_stdin)
     raw_stdin.close_write
 
+    thread_context = Context.current
     thread_ready_queue = Queue.new
     thread_done_queue = Queue.new
     line_thread = Thread.new do
+      # Ensure the new thread inherits the current context.
+      Context.current = thread_context
+
       Thread.handle_interrupt(ProcessTerminatedInterrupt => :never) do
         thread_ready_queue << true
         each_line_from [raw_stdout, raw_stderr], &block

Library/Homebrew/test/spec_helper.rb

@@ -109,6 +109,8 @@ RSpec.configure do |config|
 
   config.include(FileUtils)
 
+  config.include(Context)
+
   config.include(RuboCop::RSpec::ExpectOffense)
 
   config.include(Test::Helper::Cask)
@@ -236,6 +238,7 @@ RSpec.configure do |config|
       example.example.set_exception(e)
     ensure
       ENV.replace(@__env)
+      Context.current = Context::ContextStruct.new
 
       $stdout.reopen(@__stdout)
       $stderr.reopen(@__stderr)

Library/Homebrew/test/system_command_spec.rb

@@ -166,7 +166,7 @@ describe SystemCommand do
       include_examples("it returns '1 2 3 4 5 6'")
     end
 
-    context "with print_stdout" do
+    context "with `print_stdout: true`" do
       before do
         options.merge!(print_stdout: true)
       end
@@ -180,7 +180,38 @@ describe SystemCommand do
       include_examples("it returns '1 2 3 4 5 6'")
     end
 
-    context "without print_stderr" do
+    context "with `print_stdout: :debug`" do
+      before do
+        options.merge!(print_stdout: :debug)
+      end
+
+      it "echoes only STDERR output" do
+        expect { described_class.run(command, **options) }
+          .to output("2\n4\n6\n").to_stderr
+          .and not_to_output.to_stdout
+      end
+
+      context "when `debug?` is true" do
+        let(:options) do
+          { args: [
+            "-c",
+            "for i in $(seq 1 2 5); do echo $i; sleep 0.1; echo $(($i + 1)) >&2; sleep 0.1; done",
+          ] }
+        end
+
+        it "echoes the command and all output to STDERR when `debug?` is true" do
+          with_context debug: true do
+            expect { described_class.run(command, **options) }
+              .to output(/\A.*#{Regexp.escape(command)}.*\n1\n2\n3\n4\n5\n6\n\Z/).to_stderr
+              .and not_to_output.to_stdout
+          end
+        end
+      end
+
+      include_examples("it returns '1 2 3 4 5 6'")
+    end
+
+    context "with `print_stderr: false`" do
       before do
         options.merge!(print_stderr: false)
       end
@@ -188,13 +219,13 @@ describe SystemCommand do
       it "echoes nothing" do
         expect do
           described_class.run(command, **options)
-        end.to output("").to_stdout
+        end.not_to output.to_stdout
       end
 
       include_examples("it returns '1 2 3 4 5 6'")
     end
 
-    context "with print_stdout but without print_stderr" do
+    context "with `print_stdout: true` and `print_stderr: false`" do
       before do
         options.merge!(print_stdout: true, print_stderr: false)
       end