From bdcd0cf62662c17305f6214dcf643989649ea4d5 Mon Sep 17 00:00:00 2001 From: Caleb Thompson Date: Tue, 20 Jun 2017 17:30:14 -0500 Subject: [PATCH 1/2] Warn if ruby/rails/bundle binstub shebangs are bad Binstubs, which RubyGems and Bundler generate to prepare the environment before dispatching calls to original executables, can sometimes include shebangs that worked on developer machines, but won't work on Heroku. Often, this is caused by ruby binaries being named other than `ruby` (or `ruby.exe` on Windows). While it can happen for any binstub, we're mostly concerned about `rake`, `rails`, and `bundler` stubs. Check each of these to see if they are `!#/usr/bin/env ruby` or `!#/usr/bin/env ruby.exe` and issue a warning if this is not the case. --- .../installers/heroku_ruby_installer.rb | 15 ++++++++ spec/installers/heroku_ruby_installer_spec.rb | 37 +++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/lib/language_pack/installers/heroku_ruby_installer.rb b/lib/language_pack/installers/heroku_ruby_installer.rb index c752be37..aff7c230 100644 --- a/lib/language_pack/installers/heroku_ruby_installer.rb +++ b/lib/language_pack/installers/heroku_ruby_installer.rb @@ -22,5 +22,20 @@ def fetch_unpack(ruby_version, install_dir, build = false) @fetcher.fetch_untar(file) end end + + def setup_binstubs(*) + super + Dir["#{DEFAULT_BIN_DIR}/{rake,bundle,rails}"].select do |binstub| + begin + if File.file?(binstub) + shebang = File.open(binstub, &:readline) + if !shebang.match %r{\A#!/usr/bin/env ruby(.exe)?\z} + warn("Binstub #{binstub} contains shebang #{shebang}. This may cause issues if the program specified is unavailable.", inline: true) + end + end + rescue EOFError + end + end + end end diff --git a/spec/installers/heroku_ruby_installer_spec.rb b/spec/installers/heroku_ruby_installer_spec.rb index 6648a870..57cba2d3 100644 --- a/spec/installers/heroku_ruby_installer_spec.rb +++ b/spec/installers/heroku_ruby_installer_spec.rb @@ -48,4 +48,41 @@ end end + + describe "#setup_binstubs" do + it "warns about malformed shebangs" do + Dir.mktmpdir do |dir| + Dir.chdir(dir) do + Dir.mkdir("bin") + File.open("bin/rake", 'w') { |f| f.write("#!/usr/bin/env ruby") } + File.open("bin/rails", 'w') { |f| f.write("#!/usr/bin/env ruby.exe") } + File.open("bin/bundle", 'w') { |f| f.write("#!/usr/bin/env ruby1.7") } + + output = capture(:stdout) { installer.setup_binstubs("#{dir}/vendor/ruby") } + + expect(output).to include(<<-WARNING) +###### WARNING: + Binstub bin/bundle contains shebang #!/usr/bin/env ruby1.7. This may cause issues if the program specified is unavailable. + WARNING + end + end + end + + def capture(stream) + stream = stream.to_s + captured_stream = Tempfile.new(stream) + stream_io = eval("$#{stream}") + origin_stream = stream_io.dup + stream_io.reopen(captured_stream) + + yield + + stream_io.rewind + return captured_stream.read + ensure + captured_stream.close + captured_stream.unlink + stream_io.reopen(origin_stream) + end + end end From ad7d7648658348c9fae7683266782c4350ae9dde Mon Sep 17 00:00:00 2001 From: Caleb Thompson Date: Wed, 13 Sep 2017 11:59:37 -0500 Subject: [PATCH 2/2] Move binstub detection to main compile flow --- .../installers/heroku_ruby_installer.rb | 15 ----- lib/language_pack/ruby.rb | 15 +++++ spec/installers/heroku_ruby_installer_spec.rb | 36 ----------- spec/ruby_spec.rb | 60 +++++++++++++++---- 4 files changed, 65 insertions(+), 61 deletions(-) diff --git a/lib/language_pack/installers/heroku_ruby_installer.rb b/lib/language_pack/installers/heroku_ruby_installer.rb index aff7c230..c752be37 100644 --- a/lib/language_pack/installers/heroku_ruby_installer.rb +++ b/lib/language_pack/installers/heroku_ruby_installer.rb @@ -22,20 +22,5 @@ def fetch_unpack(ruby_version, install_dir, build = false) @fetcher.fetch_untar(file) end end - - def setup_binstubs(*) - super - Dir["#{DEFAULT_BIN_DIR}/{rake,bundle,rails}"].select do |binstub| - begin - if File.file?(binstub) - shebang = File.open(binstub, &:readline) - if !shebang.match %r{\A#!/usr/bin/env ruby(.exe)?\z} - warn("Binstub #{binstub} contains shebang #{shebang}. This may cause issues if the program specified is unavailable.", inline: true) - end - end - rescue EOFError - end - end - end end diff --git a/lib/language_pack/ruby.rb b/lib/language_pack/ruby.rb index 7837ed03..e079293f 100644 --- a/lib/language_pack/ruby.rb +++ b/lib/language_pack/ruby.rb @@ -88,6 +88,20 @@ def best_practice_warnings end end + def warn_bad_binstubs + Dir["bin/{rake,bundle,rails}"].select do |binstub| + begin + if File.file?(binstub) + shebang = File.open(binstub, &:readline) + if !shebang.match %r{\A#!/usr/bin/env ruby(.exe)?\z} + warn("Binstub #{binstub} contains shebang #{shebang}. This may cause issues if the program specified is unavailable.", inline: true) + end + end + rescue EOFError + end + end + end + def compile instrument 'ruby.compile' do # check for new app at the beginning of the compile @@ -95,6 +109,7 @@ def compile Dir.chdir(build_path) remove_vendor_bundle warn_bundler_upgrade + warn_bad_binstubs install_ruby install_jvm setup_language_pack_environment diff --git a/spec/installers/heroku_ruby_installer_spec.rb b/spec/installers/heroku_ruby_installer_spec.rb index 57cba2d3..925e3a6c 100644 --- a/spec/installers/heroku_ruby_installer_spec.rb +++ b/spec/installers/heroku_ruby_installer_spec.rb @@ -49,40 +49,4 @@ end - describe "#setup_binstubs" do - it "warns about malformed shebangs" do - Dir.mktmpdir do |dir| - Dir.chdir(dir) do - Dir.mkdir("bin") - File.open("bin/rake", 'w') { |f| f.write("#!/usr/bin/env ruby") } - File.open("bin/rails", 'w') { |f| f.write("#!/usr/bin/env ruby.exe") } - File.open("bin/bundle", 'w') { |f| f.write("#!/usr/bin/env ruby1.7") } - - output = capture(:stdout) { installer.setup_binstubs("#{dir}/vendor/ruby") } - - expect(output).to include(<<-WARNING) -###### WARNING: - Binstub bin/bundle contains shebang #!/usr/bin/env ruby1.7. This may cause issues if the program specified is unavailable. - WARNING - end - end - end - - def capture(stream) - stream = stream.to_s - captured_stream = Tempfile.new(stream) - stream_io = eval("$#{stream}") - origin_stream = stream_io.dup - stream_io.reopen(captured_stream) - - yield - - stream_io.rewind - return captured_stream.read - ensure - captured_stream.close - captured_stream.unlink - stream_io.reopen(origin_stream) - end - end end diff --git a/spec/ruby_spec.rb b/spec/ruby_spec.rb index 02d9c97f..bc88d44c 100644 --- a/spec/ruby_spec.rb +++ b/spec/ruby_spec.rb @@ -2,18 +2,18 @@ describe LanguagePack::Ruby do describe "#install_binary" do - context "installing yarn" do - before do - @old_path = ENV['PATH'] - @old_stack = ENV['STACK'] - ENV['STACK'] = 'cedar-14' - end + before do + @old_path = ENV['PATH'] + @old_stack = ENV['STACK'] + ENV['STACK'] = 'cedar-14' + end - after do - ENV['PATH'] = @old_path - ENV['STACK'] = @old_stack - end + after do + ENV['PATH'] = @old_path + ENV['STACK'] = @old_stack + end + context "installing yarn" do it "sets up PATH" do Dir.mktmpdir do |build_path| Dir.mktmpdir do |cache_path| @@ -27,5 +27,45 @@ end end + describe "#warn_bad_binstubs" do + it "warns about malformed shebangs" do + Dir.mktmpdir do |build_path| + Dir.mktmpdir do |cache_path| + Dir.chdir(build_path) do + Dir.mkdir("bin") + File.open("bin/rake", 'w') { |f| f.write("#!/usr/bin/env ruby") } + File.open("bin/rails", 'w') { |f| f.write("#!/usr/bin/env ruby.exe") } + File.open("bin/bundle", 'w') { |f| f.write("#!/usr/bin/env ruby1.7") } + + ruby = LanguagePack::Ruby.new(build_path, cache_path) + output = capture(:stdout) { ruby.send(:warn_bad_binstubs) } + + expect(output).to include(<<-WARNING) +###### WARNING: + Binstub bin/bundle contains shebang #!/usr/bin/env ruby1.7. This may cause issues if the program specified is unavailable. + WARNING + end + end + end + end + + def capture(stream) + stream = stream.to_s + captured_stream = Tempfile.new(stream) + stream_io = eval("$#{stream}") + origin_stream = stream_io.dup + stream_io.reopen(captured_stream) + + yield + + stream_io.rewind + return captured_stream.read + ensure + captured_stream.close + captured_stream.unlink + stream_io.reopen(origin_stream) + end end + end + end