Skip to content

Commit c952f02

Browse files
authored
Prefer command -v <cmd> to which <cmd> (heroku#1716)
* Prefer `command -v <cmd>` to `which <cmd>` Mentioned in heroku#1714 (comment) `command -v <cmd>` is a POSIX shell builtin that prints the path of a command (or the alias/function definition) if it exists, and returns a non-zero exit code if it doesn't. Because it's a shell builtin, it's available in any POSIX-compliant shell (bash, zsh, dash, sh, etc.) without depending on an external binary. https://stackoverflow.com/questions/592620/how-can-i-check-if-a-program-exists-from-a-bash-script * Fix failure ``` 1) LanguagePack::Helpers::BinstubCheck can determine if a file is binary or not Failure/Error: out = `command -v ruby`.strip Errno::ENOENT: No such file or directory - command # ./spec/helpers/binstub_check_spec.rb:5:in ``' # ./spec/helpers/binstub_check_spec.rb:5:in `get_ruby_path!' # ./spec/helpers/binstub_check_spec.rb:29:in `block (2 levels) in <top (required)>' # ./vendor/bundle/ruby/3.3.0/gems/rspec-core-3.13.6/lib/rspec/core/example.rb:263:in `instance_exec' # ./vendor/bundle/ruby/3.3.0/gems/rspec-core-3.13.6/lib/rspec/core/example.rb:263:in `block in run' # ./vendor/bundle/ruby/3.3.0/gems/rspec-core-3.13.6/lib/rspec/core/example.rb:511:in `block in with_around_and_singleton_context_hooks' # ./vendor/bundle/ruby/3.3.0/gems/rspec-core-3.13.6/lib/rspec/core/example.rb:468:in `block in with_around_example_hooks' # ./vendor/bundle/ruby/3.3.0/gems/rspec-core-3.13.6/lib/rspec/core/hooks.rb:486:in `block in run' # ./vendor/bundle/ruby/3.3.0/gems/rspec-core-3.13.6/lib/rspec/core/hooks.rb:626:in `block in run_around_example_hooks_for' # ./vendor/bundle/ruby/3.3.0/gems/rspec-core-3.13.6/lib/rspec/core/example.rb:352:in `call' # ./vendor/bundle/ruby/3.3.0/gems/rspec-retry-0.6.2/lib/rspec/retry.rb:124:in `block in run' # <internal:kernel>:187:in `loop' ``` The belief is that the failure mode is due to Ruby trying to lookup `command` on the PATH and not finding it. This behavior is documented in Kernel#exec https://www.rubydoc.info/stdlib/core/Kernel:exec but not on backticks (they use different codepaths, just noting). Backticks https://ruby-doc.org/3.4.1/Kernel.html#method-i-60 have an issue reported but closed https://bugs.ruby-lang.org/issues/4477. Adding a stderr redirect triggers the correct behavior.
1 parent e54976e commit c952f02

7 files changed

Lines changed: 14 additions & 14 deletions

File tree

bin/support/bash_functions.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ curl_retry_on_18() {
3333

3434
which_java()
3535
{
36-
which java > /dev/null
36+
command -v java > /dev/null
3737
}
3838

3939
# Detects if a given Gemfile.lock has jruby in it

lib/language_pack/rails3.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,13 @@ def warn_x_sendfile_use!
118118
end
119119

120120
def has_apache?
121-
path = run("which apachectl")
121+
path = run("command -v apachectl")
122122
return true if path && $?.success?
123123
false
124124
end
125125

126126
def has_nginx?
127-
path = run("which nginx")
127+
path = run("command -v nginx")
128128
return true if path && $?.success?
129129
false
130130
end

lib/language_pack/ruby.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -822,7 +822,7 @@ def node_preinstall_bin_path
822822
return @node_preinstall_bin_path if defined?(@node_preinstall_bin_path)
823823

824824
legacy_path = "#{Dir.pwd}/#{NODE_BP_PATH}"
825-
path = run("which node").strip
825+
path = run("command -v node").strip
826826
@node_preinstall_bin_path = if path && $?.success?
827827
path
828828
elsif run("#{legacy_path}/node -v") && $?.success?
@@ -850,7 +850,7 @@ def self.yarn_preinstall_bin_path
850850
def self.yarn_preinstall_binary_path
851851
return @yarn_preinstall_binary_path if defined?(@yarn_preinstall_binary_path)
852852

853-
path = run("which yarn").strip
853+
path = run("command -v yarn").strip
854854
@yarn_preinstall_binary_path = if path && $?.success?
855855
path
856856
else

spec/hatchet/getting_started_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
# Assert no warnings from `cp`
2222
# https://github.com/heroku/heroku-buildpack-ruby/pull/1586/files#r2064284286
2323
expect(app.output).to_not include("cp --help")
24-
expect(app.run("which ruby").strip).to eq("/app/bin/ruby")
24+
expect(app.run("command -v ruby").strip).to eq("/app/bin/ruby")
2525

2626
environment_variables = app.run("env")
2727
expect(environment_variables).to match("PUMA_PERSISTENT_TIMEOUT")
@@ -66,7 +66,7 @@
6666
# Assert no warnings from `cp`
6767
# https://github.com/heroku/heroku-buildpack-ruby/pull/1586/files#r2064284286
6868
expect(app.output).to_not include("cp --help")
69-
expect(app.run("which ruby").strip).to eq("/app/bin/ruby")
69+
expect(app.run("command -v ruby").strip).to eq("/app/bin/ruby")
7070
end
7171
end
7272
end

spec/hatchet/node_spec.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
expect(app.output).to include("Installing a default version (#{LanguagePack::Helpers::Nodebin::YARN_VERSION}) of Yarn")
2020
expect(app.output).to include("Installing a default version (#{LanguagePack::Helpers::Nodebin::NODE_VERSION}) of Node.js")
2121

22-
expect(app.run("which node")).to match("/app/bin/node") # We put node in bin/node
23-
expect(app.run("which yarn")).to match("/app/vendor/yarn-") # We put yarn in /app/vendor/yarn-
22+
expect(app.run("command -v node")).to match("/app/bin/node") # We put node in bin/node
23+
expect(app.run("command -v yarn")).to match("/app/vendor/yarn-") # We put yarn in /app/vendor/yarn-
2424
end
2525
end
2626

@@ -37,8 +37,8 @@
3737
expect(app.output).to include(".heroku/node/bin/yarn is the yarn directory ")
3838
expect(app.output).to include(".heroku/node/bin/node is the node directory")
3939

40-
expect(app.run("which node")).to match("/app/.heroku/node/bin")
41-
expect(app.run("which yarn")).to match("/app/.heroku/node/bin")
40+
expect(app.run("command -v node")).to match("/app/.heroku/node/bin")
41+
expect(app.run("command -v yarn")).to match("/app/.heroku/node/bin")
4242
end
4343
end
4444
end

spec/hatchet/ruby_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@
207207
raise e
208208
end
209209

210-
expect(app.run("which ruby").strip).to eq("/app/bin/ruby")
210+
expect(app.run("command -v ruby").strip).to eq("/app/bin/ruby")
211211
end
212212
end
213213
end

spec/helpers/binstub_check_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22

33
describe LanguagePack::Helpers::BinstubCheck do
44
def get_ruby_path!
5-
out = `which ruby`.strip
6-
raise "command `which ruby` failed with output: #{out}" unless $?.success?
5+
out = `command -v ruby 2>/dev/null`.strip
6+
raise "command `command -v ruby` failed with output: #{out}" unless $?.success?
77

88
Pathname.new(out)
99
end

0 commit comments

Comments
 (0)