From 655e246657e329db524177d410e4ae4aaf8cc141 Mon Sep 17 00:00:00 2001 From: Richard Schneeman Date: Thu, 3 Sep 2020 10:41:31 -0500 Subject: [PATCH] Fix gem installation location bug with Bundler env vars (#1052) * Revert "Revert "[close #943] Use Bundler env vars for config (#1039)" (#1047)" This reverts commit 858adcbe3a53997192be9f12aa475c109fc490e9. * Fix gem installation location bug with Bundler env vars When the move from bundler flags to bundler env vars was first merged in an issue was reported #1046 , this lead to an investigation and bug report to bundler https://github.com/rubygems/rubygems/issues/3890 which lead to some older issues/prs: - rubygems/bundler#3552 - rubygems/bundler#6628 The issue is that global configuration and local configuration results in different behavior when installing and using some features in bundler, notedly the ability to specify install path. Due to this change, when the switch to bundler environment variables happened, it caused the size of some applications' slugs to increase dramatically, this was because the gems were essentially being installed twice. The issue appears to not be in Bundler 2.1.4 so we're bumping the version for 2.x series. In the 1.x series an environment variable `BUNDLE_GLOBAL_PATH_APPENDS_RUBY_SCOPE=1` can be used to force the desired behavior. # This is the commit message #2: Co-authored-by: Ed Morley <501702+edmorley@users.noreply.github.com> Co-authored-by: Ed Morley <501702+edmorley@users.noreply.github.com> --- CHANGELOG.md | 2 + changelogs/unreleased/bundler-214.md | 3 + changelogs/unreleased/bundler-env-vars.md | 20 +++ lib/language_pack/helpers/bundler_wrapper.rb | 9 +- lib/language_pack/rails4.rb | 6 - lib/language_pack/ruby.rb | 126 ++++++++++++------- lib/language_pack/test/ruby.rb | 11 +- spec/hatchet/bundler_spec.rb | 59 +++++++-- spec/hatchet/node_spec.rb | 7 +- spec/hatchet/ruby_spec.rb | 4 +- 10 files changed, 178 insertions(+), 69 deletions(-) create mode 100644 changelogs/unreleased/bundler-214.md create mode 100644 changelogs/unreleased/bundler-env-vars.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d724fc52..e794a512c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Main (unreleased) +* Bundler 2.x is now 2.1.4 (https://github.com/heroku/heroku-buildpack-ruby/pull/1052) +* Persistent bundler config is now being set using the `BUNDLE_*` env vars (https://github.com/heroku/heroku-buildpack-ruby/pull/1039) * Rake task "assets:clean" will not get called if it does not exist (https://github.com/heroku/heroku-buildpack-ruby/pull/1018) * CNB: Fix the `gems` layer not being made accessible by subsequent buildpacks (https://github.com/heroku/heroku-buildpack-ruby/pull/1033) diff --git a/changelogs/unreleased/bundler-214.md b/changelogs/unreleased/bundler-214.md new file mode 100644 index 000000000..4b5afc212 --- /dev/null +++ b/changelogs/unreleased/bundler-214.md @@ -0,0 +1,3 @@ +## Ruby apps with Bundler 2.x now receive version 2.1.4 + +The [Ruby Buildpack](https://devcenter.heroku.com/articles/ruby-support#libraries) now includes Bundler 2.1.4. Applications specifying Bundler 2.x in their `Gemfile.lock` will now receive bundler: 2.1.4. diff --git a/changelogs/unreleased/bundler-env-vars.md b/changelogs/unreleased/bundler-env-vars.md new file mode 100644 index 000000000..df51681c6 --- /dev/null +++ b/changelogs/unreleased/bundler-env-vars.md @@ -0,0 +1,20 @@ +## Ruby applications now configure bundler with environment variables instead of flags + +Previously the Ruby buildpack ran bundler installation with flags: + +``` +Running: bundle install --without development:test --path vendor/bundle --binstubs vendor/bundle/bin -j4 --deployment +[DEPRECATED] The `--deployment` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set deployment 'true'`, and stop using this flag +[DEPRECATED] The `--path` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set path 'vendor/bundle'`, and stop using this flag +[DEPRECATED] The `--without` flag is deprecated because it relies on being remembered across bundler invocations, which bundler will no longer do in future versions. Instead please use `bundle config set without 'development:test'`, and stop using this flag +[DEPRECATED] The --binstubs option will be removed in favor of `bundle binstubs` +``` + +In order to remove deprecations from Bundler 2.x, Ruby applications now run bundler installation with environment variables instead: + +``` +Running: BUNDLE_WITHOUT=development:test BUNDLE_PATH=vendor/bundle BUNDLE_BIN=vendor/bundle/bin BUNDLE_DEPLOYMENT=1 bundle install -j4 +``` + +This behavior is documented in the [Heroku Ruby Support page](https://devcenter.heroku.com/articles/ruby-support). + diff --git a/lib/language_pack/helpers/bundler_wrapper.rb b/lib/language_pack/helpers/bundler_wrapper.rb index 7a4e46b37..9ca42c9ba 100644 --- a/lib/language_pack/helpers/bundler_wrapper.rb +++ b/lib/language_pack/helpers/bundler_wrapper.rb @@ -37,7 +37,7 @@ class LanguagePack::Helpers::BundlerWrapper BLESSED_BUNDLER_VERSIONS = {} BLESSED_BUNDLER_VERSIONS["1"] = "1.17.3" - BLESSED_BUNDLER_VERSIONS["2"] = "2.0.2" + BLESSED_BUNDLER_VERSIONS["2"] = "2.1.4" BUNDLED_WITH_REGEX = /^BUNDLED WITH$(\r?\n) (?\d+)\.\d+\.\d+/m class GemfileParseError < BuildpackError @@ -161,6 +161,13 @@ def lockfile_parser @lockfile_parser ||= parse_gemfile_lock end + # Some bundler versions have different behavior + # if config is global versus local. These versions need + # the environment variable BUNDLE_GLOBAL_PATH_APPENDS_RUBY_SCOPE=1 + def needs_ruby_global_append_path? + Gem::Version.new(@version) < Gem::Version.new("2.1.4") + end + private def fetch_bundler instrument 'fetch_bundler' do diff --git a/lib/language_pack/rails4.rb b/lib/language_pack/rails4.rb index 703343a6f..86e11b6fd 100644 --- a/lib/language_pack/rails4.rb +++ b/lib/language_pack/rails4.rb @@ -30,12 +30,6 @@ def default_process_types end end - def build_bundler(default_bundle_without) - instrument "rails4.build_bundler" do - super - end - end - def compile instrument "rails4.compile" do super diff --git a/lib/language_pack/ruby.rb b/lib/language_pack/ruby.rb index 10284d5c3..0583c0a0d 100644 --- a/lib/language_pack/ruby.rb +++ b/lib/language_pack/ruby.rb @@ -100,12 +100,16 @@ def compile warn_bad_binstubs install_ruby(slug_vendor_ruby, build_ruby_path) install_jvm - setup_language_pack_environment(ruby_layer_path: File.expand_path("."), gem_layer_path: File.expand_path(".")) - setup_profiled(ruby_layer_path: "$HOME", gem_layer_path: "$HOME") # $HOME is set to /app at run time + setup_language_pack_environment( + ruby_layer_path: File.expand_path("."), + gem_layer_path: File.expand_path("."), + bundle_path: "vendor/bundle", + bundle_default_without: "development:test" + ) allow_git do install_bundler_in_app(slug_vendor_base) load_bundler_cache - build_bundler(bundle_path: "vendor/bundle", default_bundle_without: "development:test") + build_bundler post_bundler create_database_yml install_binaries @@ -114,6 +118,7 @@ def compile config_detect best_practice_warnings warn_outdated_ruby + setup_profiled(ruby_layer_path: "$HOME", gem_layer_path: "$HOME") # $HOME is set to /app at run time setup_export cleanup super @@ -137,8 +142,12 @@ def build ruby_layer.write gem_layer = Layer.new(@layer_dir, "gems", launch: true, cache: true, build: true) - setup_language_pack_environment(ruby_layer_path: ruby_layer.path, gem_layer_path: gem_layer.path) - setup_profiled(ruby_layer_path: ruby_layer.path, gem_layer_path: gem_layer.path) + setup_language_pack_environment( + ruby_layer_path: ruby_layer.path, + gem_layer_path: gem_layer.path, + bundle_path: "#{gem_layer.path}/vendor/bundle", + bundle_default_without: "development:test" + ) allow_git do # TODO install bundler in separate layer topic "Loading Bundler Cache" @@ -146,7 +155,7 @@ def build valid_bundler_cache?(gem_layer.path, gem_layer.metadata) end install_bundler_in_app("#{gem_layer.path}/#{slug_vendor_base}") - build_bundler(bundle_path: "#{gem_layer.path}/vendor/bundle", default_bundle_without: "development:test") + build_bundler # TODO post_bundler might need to be done in a new layer bundler.clean gem_layer.metadata[:gems] = Digest::SHA2.hexdigest(File.read("Gemfile.lock")) @@ -161,6 +170,7 @@ def build install_binaries run_assets_precompile_rake_task end + setup_profiled(ruby_layer_path: ruby_layer.path, gem_layer_path: gem_layer.path) setup_export(gem_layer) config_detect best_practice_warnings @@ -345,7 +355,7 @@ def default_java_mem end # sets up the environment variables for the build process - def setup_language_pack_environment(ruby_layer_path:, gem_layer_path:) + def setup_language_pack_environment(ruby_layer_path:, gem_layer_path:, bundle_path:, bundle_default_without:) instrument 'ruby.setup_language_pack_environment' do if ruby_version.jruby? ENV["PATH"] += ":bin" @@ -393,6 +403,12 @@ def setup_language_pack_environment(ruby_layer_path:, gem_layer_path:) paths << ENV["PATH"] ENV["PATH"] = paths.join(":") + + ENV["BUNDLE_WITHOUT"] = env("BUNDLE_WITHOUT") || bundle_default_without + ENV["BUNDLE_PATH"] = bundle_path + ENV["BUNDLE_BIN"] = bundler_binstubs_path + ENV["BUNDLE_DEPLOYMENT"] = "1" + ENV["BUNDLE_GLOBAL_PATH_APPENDS_RUBY_SCOPE"] = "1" if bundler.needs_ruby_global_append_path? end end @@ -426,6 +442,12 @@ def setup_export(layer = nil) set_export_default "JAVA_OPTS", default_java_opts set_export_default "JRUBY_OPTS", default_jruby_opts end + + set_export_default "BUNDLE_PATH", ENV["BUNDLE_PATH"], layer + set_export_default "BUNDLE_WITHOUT", ENV["BUNDLE_WITHOUT"], layer + set_export_default "BUNDLE_BIN", ENV["BUNDLE_BIN"], layer + set_export_default "BUNDLE_GLOBAL_PATH_APPENDS_RUBY_SCOPE", ENV["BUNDLE_GLOBAL_PATH_APPENDS_RUBY_SCOPE"], layer if bundler.needs_ruby_global_append_path? + set_export_default "BUNDLE_DEPLOYMENT", ENV["BUNDLE_DEPLOYMENT"], layer if ENV["BUNDLE_DEPLOYMENT"] # Unset on windows since we delete the Gemfile.lock end end @@ -461,6 +483,12 @@ def setup_profiled(ruby_layer_path: , gem_layer_path: ) set_env_default "JAVA_OPTS", default_java_opts set_env_default "JRUBY_OPTS", default_jruby_opts end + + set_env_default "BUNDLE_PATH", ENV["BUNDLE_PATH"] + set_env_default "BUNDLE_WITHOUT", ENV["BUNDLE_WITHOUT"] + set_env_default "BUNDLE_BIN", ENV["BUNDLE_BIN"] + set_env_default "BUNDLE_GLOBAL_PATH_APPENDS_RUBY_SCOPE", ENV["BUNDLE_GLOBAL_PATH_APPENDS_RUBY_SCOPE"] if bundler.needs_ruby_global_append_path? + set_env_default "BUNDLE_DEPLOYMENT", ENV["BUNDLE_DEPLOYMENT"] if ENV["BUNDLE_DEPLOYMENT"] # Unset on windows since we delete the Gemfile.lock end end @@ -839,44 +867,48 @@ def write_bundler_shim(path) end # runs bundler to install the dependencies - def build_bundler(bundle_path:, default_bundle_without:) + def build_bundler instrument 'ruby.build_bundler' do log("bundle") do - bundle_without = env("BUNDLE_WITHOUT") || default_bundle_without - bundle_bin = "bundle" - bundle_command = "#{bundle_bin} install --without #{bundle_without} --path #{bundle_path} --binstubs #{bundler_binstubs_path}" - bundle_command << " -j4" - if File.exist?("#{Dir.pwd}/.bundle/config") - warn(<<-WARNING, inline: true) -You have the `.bundle/config` file checked into your repository - It contains local state like the location of the installed bundle - as well as configured git local gems, and other settings that should -not be shared between multiple checkouts of a single repo. Please -remove the `.bundle/` folder from your repo and add it to your `.gitignore` file. -https://devcenter.heroku.com/articles/bundler-configuration -WARNING + warn(<<~WARNING, inline: true) + You have the `.bundle/config` file checked into your repository + It contains local state like the location of the installed bundle + as well as configured git local gems, and other settings that should + not be shared between multiple checkouts of a single repo. Please + remove the `.bundle/` folder from your repo and add it to your `.gitignore` file. + + https://devcenter.heroku.com/articles/bundler-configuration + WARNING end if bundler.windows_gemfile_lock? - warn(<<-WARNING, inline: true) -Removing `Gemfile.lock` because it was generated on Windows. -Bundler will do a full resolve so native gems are handled properly. -This may result in unexpected gem versions being used in your app. -In rare occasions Bundler may not be able to resolve your dependencies at all. -https://devcenter.heroku.com/articles/bundler-windows-gemfile -WARNING - log("bundle", "has_windows_gemfile_lock") + File.unlink("Gemfile.lock") - else - # using --deployment is preferred if we can - bundle_command += " --deployment" + ENV.delete("BUNDLE_DEPLOYMENT") + + warn(<<~WARNING, inline: true) + Removing `Gemfile.lock` because it was generated on Windows. + Bundler will do a full resolve so native gems are handled properly. + This may result in unexpected gem versions being used in your app. + In rare occasions Bundler may not be able to resolve your dependencies at all. + + https://devcenter.heroku.com/articles/bundler-windows-gemfile + WARNING end + bundle_command = String.new("") + bundle_command << "BUNDLE_WITHOUT=#{ENV["BUNDLE_WITHOUT"]} " + bundle_command << "BUNDLE_PATH=#{ENV["BUNDLE_PATH"]} " + bundle_command << "BUNDLE_BIN=#{ENV["BUNDLE_BIN"]} " + bundle_command << "BUNDLE_DEPLOYMENT=#{ENV["BUNDLE_DEPLOYMENT"]} " if ENV["BUNDLE_DEPLOYMENT"] # Unset on windows since we delete the Gemfile.lock + bundle_command << "BUNDLE_GLOBAL_PATH_APPENDS_RUBY_SCOPE=#{ENV["BUNDLE_GLOBAL_PATH_APPENDS_RUBY_SCOPE"]} " if bundler.needs_ruby_global_append_path? + bundle_command << "bundle install -j4" + topic("Installing dependencies using bundler #{bundler.version}") - bundler_output = "" + bundler_output = String.new("") bundle_time = nil env_vars = {} Dir.mktmpdir("libyaml-") do |tmpdir| @@ -918,9 +950,9 @@ def build_bundler(bundle_path:, default_bundle_without:) instrument "ruby.bundle_clean" do # Only show bundle clean output when not using default cache if load_default_cache? - run("#{bundle_bin} clean > /dev/null", user_env: true, env: env_vars) + run("bundle clean > /dev/null", user_env: true, env: env_vars) else - pipe("#{bundle_bin} clean", out: "2> /dev/null", user_env: true, env: env_vars) + pipe("bundle clean", out: "2> /dev/null", user_env: true, env: env_vars) end end @bundler_cache.store @@ -934,28 +966,28 @@ def build_bundler(bundle_path:, default_bundle_without:) puts "Bundler Output: #{bundler_output}" if bundler_output.match(/An error occurred while installing sqlite3/) mcount "fail.sqlite3" - error_message += <<-ERROR + error_message += <<~ERROR -Detected sqlite3 gem which is not supported on Heroku: -https://devcenter.heroku.com/articles/sqlite3 + Detected sqlite3 gem which is not supported on Heroku: + https://devcenter.heroku.com/articles/sqlite3 ERROR end if bundler_output.match(/but your Gemfile specified/) mcount "fail.ruby_version_mismatch" - error_message += <<-ERROR + error_message += <<~ERROR -Detected a mismatch between your Ruby version installed and -Ruby version specified in Gemfile or Gemfile.lock. You can -correct this by running: + Detected a mismatch between your Ruby version installed and + Ruby version specified in Gemfile or Gemfile.lock. You can + correct this by running: - $ bundle update --ruby - $ git add Gemfile.lock - $ git commit -m "update ruby version" + $ bundle update --ruby + $ git add Gemfile.lock + $ git commit -m "update ruby version" -If this does not solve the issue please see this documentation: + If this does not solve the issue please see this documentation: -https://devcenter.heroku.com/articles/ruby-versions#your-ruby-version-is-x-but-your-gemfile-specified-y + https://devcenter.heroku.com/articles/ruby-versions#your-ruby-version-is-x-but-your-gemfile-specified-y ERROR end diff --git a/lib/language_pack/test/ruby.rb b/lib/language_pack/test/ruby.rb index 39ff78d31..19cb97fd2 100644 --- a/lib/language_pack/test/ruby.rb +++ b/lib/language_pack/test/ruby.rb @@ -8,18 +8,23 @@ def compile warn_bad_binstubs install_ruby(slug_vendor_ruby, build_ruby_path) install_jvm - setup_language_pack_environment(ruby_layer_path: File.expand_path("."), gem_layer_path: File.expand_path(".")) + setup_language_pack_environment( + ruby_layer_path: File.expand_path("."), + gem_layer_path: File.expand_path("."), + bundle_path: "vendor/bundle", + bundle_default_without: "development" + ) setup_export - setup_profiled(ruby_layer_path: "$HOME", gem_layer_path: "$HOME") # $HOME is set to /app at run time allow_git do install_bundler_in_app(slug_vendor_base) load_bundler_cache - build_bundler(bundle_path: "vendor/bundle", default_bundle_without: "development") + build_bundler post_bundler create_database_yml install_binaries prepare_tests end + setup_profiled(ruby_layer_path: "$HOME", gem_layer_path: "$HOME") # $HOME is set to /app at run time super end end diff --git a/spec/hatchet/bundler_spec.rb b/spec/hatchet/bundler_spec.rb index 469806e28..c9e1f554f 100644 --- a/spec/hatchet/bundler_spec.rb +++ b/spec/hatchet/bundler_spec.rb @@ -1,18 +1,59 @@ require 'spec_helper' describe "Bundler" do - it "deploys with version 2.x" do - before_deploy = Proc.new do - run!(%Q{echo "ruby '2.5.7'" >> Gemfile}) - run!(%Q{printf "\nBUNDLED WITH\n 2.0.1\n" >> Gemfile.lock}) + it "deploys with version 2.x with Ruby 2.5" do + ruby_version = "2.5.7" + abi_version = ruby_version.dup + abi_version[-1] = "0" # turn 2.5.7 into 2.5.0 + pending("Must enable HATCHET_EXPENSIVE_MODE") unless ENV["HATCHET_EXPENSIVE_MODE"] + + Hatchet::Runner.new("default_ruby", run_multi: true).tap do |app| + app.before_deploy do + run!(%Q{echo "ruby '#{ruby_version}'" >> Gemfile}) + run!(%Q{printf "\nBUNDLED WITH\n 2.0.1\n" >> Gemfile.lock}) + end + app.deploy do + expect(app.output).to match("Installing dependencies using bundler 2.") + expect(app.output).to_not match("BUNDLE_GLOBAL_PATH_APPENDS_RUBY_SCOPE=1") + + # Double deploy problem with Ruby 2.5.5 + app.commit! + app.push! + + app.run_multi("ls vendor/bundle/ruby/#{abi_version}/gems") do |ls_output| + expect(ls_output).to match("rake-") + end + + app.run_multi("which -a rake") do |which_rake| + expect(which_rake).to include("/app/vendor/bundle/bin/rake") + expect(which_rake).to include("/app/vendor/bundle/ruby/#{abi_version}/bin/rake") + end + end end + end + + it "deploys with version 1.x" do + abi_version = LanguagePack::RubyVersion::DEFAULT_VERSION_NUMBER + abi_version[-1] = "0" # turn 2.6.6 into 2.6.0 + pending("Must enable HATCHET_EXPENSIVE_MODE") unless ENV["HATCHET_EXPENSIVE_MODE"] + + Hatchet::Runner.new("default_ruby", run_multi: true).tap do |app| + app.before_deploy do + run!(%Q{printf "\nBUNDLED WITH\n 1.0.1\n" >> Gemfile.lock}) + end + app.deploy do + expect(app.output).to match("Installing dependencies using bundler 1.") + expect(app.output).to match("BUNDLE_GLOBAL_PATH_APPENDS_RUBY_SCOPE=1") - Hatchet::Runner.new("default_ruby", before_deploy: before_deploy).deploy do |app| - expect(app.output).to match("Installing dependencies using bundler 2.") + app.run_multi("ls vendor/bundle/ruby/#{abi_version}/gems") do |ls_output| + expect(ls_output).to match("rake-") + end - # Double deploy problem with Ruby 2.5.5 - run!(%Q{git commit --allow-empty -m 'Deploying again'}) - app.push! + app.run_multi("which -a rake") do |which_rake| + expect(which_rake).to include("/app/vendor/bundle/bin/rake") + expect(which_rake).to include("/app/vendor/bundle/ruby/#{abi_version}/bin/rake") + end + end end end end diff --git a/spec/hatchet/node_spec.rb b/spec/hatchet/node_spec.rb index 3461adc6e..7d58b22d2 100644 --- a/spec/hatchet/node_spec.rb +++ b/spec/hatchet/node_spec.rb @@ -6,7 +6,9 @@ :default, "https://github.com/sharpstone/force_absolute_paths_buildpack" ] - Hatchet::Runner.new("minimal_webpacker", buildpacks: buildpacks).deploy do |app, heroku| + config = {FORCE_ABSOLUTE_PATHS_BUILDPACK_IGNORE_PATHS: "BUNDLE_PATH"} + + Hatchet::Runner.new("minimal_webpacker", buildpacks: buildpacks, config: config).deploy do |app, heroku| # https://rubular.com/r/4bkL8fYFTQwt0Q expect(app.output).to match(/vendor\/yarn-v\d+\.\d+\.\d+\/bin\/yarn is the yarn directory/) expect(app.output).to_not include(".heroku/yarn/bin/yarn is the yarn directory") @@ -25,8 +27,9 @@ :default, "https://github.com/sharpstone/force_absolute_paths_buildpack" ] + config = {FORCE_ABSOLUTE_PATHS_BUILDPACK_IGNORE_PATHS: "BUNDLE_PATH"} - Hatchet::Runner.new("minimal_webpacker", buildpacks: buildpacks).deploy do |app, heroku| + Hatchet::Runner.new("minimal_webpacker", buildpacks: buildpacks, config: config).deploy do |app, heroku| expect(app.output).to include("yarn install") expect(app.output).to include(".heroku/yarn/bin/yarn is the yarn directory") expect(app.output).to include(".heroku/node/bin/node is the node directory") diff --git a/spec/hatchet/ruby_spec.rb b/spec/hatchet/ruby_spec.rb index 3d543a3c5..1fc7b4f74 100644 --- a/spec/hatchet/ruby_spec.rb +++ b/spec/hatchet/ruby_spec.rb @@ -113,7 +113,9 @@ :default, "https://github.com/sharpstone/force_absolute_paths_buildpack" ] - Hatchet::Runner.new('cd_ruby', stack: DEFAULT_STACK, buildpacks: buildpacks).deploy do |app| + config = {FORCE_ABSOLUTE_PATHS_BUILDPACK_IGNORE_PATHS: "BUNDLE_PATH"} + + Hatchet::Runner.new('cd_ruby', stack: DEFAULT_STACK, buildpacks: buildpacks, config: config).deploy do |app| expect(app.output).to match("cd version ruby 2.5.1") expect(app.run("which ruby").chomp).to eq("/app/bin/ruby")