Test in staging, Fix binstubs, Update AWS upload syntax #5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR replaces the prior testing method with Hatchet. It adds support for testing against the staging bucket: ``` $ bundle exec rake test[2.3.0,cedar-14,true] ```
Manual test output: ``` $ bundle exec rake test[2.3.0,cedar-14,staging] › Warning: token will expire 04/21/2021 › Use heroku authorizations:create to generate a long-term token Hatchet setup: "hatchet-t-36e571baec" for "default_ruby" Running cat bin/rake | head -n 1 on hatchet-t-36e571baec... up, run.5765 (Free) Running echo "Ruby version: $(ruby -v), Gem version: $(gem -v)" on hatchet-t-36e571baec... up, run.4327 (Free) Stack: cedar-14, Ruby version: ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-linux], Gem version: 2.5.1, s3_bucket: staging Destroying "hatchet-t-418ce77161": 9b0648c1-ac6c-40af-a949-0754c1f1b20e. Hatchet app limit: 20 Warning: Reached Heroku app limit of 100. ⛄ 2.7.1 🚀 ~/Documents/projects/docker-heroku-ruby-builder (schneems/staging-test) $ heroku run "cat bin/rake | head -n 1" -a hatchet-t-36e571baec Running cat bin/rake | head -n 1 on ⬢ hatchet-t-36e571baec... up, run.8771 (Free) #!/usr/bin/env ruby ```
schneems
changed the title
schneems
added a commit
to heroku/heroku-buildpack-ruby
that referenced
this pull request
Jul 6, 2020
After v216 was released there were several failures reported in #1029 and #1005 these are both related to `bin/` being put first on the path consistently in build and run time. After investigation it was uncovered that `bin/rake` was a file we're putting in that location if the customer does not check in a `bin/rake` binstub. This file is generated by compiling a Ruby binary: ``` $ curl -O https://heroku-buildpack-ruby.s3.amazonaws.com/heroku-18/ruby-2.7.1.tgz $ tar -xzf ruby-2.7.1.tgz bin/ $ cat bin/rake #!/bin/sh # -*- 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. # _=_\ =begin bindir="${0%/*}" exec "$bindir/ruby" "-x" "$0" "$@" =end #!/usr/bin/env ruby require 'rubygems' 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 ``` v216 release caused two related issues due to having `bin/rake first on the PATH. ## Bad shebang lines Issue #1005 is related to having bad shebang lines: ``` $ cat bin/rake | head -n 1 #!/app/vendor/ruby-2.3.8/bin/ruby ``` This was fixed by modifying our compilation code in: heroku/docker-heroku-ruby-builder#5 ## Bundler not loaded Issue #1029 occurs because the code in `bin/rake` that is generated from compiling Ruby is not bundler aware. It does not load bundler. As a result: - Git based gems do not work with the Ruby compiled `bin/rake` - Referencing code from bundler like `Bundler.setup` does not work because it's not yet required This behavior is described in detail in this comment: #1025 (comment) The short version is: Without putting `bin/` first on the path at build time then the binstub generated by bundler in the location `vendor/bundle/bin/` takes precedence and this code (since it is generated by bundler) is bundler aware. When `bin/` was moved to be first in the path, this behavior broke. ## Fix implementation This fix for #1029 works by not putting the Ruby compiled `bin/rake` on in the customer's `bin/` folder. We keep `bin/` first in the path so if a customer is checking in a binstub their binstub will be used. If they are not checking in a binstub then the bundler generated version of `vendor/bundle/bin/` will be used. This fix also would have superseded the work to fix #1005 and not required re-compiling dozens of binaries, but this bug was found and diagnosed after #1005 was reported. ## Known risks The one risk with this approach is that anyone relying on running `rake assets:precompile` at build time but they're not using Rake in their Gemfile will no longer work. I believe this is not a common case, and it's best practice to have a specific version of Rake in the Gemfile.lock.
schneems
added a commit
to heroku/heroku-buildpack-ruby
that referenced
this pull request
Jul 7, 2020
After v216 was released there were several failures reported in #1029 and #1005 these are both related to `bin/` being put first on the path consistently in build and run time. After investigation it was uncovered that `bin/rake` was a file we're putting in that location if the customer does not check in a `bin/rake` binstub. This file is generated by compiling a Ruby binary: ``` $ curl -O https://heroku-buildpack-ruby.s3.amazonaws.com/heroku-18/ruby-2.7.1.tgz $ tar -xzf ruby-2.7.1.tgz bin/ $ cat bin/rake #!/bin/sh # -*- 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. # _=_\ =begin bindir="${0%/*}" exec "$bindir/ruby" "-x" "$0" "$@" =end #!/usr/bin/env ruby require 'rubygems' 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 ``` v216 release caused two related issues due to having `bin/rake first on the PATH. ## Bad shebang lines Issue #1005 is related to having bad shebang lines: ``` $ cat bin/rake | head -n 1 #!/app/vendor/ruby-2.3.8/bin/ruby ``` This was fixed by modifying our compilation code in: heroku/docker-heroku-ruby-builder#5 ## Bundler not loaded Issue #1029 occurs because the code in `bin/rake` that is generated from compiling Ruby is not bundler aware. It does not load bundler. As a result: - Git based gems do not work with the Ruby compiled `bin/rake` - Referencing code from bundler like `Bundler.setup` does not work because it's not yet required This behavior is described in detail in this comment: #1025 (comment) The short version is: Without putting `bin/` first on the path at build time then the binstub generated by bundler in the location `vendor/bundle/bin/` takes precedence and this code (since it is generated by bundler) is bundler aware. When `bin/` was moved to be first in the path, this behavior broke. ## Fix implementation This fix for #1029 works by not putting the Ruby compiled `bin/rake` on in the customer's `bin/` folder. We keep `bin/` first in the path so if a customer is checking in a binstub their binstub will be used. If they are not checking in a binstub then the bundler generated version of `vendor/bundle/bin/` will be used. This fix also would have superseded the work to fix #1005 and not required re-compiling dozens of binaries, but this bug was found and diagnosed after #1005 was reported. ## Known risks The one risk with this approach is that anyone relying on running `rake assets:precompile` at build time but they're not using Rake in their Gemfile will no longer work. I believe this is not a common case, and it's best practice to have a specific version of Rake in the Gemfile.lock.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Test in staging
This PR replaces the prior testing method with Hatchet. It adds support for testing against the staging bucket:
Fix binstubs
Manually replace known bad binstub pattern with
usr/bin/env rubyheroku/heroku-buildpack-ruby#1005. The binstubs are generated by rubygems and it doesn't look like there's a way to get the--env-shebangflag to the method that generates them in some external/supported way.This builds off of the ability to test in staging since we can now be more confident about changes to binaries.
Update AWS upload syntax
The AWS gem has been updated and the syntax changed for how it's used. This was needed to get rid of dependency incompatabilities from upgrading the json gem version for a Ruby 2.7.1 deprecation.