Commit 27848b7
authored
Install Ruby before bundler (#1684)
* Revert "Revert v334 (#1682)"
This reverts commit 57462aa.
* Changelog
* Inject environment name
The modification in #1676 accidentally saw "test" replaced with "production" as the inheritance-based logic was no longer being invoked.
This change threads the value ("production" or "test") used through the lifecycle of the buildpack.
* Don't export bootstrap Ruby on PATH (And Fix "herokuish" CI test case)
Previously the Ruby buildpack worked like this:
- Install a "bootstrap" version of Ruby
- Use bootstrap ruby to execute the buildpack code (written in ruby)
- Download customer's bundler version
- Run that bundler version with bootstrap ruby to determine customer's ruby version `bundle platform --ruby`
- ...
With that flow, it's required that the Ruby program must be able to re-exec itself since it must have a Ruby version to run and a ruby version to run `bundle platform --ruby`.
The code was changed such that it now looks like this:
- Install a "bootstrap" version of Ruby
- Use bootstrap ruby to execute the buildpack code (written in ruby)
- Statically determine Ruby version from the Gemfile.lock
- Install customer's Ruby version
- Install customer's bundler version
- ...
With this flow, we do not ever need to re-exec the bootstrap ruby version. This lets us remove exporting it to the PATH which has caused a LOT of problems over the years.
As mentioned it also fixes the "herokuish" bug that was introduced in v334 (then rolled back). I was able to reproduce the bug via a test added in #1687. The test uses a different version of Ruby than the "bootstrap" version. The problem was this error:
```
Running: which -a ruby
/app/bin/ruby
/tmp/tmp.Dx6ifBJLoP/bin//ruby
/app/bin/ruby
Running: which -a bundle
/tmp/tmp.Dx6ifBJLoP/bin//bundle
/app/vendor/bundle/bin/bundle
/app/vendor/bundle/ruby/3.1.0/bin/bundle
Running: bundle list
/tmp/buildpacks/5332b9a13007e20408c130cf3c47b025a8417ef0/lib/language_pack/helpers/bundle_list.rb:56:in `call': Error detecting dependencies (LanguagePack::Helpers::BundleList::CmdError)
The Ruby buildpack requires information about your application???s dependencies to
complete the build. Without this information, the Ruby buildpack cannot continue.
Command failed: `bundle list`
Could not find sinatra-4.2.1, puma-7.1.0, rack-3.2.4, rake-13.3.1, rackup-2.3.1, rack-test-2.2.0, test-unit-3.7.3, logger-1.7.0, mustermann-3.0.4, rack-protection-4.2.1, rack-session-2.1.1, tilt-2.6.1, nio4r-2.7.4, power_assert-3.0.1, base64-0.3.0 in locally installed gems
Install missing gems with `bundle install`.
```
(Debugging output added for later explanation)
This comes from a Heroku CI run which works by executing `bin/test-compile` (installing ruby and bundler etc.) and then calling `bin/test`. The `bin/test` already has a full ruby + bundler environment setup and ready to go, but we still download a bootstrap version of ruby so that the code only ever has to support one Ruby version (rather than N versions across the lifecycle of M stacks).
What's happening in the output is that we see that `ruby` being used is `/app/bin/ruby` (local `./bin` binstub) because we forced it onto the path via:
```
LanguagePack::ShellHelpers.user_env_hash["PATH"] = "#{app_path.join("bin")}:#{ENV["PATH"]}"
```
However the `bundle` executable is pointing to `/tmp/tmp.Dx6ifBJLoP/bin//bundle` instead of the correct `/app/vendor/bundle/bin/bundle` because we don't symlink that to the binstub directory (people expect to check in their `./bin/bundle` binstub and not have it overwritten). And because the PATH has the bootstrap ruby directory `/tmp/tmp.Dx6ifBJLoP/bin/` and that also includes a `bundle` executable, it uses that to call `gem list`.
Why weren't all of the tests failing? Well, the gems are installed to `./vendor/bundle/ruby/<major>.<minor>.0/`. The bundler that ships with ruby doesn't use PATH but instead is hardcoded to use the version of ruby it was shipped with:
```
$ chruby 3.3.9
$ cat $HOME/.rubies/ruby-3.3.9/bin/bundle
#!/Users/rschneeman/.rubies/ruby-3.3.9/bin/ruby
```
Or
```
$ curl https://heroku-buildpack-ruby.s3.us-east-1.amazonaws.com/heroku-24/arm64/ruby-4.0.0.preview3-bb0a79e.tgz -O
$ ...
$ cat ruby-4.0.0.preview3-bb0a79e/bin/bundle
#!/bin/sh
# -*- ruby -*-
# This file was generated by RubyGems.
#
# The application 'bundler' is installed as part of a gem, and
# this file is here to facilitate running it.
#
_=_\
=begin
bindir="${0%/*}"
exec "$bindir/ruby" "-x" "$0" "$@"
=end
#!/usr/bin/env ruby
```
Now, bundler is smart enough to look in `./vendor/bundle/ruby/<major>.<minor>.0/` when BUNDLE_DEPLOYMENT is set, BUT since it's being executed with the bootstrap ruby version, it will always be looking in `./vendor/bundle/ruby/3.3.0/` (bootstrap ruby version is 3.3.9 now). The test fails because it's using Ruby 3.1.3 which has gems installed to `./vendor/bundle/ruby/3.1.0/` which doesn't exist, so `gem list` fails to find any of the gems (because it's looking at the wrong directory because it was accidentally executed with the wrong ruby version.
Previously the fix was to exploit the "user provided environment variables" for a purpose it was never intended for, to put the customer's PATH first (in front of the bootstrap ruby version):
```
# - Add bundler's bin directory to the PATH
# - Always make sure `$HOME/bin` is first on the path
user_env_hash["PATH"] = "#{build_dir}/bin:#{bundler.bundler_path}/bin:#{user_env_hash["PATH"]}"
user_env_hash["GEM_PATH"] = LanguagePack::Ruby.slug_vendor_base
```
This approach requires knowing the bundler_path which is now derived like this:
```
# ...
class RubyVersion
# Ruby versioned bundler directory
#
# When installing gems via `BUNDLE_DEPLOYMENT=1 bundle install`, they're installed into a versioned directory based on the ruby version.
#
# This becomes the location of GEM_PATH on disk https://www.schneems.com/2014/04/15/gem-path.html.
# - Executables are at bundler_directory.join("bin")
# - Gems are at bundler_directory.join("gems")
#
# For example:
#
# - Ruby 3.4.7 would be "vendor/bundle/ruby/3.4.0"
# - JRuby 9.4.14.0 would be "vendor/bundle/jruby/3.1.0" (As it implements Ruby 3.1.7 spec)
def bundler_directory
"vendor/bundle/#{engine}/#{major}.#{minor}.0"
end
```
OR previously it was the same as `slug_vendor_base` which is derived by shelling out:
```
# For example "vendor/bundle/ruby/2.6.0"
def self.slug_vendor_base
@slug_vendor_base ||= begin
command = %q(ruby -e "require 'rbconfig';puts \"vendor/bundle/#{RUBY_ENGINE}/#{RbConfig::CONFIG['ruby_version']}\"")
out = run_no_pipe(command, user_env: true).strip
error "Problem detecting bundler vendor directory: #{out}" unless $?.success?
out
end
end
```
A better approach such as un-shifting the first value of path, or passing the bootstrap ruby version wasn't really feasible due to the architecture of the program. The refactoring that happened in v334 (that is reverted and re-applied in this PR) enabled this seemingly small, but very impactful simplification (removing re-exporting of the bootstrap PATH).
* Modify CI test to exercise `bin/test` code
Previously this test worked by using the `app.json` to define a test script. When that happens, the `bin/test` code is completely bypassed, so it's not under test.
This new modification instead uses the `bin/test` logic to call `bin/rake test` and that file prints the same information using the same script.
While the assertions in the test on this branch didn't need to change (except for asserting `bin/rake` is now in the `bin` directory), they will fail on `main` due to the way that `bin/test` is implemented, it resolves path order differently which can cause different execution behavior than if you define a test via `app.json` and different than the normally deployed Heroku app (in production).
* CHANGELOG
* Fix CI failure
With the new fix for PATH for `bin/test` Heroku CI users, this was accidentally working. This is the change in that test app:
```
$ git diff HEAD~1
diff --git a/Gemfile.lock b/Gemfile.lock
index 89b041a..b8f5c7a 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1,8 +1,10 @@
GEM
remote: https://rubygems.org/
specs:
- minitest (5.11.3)
- rake (12.3.1)
+ minitest (6.0.0)
+ prism (~> 1.5)
+ prism (1.7.0)
+ rake (13.2.1)
PLATFORMS
ruby
```
When the path was fixed it started failing with an error:
```
-----> Running test: rake test
rake aborted!
NoMethodError: undefined method `=~' for an instance of Proc
/app/vendor/bundle/ruby/3.3.0/gems/rake-12.3.1/exe/rake:27:in `<top (required)>'
(See full trace by running task with --trace)
-----> Ruby buildpack tests failed with exit status 1
# ./vendor/bundle/ruby/3.3.0/gems/heroku_hatchet-8.0.6/lib/hatchet/test_run.rb:135:in `block in wait!'
# /opt/hostedtoolcache/Ruby/3.3.9/x64/lib/ruby/3.3.0/timeout.rb:186:in `block in timeout'
# /opt/hostedtoolcache/Ruby/3.3.9/x64/lib/ruby/3.3.0/timeout.rb:41:in `handle_timeout'
# /opt/hostedtoolcache/Ruby/3.3.9/x64/lib/ruby/3.3.0/timeout.rb:195:in `timeout'
# ./vendor/bundle/ruby/3.3.0/gems/heroku_hatchet-8.0.6/lib/hatchet/test_run.rb:127:in `wait!'
```
It was working before because previously the path was different:
```
$ which -a rake
vendor/bundle/ruby/3.3.0/bin/rake
/tmp/tmp.V7sALmG2lV/bin//rake
/app/vendor/bundle/bin/rake
/app/vendor/bundle/ruby/3.3.0/bin/rake
```
This called `/app/vendor/bundle/ruby/3.3.0/bin/rake` (expanded from relative path) this is a RubyGems binstub and not a Bundler binstub. That means it does NOT use the Gemfile.lock to load gems. This is the binstub:
```
~ $ cat vendor/bundle/ruby/3.3.0/bin/rake
#!/usr/bin/env ruby
#
# This file was generated by RubyGems.
#
# The application 'rake' is installed as part of a gem, and
# this file is here to facilitate running it.
#
require 'rubygems'
Gem.use_gemdeps
version = ">= 0.a"
str = ARGV.first
if str
str = str.b[/\A_(.*)_\z/, 1]
if str and Gem::Version.correct?(str)
version = str
ARGV.shift
end
end
if Gem.respond_to?(:activate_bin_path)
load Gem.activate_bin_path('rake', 'rake', version)
else
gem "rake", version
load Gem.bin_path("rake", "rake", version)
end
```
This code will execute this path:
```
load Gem.activate_bin_path('rake', 'rake', version)
```
With a version of `>= 0.a` and produce a load path of `/app/vendor/ruby-3.3.9/lib/ruby/gems/3.3.0/gems/rake-13.1.0/exe/rake` this path is the default gem that ships with Ruby 3.3.0.
But now:
```
$ which -a rake
/app/vendor/bundle/bin/rake
/app/vendor/bundle/ruby/3.3.0/bin/rake
```
This will call `/app/vendor/bundle/bin` which loads bundler and is effectively the same as calling `bundle exec rake`.
```
~ $ cat vendor/bundle/bin/rake
#!/usr/bin/env ruby
# frozen_string_literal: true
#
# This file was generated by Bundler.
#
# The application 'rake' is installed as part of a gem, and
# this file is here to facilitate running it.
#
ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../../Gemfile", __dir__)
bundle_binstub = File.expand_path("bundle", __dir__)
if File.file?(bundle_binstub)
if File.read(bundle_binstub, 300).include?("This file was generated by Bundler")
load(bundle_binstub)
else
abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run.
Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.")
end
end
require "rubygems"
require "bundler/setup"
load Gem.bin_path("rake", "rake")
```
This will use the bundled version of rake such that `Gem.bin_path` returns `/app/vendor/bundle/ruby/3.3.0/gems/rake-12.3.1/exe/rake` and Rake 12.3.1 is used.
Rake 12.3.1 is incompatible with Ruby 3.3 so it fails.
Notably it SHOULD fail as this is what happens when you `git push heroku` on the same code:
```
~ $ rake -v
rake aborted!
NoMethodError: undefined method `=~' for an instance of Proc
/app/vendor/bundle/ruby/3.3.0/gems/rake-12.3.1/exe/rake:27:in `<top (required)>'
```
So the problem is in the app, which needs to be updated.1 parent 8a44458 commit 27848b7
File tree
39 files changed
+644
-536
lines changed- bin
- support
- changelogs/unreleased
- lib
- language_pack
- helpers
- installers
- test
- spec
- hatchet
- helpers
39 files changed
+644
-536
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
5 | 17 | | |
6 | 18 | | |
7 | 19 | | |
| |||
24 | 36 | | |
25 | 37 | | |
26 | 38 | | |
27 | | - | |
| 39 | + | |
28 | 40 | | |
29 | 41 | | |
30 | 42 | | |
| |||
1845 | 1857 | | |
1846 | 1858 | | |
1847 | 1859 | | |
| 1860 | + | |
1848 | 1861 | | |
1849 | 1862 | | |
1850 | 1863 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
30 | | - | |
31 | | - | |
32 | | - | |
33 | 30 | | |
34 | 31 | | |
35 | 32 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
24 | | - | |
| 24 | + | |
25 | 25 | | |
26 | 26 | | |
27 | | - | |
| 27 | + | |
| 28 | + | |
28 | 29 | | |
29 | | - | |
30 | | - | |
31 | | - | |
32 | 30 | | |
33 | 31 | | |
34 | 32 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
27 | | - | |
28 | | - | |
29 | | - | |
30 | | - | |
31 | | - | |
32 | | - | |
33 | | - | |
34 | | - | |
35 | | - | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
36 | 35 | | |
37 | 36 | | |
38 | 37 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
32 | | - | |
33 | | - | |
34 | | - | |
35 | | - | |
36 | | - | |
37 | | - | |
| 32 | + | |
| 33 | + | |
38 | 34 | | |
39 | | - | |
40 | | - | |
41 | | - | |
42 | | - | |
43 | | - | |
44 | | - | |
45 | | - | |
46 | | - | |
47 | | - | |
48 | | - | |
49 | | - | |
50 | | - | |
51 | | - | |
52 | | - | |
53 | | - | |
54 | | - | |
55 | | - | |
56 | | - | |
| 35 | + | |
57 | 36 | | |
58 | | - | |
59 | | - | |
60 | | - | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
61 | 40 | | |
62 | 41 | | |
63 | | - | |
| 42 | + | |
64 | 43 | | |
65 | 44 | | |
66 | 45 | | |
67 | 46 | | |
68 | 47 | | |
69 | | - | |
| 48 | + | |
70 | 49 | | |
71 | 50 | | |
72 | 51 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
23 | | - | |
24 | | - | |
25 | | - | |
26 | 23 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
26 | | - | |
27 | | - | |
28 | | - | |
29 | 26 | | |
30 | 27 | | |
31 | 28 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
23 | 106 | | |
24 | | - | |
| 107 | + | |
25 | 108 | | |
26 | | - | |
| 109 | + | |
27 | 110 | | |
28 | 111 | | |
29 | 112 | | |
30 | 113 | | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
31 | 118 | | |
32 | 119 | | |
33 | | - | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
34 | 123 | | |
35 | 124 | | |
36 | 125 | | |
| |||
0 commit comments