diff --git a/Library/Homebrew/dev-cmd/ruby.rb b/Library/Homebrew/dev-cmd/ruby.rb index 47475f39e3..d696e997a5 100644 --- a/Library/Homebrew/dev-cmd/ruby.rb +++ b/Library/Homebrew/dev-cmd/ruby.rb @@ -32,14 +32,10 @@ module Homebrew ruby_sys_args << "-e #{args.e}" if args.e ruby_sys_args += args.named - begin - safe_system RUBY_PATH, - ENV["HOMEBREW_RUBY_WARNINGS"], - "-I", $LOAD_PATH.join(File::PATH_SEPARATOR), - "-rglobal", "-rdev-cmd/irb", - *ruby_sys_args - rescue ErrorDuringExecution => e - exit e.status.exitstatus - end + exec RUBY_PATH, + ENV["HOMEBREW_RUBY_WARNINGS"], + "-I", $LOAD_PATH.join(File::PATH_SEPARATOR), + "-rglobal", "-rdev-cmd/irb", + *ruby_sys_args end end diff --git a/Library/Homebrew/exceptions.rb b/Library/Homebrew/exceptions.rb index b1ffb46b75..2ac40f09fc 100644 --- a/Library/Homebrew/exceptions.rb +++ b/Library/Homebrew/exceptions.rb @@ -578,14 +578,24 @@ class ErrorDuringExecution < RuntimeError @status = status @output = output - exitstatus = if status.respond_to?(:exitstatus) - status.exitstatus - else + exitstatus = case status + when Integer status + else + status.exitstatus end redacted_cmd = redact_secrets(cmd.shelljoin.gsub('\=', "="), secrets) - s = +"Failure while executing; `#{redacted_cmd}` exited with #{exitstatus}." + + reason = if exitstatus + "exited with #{exitstatus}" + elsif (uncaught_signal = status.termsig) + "was terminated by uncaught signal #{Signal.signame(uncaught_signal)}" + else + raise ArgumentError, "Status does neither have `exitstatus` nor `termsig`." + end + + s = +"Failure while executing; `#{redacted_cmd}` #{reason}." if Array(output).present? format_output_line = lambda do |type_line| diff --git a/Library/Homebrew/system_command.rb b/Library/Homebrew/system_command.rb index 289d907e98..8436376fd9 100644 --- a/Library/Homebrew/system_command.rb +++ b/Library/Homebrew/system_command.rb @@ -20,6 +20,8 @@ class SystemCommand # Helper functions for calling {SystemCommand.run}. module Mixin + extend T::Sig + def system_command(*args) T.unsafe(SystemCommand).run(*args) end @@ -32,8 +34,6 @@ class SystemCommand include Context extend Predicable - attr_reader :pid - def self.run(executable, **options) T.unsafe(self).new(executable, **options).run! end @@ -44,7 +44,7 @@ class SystemCommand sig { returns(SystemCommand::Result) } def run! - puts redact_secrets(command.shelljoin.gsub('\=', "="), @secrets) if verbose? || debug? + $stderr.puts redact_secrets(command.shelljoin.gsub('\=', "="), @secrets) if verbose? || debug? @output = [] @@ -152,28 +152,42 @@ class SystemCommand end end - def each_output_line(&b) + sig { params(block: T.proc.params(type: Symbol, line: String).void).void } + def each_output_line(&block) executable, *args = command + options = { + # Create a new process group so that we can send `SIGINT` from + # parent to child rather than the child receiving `SIGINT` directly. + pgroup: true, + } + options[:chdir] = chdir if chdir - raw_stdin, raw_stdout, raw_stderr, raw_wait_thr = - T.unsafe(Open3).popen3(env, [executable, executable], *args, **{ chdir: chdir }.compact) - @pid = raw_wait_thr.pid + pid = T.let(nil, T.nilable(Integer)) + raw_stdin, raw_stdout, raw_stderr, raw_wait_thr = ignore_interrupts do + T.unsafe(Open3).popen3(env, [executable, executable], *args, **options) + .tap { |*, wait_thr| pid = wait_thr.pid } + end write_input_to(raw_stdin) raw_stdin.close_write - each_line_from [raw_stdout, raw_stderr], &b + each_line_from [raw_stdout, raw_stderr], &block @status = raw_wait_thr.value + rescue Interrupt + Process.kill("INT", pid) if pid + raise Interrupt rescue SystemCallError => e @status = $CHILD_STATUS @output << [:stderr, e.message] end + sig { params(raw_stdin: IO).void } def write_input_to(raw_stdin) input.each(&raw_stdin.method(:write)) end - def each_line_from(sources) + sig { params(sources: T::Array[IO], _block: T.proc.params(type: Symbol, line: String).void).void } + def each_line_from(sources, &_block) loop do readable_sources, = IO.select(sources) diff --git a/Library/Homebrew/test/system_command_spec.rb b/Library/Homebrew/test/system_command_spec.rb index fef1692deb..9124c58d4e 100644 --- a/Library/Homebrew/test/system_command_spec.rb +++ b/Library/Homebrew/test/system_command_spec.rb @@ -24,7 +24,11 @@ describe SystemCommand do it "includes the given variables explicitly" do expect(Open3) .to receive(:popen3) - .with(an_instance_of(Hash), ["/usr/bin/env", "/usr/bin/env"], "A=1", "B=2", "C=3", "env", *env_args, {}) + .with( + an_instance_of(Hash), ["/usr/bin/env", "/usr/bin/env"], "A=1", "B=2", "C=3", + "env", *env_args, + pgroup: true + ) .and_call_original command.run! @@ -49,8 +53,10 @@ describe SystemCommand do it "includes the given variables explicitly" do expect(Open3) .to receive(:popen3) - .with(an_instance_of(Hash), ["/usr/bin/sudo", "/usr/bin/sudo"], "-E", "--", - "/usr/bin/env", "A=1", "B=2", "C=3", "env", *env_args, {}) + .with( + an_instance_of(Hash), ["/usr/bin/sudo", "/usr/bin/sudo"], "-E", "--", + "/usr/bin/env", "A=1", "B=2", "C=3", "env", *env_args, pgroup: true + ) .and_wrap_original do |original_popen3, *_, &block| original_popen3.call("true", &block) end @@ -257,24 +263,22 @@ describe SystemCommand do context "when given arguments with secrets" do it "does not leak the secrets" do redacted_msg = /#{Regexp.escape("username:******")}/ - expect do + expect { described_class.run! "curl", args: %w[--user username:hunter2], verbose: true, secrets: %w[hunter2] - end.to raise_error.with_message(redacted_msg).and output(redacted_msg).to_stdout + }.to raise_error.with_message(redacted_msg).and output(redacted_msg).to_stderr end it "does not leak the secrets set by environment" do redacted_msg = /#{Regexp.escape("username:******")}/ - expect do + expect { ENV["PASSWORD"] = "hunter2" described_class.run! "curl", args: %w[--user username:hunter2], verbose: true - ensure - ENV.delete "PASSWORD" - end.to raise_error.with_message(redacted_msg).and output(redacted_msg).to_stdout + }.to raise_error.with_message(redacted_msg).and output(redacted_msg).to_stderr end end end diff --git a/Library/Homebrew/utils.rb b/Library/Homebrew/utils.rb index 6009e092b2..59a5581c9a 100644 --- a/Library/Homebrew/utils.rb +++ b/Library/Homebrew/utils.rb @@ -388,13 +388,30 @@ module Kernel Pathname.new(cmd).archs end - def ignore_interrupts(opt = nil) - std_trap = trap("INT") do - puts "One sec, just cleaning up" unless opt == :quietly + def ignore_interrupts(_opt = nil) + # rubocop:disable Style/GlobalVars + $ignore_interrupts_nesting_level = 0 unless defined?($ignore_interrupts_nesting_level) + $ignore_interrupts_nesting_level += 1 + + $ignore_interrupts_interrupted = false unless defined?($ignore_interrupts_interrupted) + old_sigint_handler = trap(:INT) do + $ignore_interrupts_interrupted = true + $stderr.print "\n" + $stderr.puts "One sec, cleaning up..." end - yield - ensure - trap("INT", std_trap) + + begin + yield + ensure + trap(:INT, old_sigint_handler) + + $ignore_interrupts_nesting_level -= 1 + if $ignore_interrupts_nesting_level == 0 && $ignore_interrupts_interrupted + $ignore_interrupts_interrupted = false + raise Interrupt + end + end + # rubocop:enable Style/GlobalVars end sig { returns(String) }