Use env utility instead of with_env.

This commit is contained in:
Markus Reiter 2018-07-11 11:13:33 +02:00
parent ae38c5d6d1
commit 37f3a603ce
4 changed files with 25 additions and 26 deletions

View File

@ -47,7 +47,7 @@ module Hbc
@must_succeed = must_succeed @must_succeed = must_succeed
options.extend(HashValidator).assert_valid_keys(:chdir) options.extend(HashValidator).assert_valid_keys(:chdir)
@options = options @options = options
@env = { "PATH" => ENV["PATH"] }.merge(env) @env = env
@env.keys.grep_v(/^[\w&&\D]\w*$/) do |name| @env.keys.grep_v(/^[\w&&\D]\w*$/) do |name|
raise ArgumentError, "Invalid variable name: '#{name}'" raise ArgumentError, "Invalid variable name: '#{name}'"
@ -55,7 +55,7 @@ module Hbc
end end
def command def command
[*sudo_prefix, executable, *args] [*sudo_prefix, *env_args, executable, *args]
end end
private private
@ -64,18 +64,22 @@ module Hbc
attr_predicate :sudo?, :print_stdout?, :print_stderr?, :must_succeed? attr_predicate :sudo?, :print_stdout?, :print_stderr?, :must_succeed?
def env_args
return [] if env.empty?
variables = env.map do |name, value|
sanitized_name = Shellwords.escape(name)
sanitized_value = Shellwords.escape(value)
"#{sanitized_name}=#{sanitized_value}"
end
["env", *variables]
end
def sudo_prefix def sudo_prefix
return [] unless sudo? return [] unless sudo?
askpass_flags = ENV.key?("SUDO_ASKPASS") ? ["-A"] : [] askpass_flags = ENV.key?("SUDO_ASKPASS") ? ["-A"] : []
prefix = ["/usr/bin/sudo", *askpass_flags, "-E"] ["/usr/bin/sudo", *askpass_flags, "-E", "--"]
env.each do |name, value|
sanitized_name = Shellwords.escape(name)
sanitized_value = Shellwords.escape(value)
prefix << "#{sanitized_name}=#{sanitized_value}"
end
prefix << "--"
end end
def assert_success def assert_success
@ -97,11 +101,7 @@ module Hbc
executable, *args = expanded_command executable, *args = expanded_command
raw_stdin, raw_stdout, raw_stderr, raw_wait_thr = raw_stdin, raw_stdout, raw_stderr, raw_wait_thr =
# We need to specifically use `with_env` for `PATH`, otherwise Open3.popen3([executable, executable], *args, **options)
# Ruby itself will not look for the executable in `PATH`.
with_env "PATH" => env["PATH"] do
Open3.popen3(env, [executable, executable], *args, **options)
end
write_input_to(raw_stdin) write_input_to(raw_stdin)
raw_stdin.close_write raw_stdin.close_write

View File

@ -15,10 +15,10 @@ describe Hbc::SystemCommand, :cask do
its("run!.stdout") { is_expected.to eq("123") } its("run!.stdout") { is_expected.to eq("123") }
describe "the resulting command line" do describe "the resulting command line" do
it "does not include the given variables" do it "includes the given variables explicitly" do
expect(Open3) expect(Open3)
.to receive(:popen3) .to receive(:popen3)
.with(a_hash_including("PATH"), ["env"] * 2, *env_args, {}) .with(["env", "env"], "A=1", "B=2", "C=3", "env", *env_args, {})
.and_call_original .and_call_original
subject.run! subject.run!
@ -41,9 +41,8 @@ describe Hbc::SystemCommand, :cask do
it "includes the given variables explicitly" do it "includes the given variables explicitly" do
expect(Open3) expect(Open3)
.to receive(:popen3) .to receive(:popen3)
.with(an_instance_of(Hash), ["/usr/bin/sudo"] * 2, .with(["/usr/bin/sudo", "/usr/bin/sudo"], "-E", "--",
"-E", a_string_starting_with("PATH="), "env", "A=1", "B=2", "C=3", "env", *env_args, {})
"A=1", "B=2", "C=3", "--", "env", *env_args, {})
.and_wrap_original do |original_popen3, *_, &block| .and_wrap_original do |original_popen3, *_, &block|
original_popen3.call("/usr/bin/true", &block) original_popen3.call("/usr/bin/true", &block)
end end
@ -197,7 +196,7 @@ describe Hbc::SystemCommand, :cask do
end end
end end
it "looks for executables in custom PATH" do it "looks for executables in a custom PATH" do
mktmpdir do |path| mktmpdir do |path|
(path/"tool").write <<~SH (path/"tool").write <<~SH
#!/bin/sh #!/bin/sh

View File

@ -1,5 +1,5 @@
def sudo(*args) def sudo(*args)
["/usr/bin/sudo", "-E", "PATH=#{ENV["PATH"]}", "--"] + args.flatten ["/usr/bin/sudo", "-E", "--"] + args.flatten
end end
module Hbc module Hbc

View File

@ -80,7 +80,7 @@ shared_examples Hbc::Staged do
allow(staged).to receive(:Pathname).and_return(fake_pathname) allow(staged).to receive(:Pathname).and_return(fake_pathname)
Hbc::FakeSystemCommand.expects_command( Hbc::FakeSystemCommand.expects_command(
["/usr/bin/sudo", "-E", "PATH=#{ENV["PATH"]}", "--", "/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname], sudo("/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname),
) )
staged.set_ownership(fake_pathname.to_s) staged.set_ownership(fake_pathname.to_s)
@ -93,7 +93,7 @@ shared_examples Hbc::Staged do
allow(staged).to receive(:Pathname).and_return(fake_pathname) allow(staged).to receive(:Pathname).and_return(fake_pathname)
Hbc::FakeSystemCommand.expects_command( Hbc::FakeSystemCommand.expects_command(
["/usr/bin/sudo", "-E", "PATH=#{ENV["PATH"]}", "--", "/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname, fake_pathname], sudo("/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname, fake_pathname),
) )
staged.set_ownership([fake_pathname.to_s, fake_pathname.to_s]) staged.set_ownership([fake_pathname.to_s, fake_pathname.to_s])
@ -105,7 +105,7 @@ shared_examples Hbc::Staged do
allow(staged).to receive(:Pathname).and_return(fake_pathname) allow(staged).to receive(:Pathname).and_return(fake_pathname)
Hbc::FakeSystemCommand.expects_command( Hbc::FakeSystemCommand.expects_command(
["/usr/bin/sudo", "-E", "PATH=#{ENV["PATH"]}", "--", "/usr/sbin/chown", "-R", "--", "other_user:other_group", fake_pathname], sudo("/usr/sbin/chown", "-R", "--", "other_user:other_group", fake_pathname),
) )
staged.set_ownership(fake_pathname.to_s, user: "other_user", group: "other_group") staged.set_ownership(fake_pathname.to_s, user: "other_user", group: "other_group")