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

[close #943] Use Bundler env vars for config #1039

Merged
merged 2 commits into from
Aug 18, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Main (unreleased)

* 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)

Expand Down
6 changes: 0 additions & 6 deletions lib/language_pack/rails4.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
122 changes: 75 additions & 47 deletions lib/language_pack/ruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -137,16 +142,20 @@ 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"
gem_layer.validate! do |metadata|
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"))
Expand All @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -393,6 +403,11 @@ 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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we display all the env vars when running the command, do we want to make BUNDLE_DEPLOYMENT configurable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's a reason to deploy without --deployment mode unless they're using a windows Gemfile.lock.

end
end

Expand Down Expand Up @@ -426,6 +441,11 @@ 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_DEPLOYMENT", ENV["BUNDLE_DEPLOYMENT"], layer if ENV["BUNDLE_DEPLOYMENT"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't setup_language_pack_environment always set this to be 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's a windows Gemfile.lock then we delete it and remove the deployment mode https://github.com/heroku/heroku-buildpack-ruby/pull/1039/files#diff-3fc66e13e389ff4d15c7ca3ddb86464eR886

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yeah, i wonder if it's worth just adding a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a comment. Thinking about it a bit more, it's unclear this behavior is needed for the export and profile.d files i.e. once we've already run bundle install then there's not much benefit in keeping the deployment mode off. The PR as is preserves existing behavior, so let's not muck it up for now. Just keep in mind we could likely make that change if we wanted.

end
end

Expand Down Expand Up @@ -461,6 +481,11 @@ 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_DEPLOYMENT", ENV["BUNDLE_DEPLOYMENT"] if ENV["BUNDLE_DEPLOYMENT"]
end
end

Expand Down Expand Up @@ -839,44 +864,47 @@ 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"]
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|
Expand Down Expand Up @@ -918,9 +946,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
Expand All @@ -934,28 +962,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

Expand Down
11 changes: 8 additions & 3 deletions lib/language_pack/test/ruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions spec/hatchet/node_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
Expand Down
4 changes: 3 additions & 1 deletion spec/hatchet/ruby_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down