Refactor ErrorDuringExecution.

Library/Homebrew/cask/lib/hbc/exceptions.rb

@@ -53,31 +53,6 @@ module Hbc
     end
   end
 
-  class CaskCommandFailedError < CaskError
-    def initialize(cmd, stdout, stderr, status)
-      @cmd = cmd
-      @stdout = stdout
-      @stderr = stderr
-      @status = status
-    end
-
-    def to_s
-      s = "Command failed to execute!\n"
-      s.concat("\n")
-      s.concat("==> Failed command:\n")
-      s.concat(@cmd.join(" ")).concat("\n")
-      s.concat("\n")
-      s.concat("==> Standard Output of failed command:\n")
-      s.concat(@stdout).concat("\n")
-      s.concat("\n")
-      s.concat("==> Standard Error of failed command:\n")
-      s.concat(@stderr).concat("\n")
-      s.concat("\n")
-      s.concat("==> Exit status of failed command:\n")
-      s.concat(@status.inspect).concat("\n")
-    end
-  end
-
   class CaskX11DependencyError < AbstractCaskErrorWithToken
     def to_s
       <<~EOS

Library/Homebrew/cask/lib/hbc/system_command.rb

@@ -84,7 +84,10 @@ module Hbc
 
     def assert_success
       return if processed_status&.success?
-      raise CaskCommandFailedError.new(command, processed_output[:stdout], processed_output[:stderr], processed_status)
+      raise ErrorDuringExecution.new(command,
+                                     stdout: processed_output[:stdout],
+                                     stderr: processed_output[:stderr],
+                                     status: processed_status)
     end
 
     def expanded_args

Library/Homebrew/download_strategy.rb

@@ -69,7 +69,7 @@ class AbstractDownloadStrategy
 
   def safe_system(*args)
     if @shutup
-      quiet_system(*args) || raise(ErrorDuringExecution.new(args.shift, args))
+      quiet_system(*args) || raise(ErrorDuringExecution.new(args, status: $CHILD_STATUS))
     else
       super(*args)
     end

Library/Homebrew/exceptions.rb

@@ -1,3 +1,5 @@
+require "shellwords"
+
 class UsageError < RuntimeError
   attr_reader :reason
 
@@ -524,9 +526,22 @@ end
 
 # raised by safe_system in utils.rb
 class ErrorDuringExecution < RuntimeError
-  def initialize(cmd, args = [])
+  def initialize(cmd, status:, stdout: nil, stderr: nil)
-    args = args.map { |a| a.to_s.gsub " ", "\\ " }.join(" ")
+    s = "Failure while executing; `#{cmd.shelljoin.gsub(/\\=/, "=")}` exited with #{status.exitstatus}."
-    super "Failure while executing: #{cmd} #{args}"
+
+    if stdout
+      s << "==> Standard Output of failed command:\n"
+      s << stdout
+      s << "\n" unless stdout.end_with?("\n")
+    end
+
+    if stderr
+      s << "==> Standard Error of failed command:\n"
+      s << stderr
+      s << "\n" unless stderr.end_with?("\n")
+    end
+
+    super s
   end
 end
 

Library/Homebrew/extend/os/linux/keg_relocate.rb

@@ -20,7 +20,9 @@ class Keg
     # patchelf requires that the ELF file have a .dynstr section.
     # Skip ELF files that do not have a .dynstr section.
     return if ["cannot find section .dynstr", "strange: no string table"].include?(old_rpath)
-    raise ErrorDuringExecution, "#{cmd_rpath}\n#{old_rpath}" unless $CHILD_STATUS.success?
+    unless $CHILD_STATUS.success?
+      raise ErrorDuringExecution.new(cmd_rpath, stdout: old_rpath, status: $CHILD_STATUS)
+    end
 
     rpath = old_rpath
             .split(":")

Library/Homebrew/os/linux/elf.rb

@@ -118,12 +118,10 @@ module ELFShim
       patchelf = DevelopmentTools.locate "patchelf"
       if path.dylib?
         command = [patchelf, "--print-soname", path.expand_path.to_s]
-        soname = Utils.popen_read(*command).chomp
+        soname = Utils.safe_popen_read(*command).chomp
-        raise ErrorDuringExecution, command unless $CHILD_STATUS.success?
       end
       command = [patchelf, "--print-needed", path.expand_path.to_s]
-      needed = Utils.popen_read(*command).split("\n")
+      needed = Utils.safe_popen_read(*command).split("\n")
-      raise ErrorDuringExecution, command unless $CHILD_STATUS.success?
       [soname, needed]
     end
 
@@ -131,8 +129,7 @@ module ELFShim
       soname = nil
       needed = []
       command = ["readelf", "-d", path.expand_path.to_s]
-      lines = Utils.popen_read(*command).split("\n")
+      lines = Utils.safe_popen_read(*command).split("\n")
-      raise ErrorDuringExecution, command unless $CHILD_STATUS.success?
       lines.each do |s|
         filename = s[/\[(.*)\]/, 1]
         next if filename.nil?

Library/Homebrew/patch.rb

@@ -67,8 +67,7 @@ class EmbeddedPatch
   def apply
     data = contents.gsub("HOMEBREW_PREFIX", HOMEBREW_PREFIX)
     args = %W[-g 0 -f -#{strip}]
-    Utils.popen_write("patch", *args) { |p| p.write(data) }
+    Utils.safe_popen_write("patch", *args) { |p| p.write(data) }
-    raise ErrorDuringExecution.new("patch", args) unless $CHILD_STATUS.success?
   end
 
   def inspect

Library/Homebrew/test/cask/system_command_spec.rb

@@ -69,7 +69,7 @@ describe Hbc::SystemCommand, :cask do
       it "throws an error" do
         expect {
           described_class.run!(command)
-        }.to raise_error(Hbc::CaskCommandFailedError)
+        }.to raise_error(ErrorDuringExecution)
       end
     end
 

Library/Homebrew/test/exceptions_spec.rb

@@ -183,9 +183,10 @@ describe CurlDownloadStrategyError do
 end
 
 describe ErrorDuringExecution do
-  subject { described_class.new("badprg", %w[arg1 arg2]) }
+  subject { described_class.new(["badprg", "arg1", "arg2"], status: status) }
+  let(:status) { instance_double(Process::Status, exitstatus: 17) }
 
-  its(:to_s) { is_expected.to eq("Failure while executing: badprg arg1 arg2") }
+  its(:to_s) { is_expected.to eq("Failure while executing; `badprg arg1 arg2` exited with 17.") }
 end
 
 describe ChecksumMismatchError do

Library/Homebrew/utils.rb

@@ -296,7 +296,7 @@ end
 
 # Kernel.system but with exceptions
 def safe_system(cmd, *args, **options)
-  Homebrew.system(cmd, *args, **options) || raise(ErrorDuringExecution.new(cmd, args))
+  Homebrew.system(cmd, *args, **options) || raise(ErrorDuringExecution.new([cmd, *args], status: $CHILD_STATUS))
 end
 
 # prints no output

Library/Homebrew/utils/popen.rb

@@ -5,7 +5,7 @@ module Utils
 
   def self.safe_popen_read(*args, **options, &block)
     output = popen_read(*args, **options, &block)
-    raise ErrorDuringExecution, args unless $CHILD_STATUS.success?
+    raise ErrorDuringExecution(args, stdout: output, status: $CHILD_STATUS) unless $CHILD_STATUS.success?
     output
   end
 
@@ -14,8 +14,8 @@ module Utils
   end
 
   def self.safe_popen_write(*args, **options, &block)
-    output = popen_write(args, **options, &block)
+    output = popen_write(*args, **options, &block)
-    raise ErrorDuringExecution, args unless $CHILD_STATUS.success?
+    raise ErrorDuringExecution(args, stdout: output, status: $CHILD_STATUS) unless $CHILD_STATUS.success?
     output
   end