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

Allow specifying pkg target: #14393

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions Library/Homebrew/cask/artifact/pkg.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class Pkg < AbstractArtifact
attr_reader :path, :stanza_options

def self.from_args(cask, path, **stanza_options)
stanza_options.assert_valid_keys!(:allow_untrusted, :choices)
stanza_options.assert_valid_keys!(:allow_untrusted, :choices, :target)
new(cask, path, **stanza_options)
end

Expand Down Expand Up @@ -54,7 +54,7 @@ def run_installer(command: nil, verbose: false, **_options)

args = [
"-pkg", path,
"-target", "/"
"-target", target
]
args << "-verboseR" if verbose
args << "-allowUntrusted" if stanza_options.fetch(:allow_untrusted, false)
Expand All @@ -65,7 +65,8 @@ def run_installer(command: nil, verbose: false, **_options)
"USER" => User.current,
"USERNAME" => User.current,
}
command.run!("/usr/sbin/installer", sudo: true, args: args, print_stdout: true, env: env)
sudo = target != "CurrentUserHomeDirectory"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we expect the cask to pass paths as strings, but special values like CurrentUserHomeDirectory as a Symbol?

Suggested change
sudo = target != "CurrentUserHomeDirectory"
sudo = target != :CurrentUserHomeDirectory

command.run!("/usr/sbin/installer", sudo: sudo, args: args, print_stdout: true, env: env)
end
end

Expand All @@ -81,6 +82,10 @@ def with_choices_file
file.unlink
end
end

def target
stanza_options.fetch(:target, "/")
end
end
end
end
48 changes: 48 additions & 0 deletions Library/Homebrew/test/cask/artifact/pkg_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,52 @@
pkg.install_phase(command: fake_system_command)
end
end

describe "target" do
describe "with volume target" do
let(:cask) { Cask::CaskLoader.load(cask_path("with-pkg-target")) }

it "runs the system installer on the specified pkgs with the specified target" do
pkg = cask.artifacts.find { |a| a.is_a?(described_class) }

expect(fake_system_command).to receive(:run!).with(
"/usr/sbin/installer",
args: ["-pkg", cask.staged_path.join("MyFancyPkg", "Fancy.pkg"),
"-target", "/Volumes/Macintosh HD2"],
sudo: true,
print_stdout: true,
env: {
"LOGNAME" => ENV.fetch("USER"),
"USER" => ENV.fetch("USER"),
"USERNAME" => ENV.fetch("USER"),
},
)

pkg.install_phase(command: fake_system_command)
end
end

describe "with CurrentUserHomeDirectory target" do
let(:cask) { Cask::CaskLoader.load(cask_path("with-pkg-target-current-user-home-directory")) }

it "runs the system installer on the specified pkgs with the specified target" do
pkg = cask.artifacts.find { |a| a.is_a?(described_class) }

expect(fake_system_command).to receive(:run!).with(
"/usr/sbin/installer",
args: ["-pkg", cask.staged_path.join("MyFancyPkg", "Fancy.pkg"),
"-target", "CurrentUserHomeDirectory"],
sudo: false,
Comment on lines +114 to +115
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm making an assumption here, but I feel like it's reasonable. If we're installing in the home directory, isn't it an error if sudo is required?

print_stdout: true,
env: {
"LOGNAME" => ENV.fetch("USER"),
"USER" => ENV.fetch("USER"),
"USERNAME" => ENV.fetch("USER"),
},
)

pkg.install_phase(command: fake_system_command)
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
cask "with-pkg-target-current-user-home-directory" do
version "1.2.3"
sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b"

url "file://#{TEST_FIXTURE_DIR}/cask/MyFancyPkg.zip"
homepage "https://brew.sh/fancy-pkg"

pkg "MyFancyPkg/Fancy.pkg", target: "CurrentUserHomeDirectory"

uninstall pkgutil: "my.fancy.package"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied both these fixtures from the with-installable fixture, but I simplified the uninstall stanza, since all the extra stuff there isn't relevant to the specs these are used in.

end
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
cask "with-pkg-target" do
version "1.2.3"
sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b"

url "file://#{TEST_FIXTURE_DIR}/cask/MyFancyPkg.zip"
homepage "https://brew.sh/fancy-pkg"

pkg "MyFancyPkg/Fancy.pkg", target: "/Volumes/Macintosh HD2"

uninstall pkgutil: "my.fancy.package"
end