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

Doubledash hygiene for external commands #2613

Merged
merged 13 commits into from
Feb 4, 2014
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions developer/bin/develop_brew_cask
Original file line number Diff line number Diff line change
Expand Up @@ -56,23 +56,23 @@ cd_to_version_dir () {
not_inside_homebrew () {
local tap_dir="$1"
local git_root="$2"
if [[ "$(/usr/bin/stat -L -f '%i' "$tap_dir")" -eq "$(/usr/bin/stat -L -f '%i' "$git_root")" ]]; then
if [[ "$(/usr/bin/stat -L -f '%i' -- "$tap_dir")" -eq "$(/usr/bin/stat -L -f '%i' -- "$git_root")" ]]; then
die "\nERROR: Run this script in your private repo, not inside Homebrew.\n"
fi
}

create_dev_links () {
local tap_dir="$1"
local git_root="$2"
/bin/mv rubylib production_rubylib
/bin/mv Casks production_Casks
/bin/ln -s "$git_root/Casks" .
/bin/ln -s "$git_root/lib" rubylib
/bin/mv -- rubylib production_rubylib
/bin/mv -- Casks production_Casks
/bin/ln -s -- "$git_root/Casks" .
/bin/ln -s -- "$git_root/lib" rubylib
cd "$tap_dir"
/bin/mv lib production_lib
/bin/mv Casks production_Casks
/bin/ln -s "$git_root/Casks" .
/bin/ln -s "$git_root/lib" .
/bin/mv -- lib production_lib
/bin/mv -- Casks production_Casks
/bin/ln -s -- "$git_root/Casks" .
/bin/ln -s -- "$git_root/lib" .
printf "brew-cask is now in development mode\n"
printf "Note: it is not safe to run 'brew update' while in development mode\n"
}
Expand All @@ -88,7 +88,7 @@ _develop_brew_cask () {
local git_root="$(/bin/pwd)"
local brew_prefix="$(brew --prefix)"
local cellar_dir="$brew_prefix/Cellar/brew-cask"
local version_dir="$(/bin/ls "$cellar_dir/" | /usr/bin/sort | /usr/bin/tail -1)"
local version_dir="$(/bin/ls -- "$cellar_dir/" | /usr/bin/sort | /usr/bin/tail -1)"
local tap_dir="$brew_prefix/$tap_subdir"

# sanity check
Expand Down
2 changes: 1 addition & 1 deletion developer/bin/list_ids_in_pkg
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ mark_up_sources () {
_list_ids_in_pkg () {

tmpdir=`/usr/bin/mktemp -d -t list_ids_in_pkg`
trap "/bin/rm -rf '$tmpdir'" EXIT
trap "/bin/rm -rf -- '$tmpdir'" EXIT

/usr/sbin/pkgutil --expand "$1" "$tmpdir/unpack"

Expand Down
16 changes: 8 additions & 8 deletions developer/bin/production_brew_cask
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,20 @@ cd_to_version_dir () {
not_inside_homebrew () {
local tap_dir="$1"
local git_root="$2"
if [[ "$(/usr/bin/stat -L -f '%i' "$tap_dir")" -eq "$(/usr/bin/stat -L -f '%i' "$git_root")" ]]; then
if [[ "$(/usr/bin/stat -L -f '%i' -- "$tap_dir")" -eq "$(/usr/bin/stat -L -f '%i' -- "$git_root")" ]]; then
die "\nERROR: Run this script in your private repo, not inside Homebrew.\n"
fi
}

remove_dev_links () {
local tap_dir="$1"
/bin/rm rubylib Casks
/bin/mv production_rubylib rubylib
/bin/mv production_Casks Casks
/bin/rm -- rubylib Casks
/bin/mv -- production_rubylib rubylib
/bin/mv -- production_Casks Casks
cd "$tap_dir"
/bin/rm lib Casks
/bin/mv production_lib lib
/bin/mv production_Casks Casks
/bin/rm -- lib Casks
/bin/mv -- production_lib lib
/bin/mv -- production_Casks Casks
printf "brew-cask is now in production mode\n"
printf "It is safe to run 'brew update' if you are in production mode for all Caskroom repos.\n"
}
Expand All @@ -85,7 +85,7 @@ _production_brew_cask () {
local git_root="$(/bin/pwd)"
local brew_prefix="$(brew --prefix)"
local cellar_dir="$brew_prefix/Cellar/brew-cask"
local version_dir="$(/bin/ls "$cellar_dir/" | /usr/bin/sort | /usr/bin/tail -1)"
local version_dir="$(/bin/ls -- "$cellar_dir/" | /usr/bin/sort | /usr/bin/tail -1)"
local tap_dir="$brew_prefix/$tap_subdir"

# sanity check
Expand Down
4 changes: 2 additions & 2 deletions lib/cask.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ def self.init
system '/bin/mkdir', caskroom
else
# sudo in system is rude.
system '/usr/bin/sudo', '/bin/mkdir', '-p', caskroom
system '/usr/bin/sudo', '/usr/sbin/chown', '-R', "#{current_user}:staff", caskroom.parent
system '/usr/bin/sudo', '--', '/bin/mkdir', '-p', '--', caskroom
system '/usr/bin/sudo', '--', '/usr/sbin/chown', '-R', '--', "#{current_user}:staff", caskroom.parent
end
end
appdir.mkpath unless appdir.exist?
Expand Down
2 changes: 1 addition & 1 deletion lib/cask/artifact/hardlinked.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ def self.link_type_english_name
end

def create_filesystem_link(source, target)
@command.run!('/bin/ln', :args => ['-hf', source, target])
@command.run!('/bin/ln', :args => ['-hf', '--', source, target])
end
end
6 changes: 3 additions & 3 deletions lib/cask/artifact/pkg.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def manually_uninstall(uninstall_options)
[false, true].each do |with_sudo|
xml_status = @command.run('/bin/launchctl', :args => ['list', '-x', service], :sudo => with_sudo)
if %r{^<\?xml}.match(xml_status)
@command.run!('/bin/launchctl', :args => ['remove', service], :sudo => with_sudo)
@command.run!('/bin/launchctl', :args => ['remove', '--', service], :sudo => with_sudo)
end
end
end
Expand All @@ -97,7 +97,7 @@ def manually_uninstall(uninstall_options)
ohai "Unloading kernel extension #{kext}"
is_loaded = @command.run!('/usr/sbin/kextstat', :args => ['-l', '-b', kext], :sudo => true)
if is_loaded.length > 1
@command.run!('/sbin/kextunload', :args => ['-b', kext], :sudo => true)
@command.run!('/sbin/kextunload', :args => ['-b', '--', kext], :sudo => true)
end
end
end
Expand All @@ -118,7 +118,7 @@ def manually_uninstall(uninstall_options)
if uninstall_options.key? :files
uninstall_options[:files].each do |file|
ohai "Removing file #{file}"
@command.run!('/bin/rm', :args => ['-rf', file], :sudo => true)
@command.run!('/bin/rm', :args => ['-rf', '--', file], :sudo => true)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/cask/artifact/symlinked.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def self.link_type_english_name
end

def create_filesystem_link(source, target)
@command.run!('/bin/ln', :args => ['-hfs', source, target])
@command.run!('/bin/ln', :args => ['-hfs', '--', source, target])
end

def link(artifact_relative_path)
Expand Down
4 changes: 2 additions & 2 deletions lib/cask/cli/home.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ module Cask::CLI::Home
def self.run(*cask_names)
if cask_names.empty?
odebug "Opening project homepage"
system "/usr/bin/open", 'http://caskroom.io/'
system "/usr/bin/open", '--', 'http://caskroom.io/'
else
cask_names.each do |cask_name|
odebug "Opening homepage for Cask #{cask_name}"
cask = Cask.load(cask_name)
system "/usr/bin/open", cask.homepage
system "/usr/bin/open", '--', cask.homepage
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/cask/container/criteria.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def initialize(path, command)
end

def file
@file ||= @command.run('/usr/bin/file', :args => ['-Izb', path])
@file ||= @command.run('/usr/bin/file', :args => ['-Izb', '--', path])
end

def imageinfo
Expand Down
2 changes: 1 addition & 1 deletion lib/cask/container/dmg.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def extract
mount!
assert_mounts_found
@mounts.each do |mount|
@command.run('/usr/bin/ditto', :args => [mount, @cask.destination_path])
@command.run('/usr/bin/ditto', :args => ['--', mount, @cask.destination_path])
end
ensure
eject!
Expand Down
2 changes: 1 addition & 1 deletion lib/cask/container/naked.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def self.me?(criteria)
end

def extract
@command.run!('/usr/bin/ditto', :args => [@path, @cask.destination_path.join(target_file)])
@command.run!('/usr/bin/ditto', :args => ['--', @path, @cask.destination_path.join(target_file)])
end

def target_file
Expand Down
2 changes: 1 addition & 1 deletion lib/cask/container/tar.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def self.me?(criteria)
def extract
Dir.mktmpdir do |staging_dir|
@command.run!('/usr/bin/tar', :args => ['xf', @path, '-C', staging_dir])
@command.run!('/usr/bin/ditto', :args => [staging_dir, @cask.destination_path])
@command.run!('/usr/bin/ditto', :args => ['--', staging_dir, @cask.destination_path])
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/cask/container/zip.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def self.me?(criteria)
def extract
Dir.mktmpdir do |staging_dir|
@command.run!('/usr/bin/unzip', :args => ['-qq', '-d', staging_dir, @path, '-x', '__MACOSX/*'])
@command.run!('/usr/bin/ditto', :args => [staging_dir, @cask.destination_path])
@command.run!('/usr/bin/ditto', :args => ['--', staging_dir, @cask.destination_path])
end
end
end
12 changes: 6 additions & 6 deletions lib/cask/pkg.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def initialize(package_id, command=Cask::SystemCommand)
def uninstall
odebug "Deleting pkg files"
list('files').each_slice(500) do |file_slice|
@command.run('/bin/rm', :args => file_slice.unshift('-f'), :sudo => true)
@command.run('/bin/rm', :args => file_slice.unshift('-f', '--'), :sudo => true)
end
odebug "Deleting pkg directories"
_deepest_path_first(list('dirs')).each do |dir|
Expand Down Expand Up @@ -54,17 +54,17 @@ def info

def _rmdir(path)
if path.children.empty?
@command.run!('/bin/rmdir', :args => [path], :sudo => true)
@command.run!('/bin/rmdir', :args => ['--', path], :sudo => true)
end
end

def _with_full_permissions(path, &block)
original_mode = (path.stat.mode % 01000).to_s(8)
@command.run!('/bin/chmod', :args => ['777', path], :sudo => true)
@command.run!('/bin/chmod', :args => ['--', '777', path], :sudo => true)
block.call
ensure
if path.exist? # block may have removed dir
@command.run!('/bin/chmod', :args => [original_mode, path], :sudo => true)
@command.run!('/bin/chmod', :args => ['--', original_mode, path], :sudo => true)
end
end

Expand All @@ -80,14 +80,14 @@ def _deepest_path_first(paths)
def _clean_broken_symlinks(dir)
dir.children.each do |child|
if _broken_symlink?(child)
@command.run!('/bin/rm', :args => [child], :sudo => true)
@command.run!('/bin/rm', :args => ['--', child], :sudo => true)
end
end
end

def _clean_ds_store(dir)
ds_store = dir.join('.DS_Store')
@command.run!('/bin/rm', :args => [ds_store], :sudo => true) if ds_store.exist?
@command.run!('/bin/rm', :args => ['--', ds_store], :sudo => true) if ds_store.exist?
end

def _broken_symlink?(path)
Expand Down
2 changes: 1 addition & 1 deletion lib/cask/system_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def self.run!(command, options)

def self._process_options(command, options)
if options[:sudo]
command = "/usr/bin/sudo -E #{_quote(command)}"
command = "/usr/bin/sudo -E -- #{_quote(command)}"
end
if options[:args]
command = "#{command} #{options[:args].map { |arg| _quote(arg) }.join(' ')}"
Expand Down
27 changes: 14 additions & 13 deletions test/cask/artifact/pkg_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
it 'runs the system installer on the specified pkgs' do
pkg = Cask::Artifact::Pkg.new(@cask, Cask::FakeSystemCommand)

expected_command = "/usr/bin/sudo -E '/usr/sbin/installer' '-pkg' '#{@cask.destination_path/'MyFancyPkg'/'Fancy.pkg'}' '-target' '/' 2>&1"
expected_command = "/usr/bin/sudo -E -- '/usr/sbin/installer' '-pkg' '#{@cask.destination_path/'MyFancyPkg'/'Fancy.pkg'}' '-target' '/' 2>&1"
Cask::FakeSystemCommand.stubs_command(expected_command)

shutup do
Expand All @@ -27,10 +27,10 @@
it 'runs the specified uninstaller for the cask' do
pkg = Cask::Artifact::Pkg.new(@cask, Cask::FakeSystemCommand)

Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E '/usr/bin/osascript' '-e' 'tell application "System Events" to count processes whose bundle identifier is "my.fancy.package.app"' 2>&1), '1')
Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E '/usr/bin/osascript' '-e' 'tell application id "my.fancy.package.app" to quit' 2>&1))
Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/usr/bin/osascript' '-e' 'tell application "System Events" to count processes whose bundle identifier is "my.fancy.package.app"' 2>&1), '1')
Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/usr/bin/osascript' '-e' 'tell application id "my.fancy.package.app" to quit' 2>&1))

expected_command = "/usr/bin/sudo -E '#{@cask.destination_path/'MyFancyPkg'/'FancyUninstaller.tool'}' '--please' 2>&1"
expected_command = "/usr/bin/sudo -E -- '#{@cask.destination_path/'MyFancyPkg'/'FancyUninstaller.tool'}' '--please' 2>&1"
Cask::FakeSystemCommand.stubs_command(expected_command)

shutup do
Expand Down Expand Up @@ -82,12 +82,13 @@
</plist>
PLIST
)

Cask::FakeSystemCommand.stubs_command(
%Q(/bin/launchctl 'list' '-x' 'my.fancy.package.service' 2>&1),
"launchctl list returned unknown response\n"
)
Cask::FakeSystemCommand.stubs_command(
%Q(/usr/bin/sudo -E '/bin/launchctl' 'list' '-x' 'my.fancy.package.service' 2>&1),
%Q(/usr/bin/sudo -E -- '/bin/launchctl' 'list' '-x' 'my.fancy.package.service' 2>&1),
<<-PLIST
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
Expand All @@ -111,11 +112,11 @@
</plist>
PLIST
)
Cask::FakeSystemCommand.expects_command(%Q(/usr/bin/sudo -E '/bin/launchctl' 'remove' 'my.fancy.package.service' 2>&1))
Cask::FakeSystemCommand.expects_command(%Q(/usr/bin/sudo -E -- '/bin/launchctl' 'remove' '--' 'my.fancy.package.service' 2>&1))

Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E '/usr/sbin/kextstat' '-l' '-b' 'my.fancy.package.kernelextension' 2>&1), 'loaded')
Cask::FakeSystemCommand.expects_command(%Q(/usr/bin/sudo -E '/sbin/kextunload' '-b' 'my.fancy.package.kernelextension' 2>&1))
Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E '/usr/sbin/pkgutil' '--forget' 'my.fancy.package.main' 2>&1))
Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/usr/sbin/kextstat' '-l' '-b' 'my.fancy.package.kernelextension' 2>&1), 'loaded')
Cask::FakeSystemCommand.expects_command(%Q(/usr/bin/sudo -E -- '/sbin/kextunload' '-b' '--' 'my.fancy.package.kernelextension' 2>&1))
Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/usr/sbin/pkgutil' '--forget' 'my.fancy.package.main' 2>&1))

Cask::FakeSystemCommand.stubs_command(
%Q(/usr/sbin/pkgutil '--only-files' '--files' 'my.fancy.package.agent' 2>&1),
Expand Down Expand Up @@ -154,13 +155,13 @@
/tmp/fancy/bin
/tmp/fancy/var
].each do |dir|
Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E '/bin/chmod' '777' '#{dir}' 2>&1))
Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/bin/chmod' '--' '777' '#{dir}' 2>&1))
end

Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E '/usr/sbin/pkgutil' '--forget' 'my.fancy.package.agent' 2>&1))
Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/usr/sbin/pkgutil' '--forget' 'my.fancy.package.agent' 2>&1))

Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E '/bin/rm' '-f' '/tmp/fancy/bin/fancy.exe' '/tmp/fancy/var/fancy.data' 2>&1))
Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E '/bin/rm' '-f' '/tmp/fancy/agent/fancy-agent.exe' '/tmp/fancy/agent/fancy-agent.pid' '/tmp/fancy/agent/fancy-agent.log' 2>&1))
Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/bin/rm' '-f' '--' '/tmp/fancy/bin/fancy.exe' '/tmp/fancy/var/fancy.data' 2>&1))
Cask::FakeSystemCommand.stubs_command(%Q(/usr/bin/sudo -E -- '/bin/rm' '-f' '--' '/tmp/fancy/agent/fancy-agent.exe' '/tmp/fancy/agent/fancy-agent.pid' '/tmp/fancy/agent/fancy-agent.log' 2>&1))

# No assertions after call since all assertions are implicit from the interactions setup above.
# TODO: verify rmdir commands (requires setting up actual file tree or faking out .exists?
Expand Down
8 changes: 4 additions & 4 deletions test/cask/cli/home_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,22 @@ module Cask::CLI::Home
it 'opens the homepage for the specified cask' do
Cask::CLI::Home.run('alfred')
Cask::CLI::Home.system_commands.must_equal [
['/usr/bin/open', 'http://www.alfredapp.com/']
['/usr/bin/open', '--', 'http://www.alfredapp.com/']
]
end

it 'works for multiple casks' do
Cask::CLI::Home.run('alfred', 'adium')
Cask::CLI::Home.system_commands.must_equal [
['/usr/bin/open', 'http://www.alfredapp.com/'],
['/usr/bin/open', 'https://www.adium.im/']
['/usr/bin/open', '--', 'http://www.alfredapp.com/'],
['/usr/bin/open', '--', 'https://www.adium.im/']
]
end

it "opens the project page when no cask is specified" do
Cask::CLI::Home.run
Cask::CLI::Home.system_commands.must_equal [
['/usr/bin/open', 'http://caskroom.io/'],
['/usr/bin/open', '--', 'http://caskroom.io/'],
]
end
end
Expand Down
2 changes: 1 addition & 1 deletion test/cask/container/naked_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
cask = SpaceyCask.new
path = '/tmp/downloads/kevin-spacey-1.2.pkg'
expected_destination = cask.destination_path.join('kevin spacey.pkg')
expected_command = %Q(/usr/bin/ditto '#{path}' '#{expected_destination}' 2>&1)
expected_command = %Q(/usr/bin/ditto '--' '#{path}' '#{expected_destination}' 2>&1)
Cask::FakeSystemCommand.stubs_command(expected_command)

container = Cask::Container::Naked.new(cask, path, Cask::FakeSystemCommand)
Expand Down
2 changes: 1 addition & 1 deletion test/cask/pkg_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
)

Cask::FakeSystemCommand.expects_command(
%q(/usr/bin/sudo -E '/usr/sbin/pkgutil' '--forget' 'my.fake.pkg' 2>&1)
%q(/usr/bin/sudo -E -- '/usr/sbin/pkgutil' '--forget' 'my.fake.pkg' 2>&1)
)

pkg.uninstall
Expand Down