From 45981c0580967badae19d70b2aa64d66916a1ebd Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 5 Feb 2019 11:10:26 -0600 Subject: [PATCH 1/5] De-couple BundlerWrapper from LanguagePack::Ruby --- lib/language_pack/helpers/bundler_wrapper.rb | 36 +++++++++++--------- lib/language_pack/ruby.rb | 14 ++++---- spec/helpers/fetcher_spec.rb | 5 ++- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/lib/language_pack/helpers/bundler_wrapper.rb b/lib/language_pack/helpers/bundler_wrapper.rb index 218230b26..2f2b7902b 100644 --- a/lib/language_pack/helpers/bundler_wrapper.rb +++ b/lib/language_pack/helpers/bundler_wrapper.rb @@ -1,5 +1,9 @@ require 'language_pack/fetcher' +# This class is responsible for installing and maintaining a +# reference to bundler. It contains access to bundler internals +# that are used to introspect a project such as detecting presence +# of gems and their versions. class LanguagePack::Helpers::BundlerWrapper include LanguagePack::ShellHelpers @@ -11,24 +15,20 @@ def initialize(error) end end - VENDOR_URL = LanguagePack::Base::VENDOR_URL # coupling - DEFAULT_FETCHER = LanguagePack::Fetcher.new(VENDOR_URL) # coupling - BUNDLER_DIR_NAME = LanguagePack::Ruby::BUNDLER_GEM_PATH # coupling - BUNDLER_PATH = File.expand_path("../../../../tmp/#{BUNDLER_DIR_NAME}", __FILE__) - GEMFILE_PATH = Pathname.new "./Gemfile" - - attr_reader :bundler_path + attr_reader :bundler_path def initialize(options = {}) - @fetcher = options[:fetcher] || DEFAULT_FETCHER + @version = "1.15.2" @bundler_tmp = Dir.mktmpdir - @bundler_path = options[:bundler_path] || File.join(@bundler_tmp, "#{BUNDLER_DIR_NAME}") - @gemfile_path = options[:gemfile_path] || GEMFILE_PATH - @bundler_tar = options[:bundler_tar] || "#{BUNDLER_DIR_NAME}.tgz" + @fetcher = options[:fetcher] || LanguagePack::Fetcher.new(LanguagePack::Base::VENDOR_URL) # coupling + @bundler_path = options[:bundler_path] || File.join(@bundler_tmp, "#{dir_name}") + @gemfile_path = options[:gemfile_path] || Pathname.new("./Gemfile") + @bundler_tar = options[:bundler_tar] || "#{dir_name}.tgz" + @gemfile_lock_path = "#{@gemfile_path}.lock" @orig_bundle_gemfile = ENV['BUNDLE_GEMFILE'] ENV['BUNDLE_GEMFILE'] = @gemfile_path.to_s - @path = Pathname.new "#{@bundler_path}/gems/#{BUNDLER_DIR_NAME}/lib" + @path = Pathname.new "#{@bundler_path}/gems/#{dir_name}/lib" end def install @@ -42,7 +42,7 @@ def clean ENV['BUNDLE_GEMFILE'] = @orig_bundle_gemfile FileUtils.remove_entry_secure(@bundler_tmp) if Dir.exist?(@bundler_tmp) - if LanguagePack::Ruby::BUNDLER_VERSION == "1.7.12" + if version == "1.7.12" # Hack to cleanup after pre 1.8 versions of bundler. See https://github.com/bundler/bundler/pull/3277/ Dir["#{Dir.tmpdir}/bundler*"].each do |dir| FileUtils.remove_entry_secure(dir) if Dir.exist?(dir) && File.stat(dir).writable? @@ -71,7 +71,7 @@ def windows_gemfile_lock? end def specs - @specs ||= lockfile_parser.specs.each_with_object({}) {|spec, hash| hash[spec.name] = spec } + @specs ||= lockfile_parser.specs.each_with_object({}) {|spec, hash| hash[spec.name] = spec } end def platforms @@ -79,7 +79,11 @@ def platforms end def version - Bundler::VERSION + @version + end + + def dir_name + "bundler-#{version}" end def instrument(*args, &block) @@ -89,7 +93,7 @@ def instrument(*args, &block) def ruby_version instrument 'detect_ruby_version' do env = { "PATH" => "#{bundler_path}/bin:#{ENV['PATH']}", - "RUBYLIB" => File.join(bundler_path, "gems", BUNDLER_DIR_NAME, "lib"), + "RUBYLIB" => File.join(bundler_path, "gems", dir_name, "lib"), "GEM_PATH" => "#{bundler_path}:#{ENV["GEM_PATH"]}", "BUNDLE_DISABLE_VERSION_CHECK" => "true" } diff --git a/lib/language_pack/ruby.rb b/lib/language_pack/ruby.rb index 88e42faee..a44977d85 100644 --- a/lib/language_pack/ruby.rb +++ b/lib/language_pack/ruby.rb @@ -16,8 +16,6 @@ class LanguagePack::Ruby < LanguagePack::Base NAME = "ruby" LIBYAML_VERSION = "0.1.7" LIBYAML_PATH = "libyaml-#{LIBYAML_VERSION}" - BUNDLER_VERSION = "1.15.2" - BUNDLER_GEM_PATH = "bundler-#{BUNDLER_VERSION}" RBX_BASE_URL = "http://binaries.rubini.us/heroku" NODE_BP_PATH = "vendor/node/bin" @@ -126,9 +124,9 @@ def config_detect def warn_bundler_upgrade old_bundler_version = @metadata.read("bundler_version").chomp if @metadata.exists?("bundler_version") - if old_bundler_version && old_bundler_version != BUNDLER_VERSION + if old_bundler_version && old_bundler_version != bundler.version puts(<<-WARNING) -Your app was upgraded to bundler #{ BUNDLER_VERSION }. +Your app was upgraded to bundler #{ bundler.version }. Previously you had a successful deploy with bundler #{ old_bundler_version }. If you see problems related to the bundler version please refer to: @@ -596,7 +594,7 @@ def bundler_binstubs_path end def bundler_path - @bundler_path ||= "#{slug_vendor_base}/gems/#{BUNDLER_GEM_PATH}" + @bundler_path ||= "#{slug_vendor_base}/gems/#{bundler.dir_name}" end def write_bundler_shim(path) @@ -607,7 +605,7 @@ def write_bundler_shim(path) #!/usr/bin/env ruby require 'rubygems' -version = "#{BUNDLER_VERSION}" +version = "#{bundler.version}" if ARGV.first str = ARGV.first @@ -678,7 +676,7 @@ def build_bundler(default_bundle_without) yaml_include = File.expand_path("#{libyaml_dir}/include").shellescape yaml_lib = File.expand_path("#{libyaml_dir}/lib").shellescape pwd = Dir.pwd - bundler_path = "#{pwd}/#{slug_vendor_base}/gems/#{BUNDLER_GEM_PATH}/lib" + bundler_path = "#{pwd}/#{slug_vendor_base}/gems/#{bundler.dir_name}/lib" # we need to set BUNDLE_CONFIG and BUNDLE_GEMFILE for # codon since it uses bundler. env_vars = { @@ -1081,7 +1079,7 @@ def load_bundler_cache FileUtils.mkdir_p(heroku_metadata) @metadata.write(ruby_version_cache, full_ruby_version, false) @metadata.write(buildpack_version_cache, BUILDPACK_VERSION, false) - @metadata.write(bundler_version_cache, BUNDLER_VERSION, false) + @metadata.write(bundler_version_cache, bundler.version, false) @metadata.write(rubygems_version_cache, rubygems_version, false) @metadata.write(stack_cache, @stack, false) @metadata.save diff --git a/spec/helpers/fetcher_spec.rb b/spec/helpers/fetcher_spec.rb index 03fd6d8ad..78b12d964 100644 --- a/spec/helpers/fetcher_spec.rb +++ b/spec/helpers/fetcher_spec.rb @@ -1,15 +1,14 @@ require 'spec_helper' describe "Fetches" do - it "bundler" do Dir.mktmpdir do |dir| Dir.chdir(dir) do + fetcher = LanguagePack::Fetcher.new(LanguagePack::Base::VENDOR_URL) - fetcher.fetch_untar("#{LanguagePack::Ruby::BUNDLER_GEM_PATH}.tgz") + fetcher.fetch_untar("#{LanguagePack::Helpers::BundlerWrapper.new.dir_name}.tgz") expect(`ls bin`).to match("bundle") end end end end - From 3eed97936ea69bf22fb1da7cdd855522becfe2e7 Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 5 Feb 2019 13:00:14 -0600 Subject: [PATCH 2/5] Detect bundler version ## Problem 1 Bundler 2.x cannot be used with a Gemfile.lock that specifies bundler 1.x: ``` BUNDLED WITH 1.16.4 ``` If you try, then Bundler 2.x will look for an installed bundler 1.x version and try to use it. If no such version exists on the system then an error is thrown: ``` can't find gem bundler (>= 0.a) with executable bundle (Gem::GemNotFoundException) ``` - https://github.com/rubygems/rubygems/pull/2426 - https://github.com/rubygems/rubygems/pull/2515 ## Problem 2 Likewise a version of bundler 1.x cannot be used on a project where a `BUNDLED WITH` specifies 2.x: ``` $ bundle -v 1.17.3 $ cat Gemfile.lock | grep -A 1 BUNDLED WITH BUNDLED WITH 2.0.1 $ bundle Traceback (most recent call last): 2: from /Users/rschneeman/.gem/ruby/2.6.0/bin/bundle:23:in `
' 1: from /Users/rschneeman/.rubies/ruby-2.6.0/lib/ruby/2.6.0/rubygems.rb:302:in `activate_bin_path' /Users/rschneeman/.rubies/ruby-2.6.0/lib/ruby/2.6.0/rubygems.rb:283:in `find_spec_for_exe': Could not find 'bundler' (2.0.0) required by your /private/tmp/default_ruby/Gemfile.lock. (Gem::GemNotFoundException) To update to the latest version installed on your system, run `bundle update --bundler`. To install the missing version, run `gem install bundler:2.0.0` ``` This is due to a bug in Rubygems 3.x https://github.com/rubygems/rubygems/issues/2592 ## Proposed Solution This PR implements a solution where we have a different "blessed" version of bundler for each major version. The way this is implemented is to read in a Gemfile.lock and to read in the `BUNDLED WITH` value. Then the major version is pulled out and converted into a "blessed" version. This allows for multiple major versions of bundler to be used on Heroku without sacrificing stability. --- lib/language_pack/helpers/bundler_wrapper.rb | 79 ++++++++++++++++++-- spec/hatchet/bundler_spec.rb | 11 +++ spec/helpers/fetcher_spec.rb | 1 + 3 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 spec/hatchet/bundler_spec.rb diff --git a/lib/language_pack/helpers/bundler_wrapper.rb b/lib/language_pack/helpers/bundler_wrapper.rb index 2f2b7902b..8d24bd4f1 100644 --- a/lib/language_pack/helpers/bundler_wrapper.rb +++ b/lib/language_pack/helpers/bundler_wrapper.rb @@ -1,34 +1,74 @@ +# frozen_string_literal: true + require 'language_pack/fetcher' # This class is responsible for installing and maintaining a # reference to bundler. It contains access to bundler internals # that are used to introspect a project such as detecting presence # of gems and their versions. +# +# Example: +# +# bundler = LanguagePack::Helpers::BundlerWrapper.new +# bundler.install +# bundler.version => "1.15.2" +# bundler.dir_name => "bundler-1.15.2" +# bundler.has_gem?("railties") => true +# bundler.gem_version("railties") => "5.2.2" +# +# Also used to determine the version of Ruby that a project is using +# based on `bundle platform --ruby` +# +# bundler.ruby_version # => "ruby-2.5.1" +# class LanguagePack::Helpers::BundlerWrapper include LanguagePack::ShellHelpers + BLESSED_BUNDLER_VERSIONS = {} + BLESSED_BUNDLER_VERSIONS["1"] = "1.15.2" + BLESSED_BUNDLER_VERSIONS["2"] = "2.0.1" + private_constant :BLESSED_BUNDLER_VERSIONS + class GemfileParseError < BuildpackError def initialize(error) - msg = "There was an error parsing your Gemfile, we cannot continue\n" + msg = String.new("There was an error parsing your Gemfile, we cannot continue\n") msg << error super msg end end + class UnsupportedBundlerVersion < BuildpackError + def initialize(version_hash, major) + msg = String.new("Your Gemfile.lock indicates you need bundler `#{major}.x`\n") + msg << "which is not currently supported. You can deploy with bundler version:\n" + version_hash.keys.each do |v| + msg << " - `#{v}.x`\n" + end + msg << "\nTo use another version of bundler, update your `Gemfile.lock` to poin\n" + msg << "to as supported version. For example:\n" + msg << "\n" + msg << "```\n" + msg << "BUNDLED WITH\n" + msg << " #{version_hash["1"]}" + msg << "```" + super msg + end + end + attr_reader :bundler_path def initialize(options = {}) - @version = "1.15.2" - @bundler_tmp = Dir.mktmpdir + @bundler_tmp = Pathname.new(Dir.mktmpdir) @fetcher = options[:fetcher] || LanguagePack::Fetcher.new(LanguagePack::Base::VENDOR_URL) # coupling - @bundler_path = options[:bundler_path] || File.join(@bundler_tmp, "#{dir_name}") @gemfile_path = options[:gemfile_path] || Pathname.new("./Gemfile") - @bundler_tar = options[:bundler_tar] || "#{dir_name}.tgz" + @gemfile_lock_path = Pathname.new("#{@gemfile_path}.lock") + detect_bundler_version_and_dir_name! - @gemfile_lock_path = "#{@gemfile_path}.lock" + @bundler_path = options[:bundler_path] || @bundler_tmp.join(dir_name) + @bundler_tar = options[:bundler_tar] || "bundler/#{dir_name}.tgz" @orig_bundle_gemfile = ENV['BUNDLE_GEMFILE'] ENV['BUNDLE_GEMFILE'] = @gemfile_path.to_s - @path = Pathname.new "#{@bundler_path}/gems/#{dir_name}/lib" + @path = Pathname.new("#{@bundler_path}/gems/#{dir_name}/lib") end def install @@ -40,7 +80,7 @@ def install def clean ENV['BUNDLE_GEMFILE'] = @orig_bundle_gemfile - FileUtils.remove_entry_secure(@bundler_tmp) if Dir.exist?(@bundler_tmp) + @bundler_tmp.rmtree if @bundler_tmp.directory? if version == "1.7.12" # Hack to cleanup after pre 1.8 versions of bundler. See https://github.com/bundler/bundler/pull/3277/ @@ -134,4 +174,27 @@ def parse_gemfile_lock Bundler::LockfileParser.new(gemfile_contents) end end + + def major_bundler_version + # https://rubular.com/r/uuRpai9IheL68d + bundler_version_match = @gemfile_lock_path.read.match(/^BUNDLED WITH$(\r?\n) (?\d*)\.\d*\.\d*/m) + + if bundler_version_match + bundler_version_match[:major] + else + "1" + end + end + + # You cannot use Bundler 2.x with a Gemfile.lock that points to a 1.x bundler + # version. The solution here is to read in the value set in the Gemfile.lock + # and download the "blessed" version with the same major version. + def detect_bundler_version_and_dir_name! + major = major_bundler_version + if BLESSED_BUNDLER_VERSIONS.key?(major) + @version = BLESSED_BUNDLER_VERSIONS[major] + else + raise UnsupportedBundlerVersion.new(BLESSED_BUNDLER_VERSIONS, major) + end + end end diff --git a/spec/hatchet/bundler_spec.rb b/spec/hatchet/bundler_spec.rb new file mode 100644 index 000000000..a774c6d14 --- /dev/null +++ b/spec/hatchet/bundler_spec.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +describe "Bundler" do + it "deploys with version 2.x" do + before_deploy = -> { run!(%Q{printf "\nBUNDLED WITH\n 2.0.1\n" >> Gemfile.lock}) } + + Hatchet::Runner.new("default_ruby", before_deploy: before_deploy).deploy do |app| + expect(app.output).to match("Installing dependencies using bundler 2.") + end + end +end diff --git a/spec/helpers/fetcher_spec.rb b/spec/helpers/fetcher_spec.rb index 78b12d964..17d9b1368 100644 --- a/spec/helpers/fetcher_spec.rb +++ b/spec/helpers/fetcher_spec.rb @@ -4,6 +4,7 @@ it "bundler" do Dir.mktmpdir do |dir| Dir.chdir(dir) do + FileUtils.touch("Gemfile.lock") fetcher = LanguagePack::Fetcher.new(LanguagePack::Base::VENDOR_URL) fetcher.fetch_untar("#{LanguagePack::Helpers::BundlerWrapper.new.dir_name}.tgz") From 80f3d88fc380273d6d16d9230ff01840480e285d Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 5 Feb 2019 16:23:03 -0600 Subject: [PATCH 3/5] Fix a flappy test based on the bundler wrapper Usually this would take a LONG time as the test suite when run serially takes about an hour. The other thing that is tough about this, is it only fails on CI and not locally. On a hunch I guessed the failing behavior came only from `spec/helpers` which is all unit tested and therefore pretty fast. This allowed me to actually find the failing order. Via `$ heroku ci:debug` First I ran: ``` $ bundle exec rspec spec/helpers --bisect --seed 1 ``` The `--seed` argument is used because otherwise the order is randomized on every run. It doesn't matter what value is used as long as it's consistent. This produce a "minimal failing example" of running: ``` $ rspec './spec/helpers/fetcher_spec.rb[1:1]' './spec/helpers/rails_runner_spec.rb[1:3,1:5]' './spec/helpers/rake_runner_spec.rb[1:1,1:2]' --seed 1 ``` Sure enough this command fails 100% of the time. I knew the culprit had to be mutating something global. Previously I've been focused on files on disk but considering every example is now run inside of it's own temp directory then it is likely something else. The other culprit could be an environment variable. This lead me to look into the bundler wrapper which sure enoug mutates the env var `BUNDLE_GEMFILE`. --- lib/language_pack/helpers/bundler_wrapper.rb | 22 +++++++++++++++----- spec/helpers/rails_runner_spec.rb | 7 +++++-- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/lib/language_pack/helpers/bundler_wrapper.rb b/lib/language_pack/helpers/bundler_wrapper.rb index 8d24bd4f1..cc6aecd72 100644 --- a/lib/language_pack/helpers/bundler_wrapper.rb +++ b/lib/language_pack/helpers/bundler_wrapper.rb @@ -15,11 +15,22 @@ # bundler.dir_name => "bundler-1.15.2" # bundler.has_gem?("railties") => true # bundler.gem_version("railties") => "5.2.2" +# bundler.clean # # Also used to determine the version of Ruby that a project is using # based on `bundle platform --ruby` # # bundler.ruby_version # => "ruby-2.5.1" +# bundler.clean +# +# IMPORTANT: Calling `BundlerWrapper#install` on this class mutates the environment variable +# ENV['BUNDLE_GEMFILE']. If you're calling in a test context (or anything outside) +# of an isolated dyno, you must call `BundlerWrapper#clean`. To reset the environment +# variable: +# +# bundler = LanguagePack::Helpers::BundlerWrapper.new +# bundler.install +# bundler.clean # <========== IMPORTANT ============= # class LanguagePack::Helpers::BundlerWrapper include LanguagePack::ShellHelpers @@ -44,13 +55,13 @@ def initialize(version_hash, major) version_hash.keys.each do |v| msg << " - `#{v}.x`\n" end - msg << "\nTo use another version of bundler, update your `Gemfile.lock` to poin\n" - msg << "to as supported version. For example:\n" + msg << "\nTo use another version of bundler, update your `Gemfile.lock` to point\n" + msg << "to a supported version. For example:\n" msg << "\n" msg << "```\n" msg << "BUNDLED WITH\n" - msg << " #{version_hash["1"]}" - msg << "```" + msg << " #{version_hash["1"]}\n" + msg << "```\n" super msg end end @@ -67,11 +78,12 @@ def initialize(options = {}) @bundler_path = options[:bundler_path] || @bundler_tmp.join(dir_name) @bundler_tar = options[:bundler_tar] || "bundler/#{dir_name}.tgz" @orig_bundle_gemfile = ENV['BUNDLE_GEMFILE'] - ENV['BUNDLE_GEMFILE'] = @gemfile_path.to_s @path = Pathname.new("#{@bundler_path}/gems/#{dir_name}/lib") end def install + ENV['BUNDLE_GEMFILE'] = @gemfile_path.to_s + fetch_bundler $LOAD_PATH << @path require "bundler" diff --git a/spec/helpers/rails_runner_spec.rb b/spec/helpers/rails_runner_spec.rb index b8ea57f09..7bae1c922 100644 --- a/spec/helpers/rails_runner_spec.rb +++ b/spec/helpers/rails_runner_spec.rb @@ -2,12 +2,16 @@ describe "Rails Runner" do around(:each) do |test| + original_path = ENV["PATH"] + ENV["PATH"] = "./bin/:#{ENV['PATH']}" + Dir.mktmpdir do |tmpdir| - @tmpdir = tmpdir Dir.chdir(tmpdir) do test.run end end + ensure + ENV["PATH"] = original_path if original_path end it "config objects build propperly formatted commands" do @@ -119,7 +123,6 @@ def to_s FileUtils.mkdir("bin") File.open("bin/rails", "w") { |f| f << executable_contents } File.chmod(0777, "bin/rails") - ENV["PATH"] = "./bin/:#{ENV['PATH']}" unless ENV["PATH"].include?("./bin:") # BUILDPACK_LOG_FILE support for logging FileUtils.mkdir("tmp") From c1fd23ee8e37fc82766e141aa7cefbdab9b66a0a Mon Sep 17 00:00:00 2001 From: schneems Date: Wed, 6 Feb 2019 20:05:04 -0600 Subject: [PATCH 4/5] Ensure at least 1 digit in version --- lib/language_pack/helpers/bundler_wrapper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/language_pack/helpers/bundler_wrapper.rb b/lib/language_pack/helpers/bundler_wrapper.rb index cc6aecd72..51ae1c815 100644 --- a/lib/language_pack/helpers/bundler_wrapper.rb +++ b/lib/language_pack/helpers/bundler_wrapper.rb @@ -188,8 +188,8 @@ def parse_gemfile_lock end def major_bundler_version - # https://rubular.com/r/uuRpai9IheL68d - bundler_version_match = @gemfile_lock_path.read.match(/^BUNDLED WITH$(\r?\n) (?\d*)\.\d*\.\d*/m) + # https://rubular.com/r/jt9yj0aY7fU3hD + bundler_version_match = @gemfile_lock_path.read.match(/^BUNDLED WITH$(\r?\n) (?\d+)\.\d+\.\d+/m) if bundler_version_match bundler_version_match[:major] From 7108542b400297d30fac2d9bc9ab8833c04ef110 Mon Sep 17 00:00:00 2001 From: schneems Date: Thu, 7 Feb 2019 12:12:37 -0600 Subject: [PATCH 5/5] Remove code that will never get called Since we control the version, this code will never execute unless we regress to a REALLY old version. I'm deleting it. --- lib/language_pack/helpers/bundler_wrapper.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/language_pack/helpers/bundler_wrapper.rb b/lib/language_pack/helpers/bundler_wrapper.rb index 51ae1c815..551416d2b 100644 --- a/lib/language_pack/helpers/bundler_wrapper.rb +++ b/lib/language_pack/helpers/bundler_wrapper.rb @@ -93,13 +93,6 @@ def install def clean ENV['BUNDLE_GEMFILE'] = @orig_bundle_gemfile @bundler_tmp.rmtree if @bundler_tmp.directory? - - if version == "1.7.12" - # Hack to cleanup after pre 1.8 versions of bundler. See https://github.com/bundler/bundler/pull/3277/ - Dir["#{Dir.tmpdir}/bundler*"].each do |dir| - FileUtils.remove_entry_secure(dir) if Dir.exist?(dir) && File.stat(dir).writable? - end - end end def has_gem?(name)