Merge pull request #4536 from reitermarkus/container-system-command

Add `system_command` helpers.

Library/Homebrew/cask/lib/hbc/artifact/nested_container.rb

@@ -20,7 +20,7 @@ module Hbc
         path.relative_path_from(cask.staged_path).to_s
       end
 
-      def extract(command: nil, verbose: nil, **_)
+      def extract(verbose: nil, **_)
         container = Container.for_path(path)
 
         unless container
@@ -28,7 +28,7 @@ module Hbc
         end
 
         ohai "Extracting nested container #{path.relative_path_from(cask.staged_path)}"
-        container.new(cask, path, command).extract(to: cask.staged_path, verbose: verbose)
+        container.new(cask, path).extract(to: cask.staged_path, verbose: verbose)
         FileUtils.remove_entry_secure(path)
       end
     end

Library/Homebrew/cask/lib/hbc/container/air.rb

@@ -8,7 +8,7 @@ module Hbc
       end
 
       def extract_to_dir(unpack_dir, basename:, verbose:)
-        @command.run!(
+        system_command!(
           "/Applications/Utilities/Adobe AIR Application Installer.app/Contents/MacOS/Adobe AIR Application Installer",
           args: ["-silent", "-location", unpack_dir, path],
         )

Library/Homebrew/cask/lib/hbc/container/base.rb

@@ -5,10 +5,9 @@ module Hbc
 
       attr_reader :path
 
-      def initialize(cask, path, command, nested: false)
+      def initialize(cask, path, nested: false)
         @cask = cask
         @path = path
-        @command = command
       end
 
       def extract(to: nil, basename: nil, verbose: false)
@@ -42,7 +41,7 @@ module Hbc
         return false unless container
 
         ohai "Extracting nested container #{source.basename}"
-        container.new(@cask, source, @command).extract(to: to, verbose: verbose)
+        container.new(@cask, source).extract(to: to, verbose: verbose)
 
         true
       end

Library/Homebrew/cask/lib/hbc/container/bzip2.rb

@@ -12,7 +12,7 @@ module Hbc
           tmp_unpack_dir = Pathname(tmp_unpack_dir)
 
           FileUtils.cp path, tmp_unpack_dir/basename, preserve: true
-          @command.run!("bunzip2", args: ["--quiet", "--", tmp_unpack_dir/basename])
+          system_command!("bunzip2", args: ["--quiet", "--", tmp_unpack_dir/basename])
 
           extract_nested_inside(tmp_unpack_dir, to: unpack_dir)
         end

Library/Homebrew/cask/lib/hbc/container/cab.rb

@@ -8,9 +8,9 @@ module Hbc
       end
 
       def extract_to_dir(unpack_dir, basename:, verbose:)
-        @command.run!("cabextract",
+        system_command!("cabextract",
-                      args: ["-d", unpack_dir, "--", path],
+                        args: ["-d", unpack_dir, "--", path],
-                      env: { "PATH" => PATH.new(Formula["cabextract"].opt_bin, ENV["PATH"]) })
+                        env: { "PATH" => PATH.new(Formula["cabextract"].opt_bin, ENV["PATH"]) })
       end
 
       def dependencies

Library/Homebrew/cask/lib/hbc/container/dmg.rb

@@ -6,18 +6,18 @@ module Hbc
   class Container
     class Dmg < Base
       def self.can_extract?(path:, magic_number:)
-        imageinfo = SystemCommand.run("/usr/bin/hdiutil",
+        imageinfo = system_command("/usr/bin/hdiutil",
-                                      # realpath is a failsafe against unusual filenames
+                                   # realpath is a failsafe against unusual filenames
-                                      args:         ["imageinfo", path.realpath],
+                                   args:         ["imageinfo", path.realpath],
-                                      print_stderr: false).stdout
+                                   print_stderr: false).stdout
 
         !imageinfo.empty?
       end
 
       def extract_to_dir(unpack_dir, basename:, verbose:)
-        mount do |mounts|
+        mount(verbose: verbose) do |mounts|
           begin
-            raise CaskError, "No mounts found in '#{@path}'; perhaps it is a bad disk image?" if mounts.empty?
+            raise "No mounts found in '#{path}'; perhaps it is a bad disk image?" if mounts.empty?
             mounts.each do |mount|
               extract_mount(mount, to: unpack_dir)
             end
@@ -27,28 +27,29 @@ module Hbc
         end
       end
 
-      def mount
+      def mount(verbose: false)
         # realpath is a failsafe against unusual filenames
-        path = Pathname.new(@path).realpath
+        realpath = path.realpath
+        path = realpath
 
         Dir.mktmpdir do |unpack_dir|
-          cdr_path = Pathname.new(unpack_dir).join("#{path.basename(".dmg")}.cdr")
+          without_eula = system_command("/usr/bin/hdiutil",
-
+                                        args:  ["attach", "-plist", "-nobrowse", "-readonly", "-noidme", "-mountrandom", unpack_dir, path],
-          without_eula = @command.run("/usr/bin/hdiutil",
+                                        input: "qn\n",
-                                 args:  ["attach", "-plist", "-nobrowse", "-readonly", "-noidme", "-mountrandom", unpack_dir, path],
+                                        print_stderr: false)
-                                 input: "qn\n",
-                                 print_stderr: false)
 
           # If mounting without agreeing to EULA succeeded, there is none.
           plist = if without_eula.success?
             without_eula.plist
           else
-            @command.run!("/usr/bin/hdiutil", args: ["convert", "-quiet", "-format", "UDTO", "-o", cdr_path, path])
+            cdr_path = Pathname.new(unpack_dir).join("#{path.basename(".dmg")}.cdr")
 
-            with_eula = @command.run!("/usr/bin/hdiutil",
+            system_command!("/usr/bin/hdiutil", args: ["convert", "-quiet", "-format", "UDTO", "-o", cdr_path, path])
+
+            with_eula = system_command!("/usr/bin/hdiutil",
                           args: ["attach", "-plist", "-nobrowse", "-readonly", "-noidme", "-mountrandom", unpack_dir, cdr_path])
 
-            if verbose? && !(eula_text = without_eula.stdout).empty?
+            if verbose && !(eula_text = without_eula.stdout).empty?
               ohai "Software License Agreement for '#{path}':"
               puts eula_text
             end
@@ -63,21 +64,22 @@ module Hbc
       def eject(mount)
         # realpath is a failsafe against unusual filenames
         mountpath = Pathname.new(mount).realpath
-        return unless mountpath.exist?
 
         begin
           tries ||= 3
+
+          return unless mountpath.exist?
+
           if tries > 1
-            @command.run("/usr/sbin/diskutil",
+            system_command!("/usr/sbin/diskutil",
                          args:         ["eject", mountpath],
                          print_stderr: false)
           else
-            @command.run("/usr/sbin/diskutil",
+            system_command!("/usr/sbin/diskutil",
                          args:         ["unmount", "force", mountpath],
                          print_stderr: false)
           end
-          raise CaskError, "Failed to eject #{mountpath}" if mountpath.exist?
+        rescue ErrorDuringExecution => e
-        rescue CaskError => e
           raise e if (tries -= 1).zero?
           sleep 1
           retry
@@ -94,8 +96,8 @@ module Hbc
             filelist.puts(bom_filelist_from_path(mount))
             filelist.close
 
-            @command.run!("/usr/bin/mkbom", args: ["-s", "-i", filelist.path, "--", bomfile.path])
+            system_command!("/usr/bin/mkbom", args: ["-s", "-i", filelist.path, "--", bomfile.path])
-            @command.run!("/usr/bin/ditto", args: ["--bom", bomfile.path, "--", mount, to])
+            system_command!("/usr/bin/ditto", args: ["--bom", bomfile.path, "--", mount, to])
           end
         end
       end
@@ -103,14 +105,15 @@ module Hbc
       def bom_filelist_from_path(mount)
         # We need to use `find` here instead of Ruby in order to properly handle
         # file names containing special characters, such as “e” + “´” vs. “é”.
-        @command.run("/usr/bin/find", args: [".", "-print0"], chdir: mount, print_stderr: false).stdout
+        system_command("/usr/bin/find", args: [".", "-print0"], chdir: mount, print_stderr: false)
-                .split("\0")
+          .stdout
-                .reject { |path| skip_path?(mount, path) }
+          .split("\0")
-                .join("\n")
+          .reject { |path| skip_path?(mount, path) }
+          .join("\n")
       end
 
       def skip_path?(mount, path)
-        path = Pathname(path.sub(%r{^\./}, ""))
+        path = Pathname(path.sub(%r{\A\./}, ""))
         dmg_metadata?(path) || system_dir_symlink?(mount, path)
       end
 

Library/Homebrew/cask/lib/hbc/container/generic_unar.rb

@@ -8,9 +8,9 @@ module Hbc
       end
 
       def extract_to_dir(unpack_dir, basename:, verbose:)
-        @command.run!("unar",
+        system_command!("unar",
-                      args: ["-force-overwrite", "-quiet", "-no-directory", "-output-directory", unpack_dir, "--", path],
+                        args: ["-force-overwrite", "-quiet", "-no-directory", "-output-directory", unpack_dir, "--", path],
-                      env: { "PATH" => PATH.new(Formula["unar"].opt_bin, ENV["PATH"]) })
+                        env: { "PATH" => PATH.new(Formula["unar"].opt_bin, ENV["PATH"]) })
       end
 
       def dependencies

Library/Homebrew/cask/lib/hbc/container/gzip.rb

@@ -12,7 +12,7 @@ module Hbc
           tmp_unpack_dir = Pathname(tmp_unpack_dir)
 
           FileUtils.cp path, tmp_unpack_dir/basename, preserve: true
-          @command.run!("gunzip", args: ["--quiet", "--name", "--", tmp_unpack_dir/basename])
+          system_command!("gunzip", args: ["--quiet", "--name", "--", tmp_unpack_dir/basename])
 
           extract_nested_inside(tmp_unpack_dir, to: unpack_dir)
         end

Library/Homebrew/cask/lib/hbc/container/lzma.rb

@@ -8,10 +8,10 @@ module Hbc
       end
 
       def extract_to_dir(unpack_dir, basename:, verbose:)
-        @command.run!("/usr/bin/ditto", args: ["--", path, unpack_dir])
+        system_command!("/usr/bin/ditto", args: ["--", path, unpack_dir])
-        @command.run!("unlzma",
+        system_command!("unlzma",
-                      args: ["-q", "--", Pathname(unpack_dir).join(basename)],
+                        args: ["-q", "--", Pathname(unpack_dir).join(basename)],
-                      env: { "PATH" => PATH.new(Formula["unlzma"].opt_bin, ENV["PATH"]) })
+                        env: { "PATH" => PATH.new(Formula["unlzma"].opt_bin, ENV["PATH"]) })
       end
 
       def dependencies

Library/Homebrew/cask/lib/hbc/container/naked.rb

@@ -8,7 +8,7 @@ module Hbc
       end
 
       def extract_to_dir(unpack_dir, basename:, verbose:)
-        @command.run!("/usr/bin/ditto", args: ["--", path, unpack_dir/basename])
+        system_command!("/usr/bin/ditto", args: ["--", path, unpack_dir/basename])
       end
     end
   end

Library/Homebrew/cask/lib/hbc/container/rar.rb

@@ -8,9 +8,9 @@ module Hbc
       end
 
       def extract_to_dir(unpack_dir, basename:, verbose:)
-        @command.run!("unrar",
+        system_command!("unrar",
-                      args: ["x", "-inul", path, unpack_dir],
+                        args: ["x", "-inul", path, unpack_dir],
-                      env: { "PATH" => PATH.new(Formula["unrar"].opt_bin, ENV["PATH"]) })
+                        env: { "PATH" => PATH.new(Formula["unrar"].opt_bin, ENV["PATH"]) })
       end
 
       def dependencies

Library/Homebrew/cask/lib/hbc/container/seven_zip.rb

@@ -8,9 +8,9 @@ module Hbc
       end
 
       def extract_to_dir(unpack_dir, basename:, verbose:)
-        @command.run!("7zr",
+        system_command!("7zr",
-                      args: ["x", "-y", "-bd", "-bso0", path, "-o#{unpack_dir}"],
+                        args: ["x", "-y", "-bd", "-bso0", path, "-o#{unpack_dir}"],
-                      env: { "PATH" => PATH.new(Formula["p7zip"].opt_bin, ENV["PATH"]) })
+                        env: { "PATH" => PATH.new(Formula["p7zip"].opt_bin, ENV["PATH"]) })
       end
 
       def dependencies

Library/Homebrew/cask/lib/hbc/container/svn_repository.rb

@@ -6,7 +6,7 @@ module Hbc
       end
 
       def extract_to_dir(unpack_dir, basename:, verbose:)
-        @command.run!("svn", args: ["export", "--force", path, unpack_dir])
+        system_command!("svn", args: ["export", "--force", path, unpack_dir])
       end
     end
   end

Library/Homebrew/cask/lib/hbc/container/tar.rb

@@ -13,7 +13,7 @@ module Hbc
       end
 
       def extract_to_dir(unpack_dir, basename:, verbose:)
-        @command.run!("tar", args: ["xf", path, "-C", unpack_dir])
+        system_command!("tar", args: ["xf", path, "-C", unpack_dir])
       end
     end
   end

Library/Homebrew/cask/lib/hbc/container/xar.rb

@@ -8,7 +8,7 @@ module Hbc
       end
 
       def extract_to_dir(unpack_dir, basename:, verbose:)
-        @command.run!("xar", args: ["-x", "-f", @path, "-C", unpack_dir])
+        system_command!("xar", args: ["-x", "-f", @path, "-C", unpack_dir])
       end
     end
   end

Library/Homebrew/cask/lib/hbc/container/xz.rb

@@ -8,10 +8,10 @@ module Hbc
       end
 
       def extract_to_dir(unpack_dir, basename:, verbose:)
-        @command.run!("/usr/bin/ditto", args: ["--", path, unpack_dir])
+        system_command!("/usr/bin/ditto", args: ["--", path, unpack_dir])
-        @command.run!("unxz",
+        system_command!("unxz",
-                      args: ["-q", "--", unpack_dir/basename],
+                        args: ["-q", "--", unpack_dir/basename],
-                      env: { "PATH" => PATH.new(Formula["xz"].opt_bin, ENV["PATH"]) })
+                        env: { "PATH" => PATH.new(Formula["xz"].opt_bin, ENV["PATH"]) })
       end
 
       def dependencies

Library/Homebrew/cask/lib/hbc/container/zip.rb

@@ -9,7 +9,7 @@ module Hbc
 
       def extract_to_dir(unpack_dir, basename:, verbose:)
         Dir.mktmpdir do |tmp_unpack_dir|
-          @command.run!("/usr/bin/ditto", args: ["-x", "-k", "--", path, tmp_unpack_dir])
+          system_command!("/usr/bin/ditto", args: ["-x", "-k", "--", path, tmp_unpack_dir])
 
           extract_nested_inside(tmp_unpack_dir, to: unpack_dir)
         end

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

@@ -158,7 +158,7 @@ module Hbc
           raise CaskError, "Uh oh, could not figure out how to unpack '#{@downloaded_path}'."
         end
 
-        container.new(@cask, @downloaded_path, @command)
+        container.new(@cask, @downloaded_path)
       end
     end
 

Library/Homebrew/system_command.rb

@@ -6,6 +6,14 @@ require "extend/io"
 require "extend/hash_validator"
 using HashValidator
 
+def system_command(*args)
+  SystemCommand.run(*args)
+end
+
+def system_command!(*args)
+  SystemCommand.run!(*args)
+end
+
 class SystemCommand
   extend Predicable
 

Library/Homebrew/test/cask/container/dmg_spec.rb

@@ -1,14 +1,9 @@
 describe Hbc::Container::Dmg, :cask do
-  describe "#mount!" do
+  describe "#mount" do
+    let(:transmission) { Hbc::CaskLoader.load(cask_path("local-transmission")) }
+    subject(:dmg) { described_class.new(transmission, Pathname(transmission.url.path)) }
+
     it "does not store nil mounts for dmgs with extra data" do
-      transmission = Hbc::CaskLoader.load(cask_path("local-transmission"))
-
-      dmg = Hbc::Container::Dmg.new(
-        transmission,
-        Pathname(transmission.url.path),
-        SystemCommand,
-      )
-
       dmg.mount do |mounts|
         begin
           expect(mounts).not_to include nil

Library/Homebrew/test/cask/container/naked_spec.rb

@@ -8,14 +8,11 @@ describe Hbc::Container::Naked, :cask do
     path                 = Pathname("/tmp/downloads/kevin-spacey.pkg")
     expected_destination = cask.staged_path.join("kevin spacey.pkg")
 
-    container = Hbc::Container::Naked.new(cask, path, FakeSystemCommand)
+    container = Hbc::Container::Naked.new(cask, path)
 
-    FakeSystemCommand.expects_command(
+    expect(container).to receive(:system_command!)
-      ["/usr/bin/ditto", "--", path, expected_destination],
+      .with("/usr/bin/ditto", args: ["--", path, expected_destination])
-    )
 
-    expect {
+    container.extract(to: cask.staged_path, basename: "kevin spacey.pkg")
-      container.extract(to: cask.staged_path, basename: "kevin spacey.pkg")
-    }.not_to raise_error
   end
 end