Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add system_command helpers. #4536

Merged
merged 2 commits into from
Jul 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions Library/Homebrew/cask/lib/hbc/artifact/nested_container.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ def summarize
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
raise CaskError, "Aw dang, could not identify nested container at '#{source}'"
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
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/container/air.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def self.can_extract?(path:, magic_number:)
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],
)
Expand Down
5 changes: 2 additions & 3 deletions Library/Homebrew/cask/lib/hbc/container/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ class Base

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)
Expand Down Expand Up @@ -42,7 +41,7 @@ def extract_nested_container(source, to:, verbose: false)
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
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/container/bzip2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def extract_to_dir(unpack_dir, basename:, verbose:)
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
Expand Down
6 changes: 3 additions & 3 deletions Library/Homebrew/cask/lib/hbc/container/cab.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ def self.can_extract?(path:, magic_number:)
end

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

def dependencies
Expand Down
61 changes: 32 additions & 29 deletions Library/Homebrew/cask/lib/hbc/container/dmg.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@ module Hbc
class Container
class Dmg < Base
def self.can_extract?(path:, magic_number:)
imageinfo = SystemCommand.run("/usr/bin/hdiutil",
# realpath is a failsafe against unusual filenames
args: ["imageinfo", path.realpath],
print_stderr: false).stdout
imageinfo = system_command("/usr/bin/hdiutil",
# realpath is a failsafe against unusual filenames
args: ["imageinfo", path.realpath],
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
Expand All @@ -27,28 +27,29 @@ def extract_to_dir(unpack_dir, basename:, verbose:)
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 = @command.run("/usr/bin/hdiutil",
args: ["attach", "-plist", "-nobrowse", "-readonly", "-noidme", "-mountrandom", unpack_dir, path],
input: "qn\n",
print_stderr: false)
without_eula = system_command("/usr/bin/hdiutil",
args: ["attach", "-plist", "-nobrowse", "-readonly", "-noidme", "-mountrandom", unpack_dir, path],
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")

system_command!("/usr/bin/hdiutil", args: ["convert", "-quiet", "-format", "UDTO", "-o", cdr_path, path])

with_eula = @command.run!("/usr/bin/hdiutil",
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
Expand All @@ -63,21 +64,22 @@ def mount
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 CaskError => e
rescue ErrorDuringExecution => e
raise e if (tries -= 1).zero?
sleep 1
retry
Expand All @@ -94,23 +96,24 @@ def extract_mount(mount, to:)
filelist.puts(bom_filelist_from_path(mount))
filelist.close

@command.run!("/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/mkbom", args: ["-s", "-i", filelist.path, "--", bomfile.path])
system_command!("/usr/bin/ditto", args: ["--bom", bomfile.path, "--", mount, to])
end
end
end

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
.split("\0")
.reject { |path| skip_path?(mount, path) }
.join("\n")
system_command("/usr/bin/find", args: [".", "-print0"], chdir: mount, print_stderr: false)
.stdout
.split("\0")
.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

Expand Down
6 changes: 3 additions & 3 deletions Library/Homebrew/cask/lib/hbc/container/generic_unar.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ def self.can_extract?(path:, magic_number:)
end

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

def dependencies
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/container/gzip.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def extract_to_dir(unpack_dir, basename:, verbose:)
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
Expand Down
8 changes: 4 additions & 4 deletions Library/Homebrew/cask/lib/hbc/container/lzma.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ def self.can_extract?(path:, magic_number:)
end

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

def dependencies
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/container/naked.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def self.can_extract?(path:, magic_number:)
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
Expand Down
6 changes: 3 additions & 3 deletions Library/Homebrew/cask/lib/hbc/container/rar.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ def self.can_extract?(path:, magic_number:)
end

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

def dependencies
Expand Down
6 changes: 3 additions & 3 deletions Library/Homebrew/cask/lib/hbc/container/seven_zip.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ def self.can_extract?(path:, magic_number:)
end

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

def dependencies
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/container/svn_repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def self.can_extract?(path:, magic_number:)
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
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/container/tar.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def self.can_extract?(path:, magic_number:)
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
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/container/xar.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def self.can_extract?(path:, magic_number:)
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
Expand Down
8 changes: 4 additions & 4 deletions Library/Homebrew/cask/lib/hbc/container/xz.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ def self.can_extract?(path:, magic_number:)
end

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

def dependencies
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/container/zip.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def self.can_extract?(path:, magic_number:)

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
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/lib/hbc/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ def primary_container
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

Expand Down
8 changes: 8 additions & 0 deletions Library/Homebrew/system_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@
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

Expand Down
13 changes: 4 additions & 9 deletions Library/Homebrew/test/cask/container/dmg_spec.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
describe Hbc::Container::Dmg, :cask do
describe "#mount!" do
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,
)
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
dmg.mount do |mounts|
begin
expect(mounts).not_to include nil
Expand Down
11 changes: 4 additions & 7 deletions Library/Homebrew/test/cask/container/naked_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,11 @@
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(
["/usr/bin/ditto", "--", path, expected_destination],
)
expect(container).to receive(:system_command!)
.with("/usr/bin/ditto", args: ["--", path, expected_destination])

expect {
container.extract(to: cask.staged_path, basename: "kevin spacey.pkg")
}.not_to raise_error
container.extract(to: cask.staged_path, basename: "kevin spacey.pkg")
end
end