Fix user binstubs not on PATH and fix PATH order #383
Merged
Conversation
This file contains 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
Note that this places `/layers/heroku_ruby/gems/bin` before `/layers/heroku_ruby/binruby` on the path. Related to, but doesn't entirely fix #380.
Using `-a` also shows that there's more than one value present.
It's common and expected that Rails applications will include a `bin` directory containing "binstubs" of executables their app depends on. For example https://github.com/heroku/ruby-getting-started/tree/5e7ce01610a21cf9e5381daea66f79178e2b3c06/bin. They're largely used to ensure that bundler is invoked/used so that you can run `bin/rails` rather than needing to use `bundle exec rails`. However it's not strictly limited to only that. This change: Adds the `bin` folder in the root of the workspace to the PATH and changes the layer to `venv` so it is loaded after other layers (and takes precedence in the case of a PATH prepend). This fixes the previously committed failing test. Close #380
schneems
force-pushed
the
schneems/add-user-binstubs
branch
from
10a45ed to
2bec8ef
Compare
schneems
commented
Jan 10, 2025
The Application Contract does not specify that commands such as `rake -P` will be called with `bundle exec` and the classic buildpack does not rely on `bundle exec` internally. This brings the CNB closer to parity with the classic buildpack. In the container environment, the first gems on the PATH should be those installed by the buildpack, negating the strict need to call `bundle exec` as you would on a development machine. Usually prepending a Ruby command with `bundle exec` will have no discernible difference for an application that's bug free. This is evidenced by all tests passing with this change. However someone can commit their own `bin/rake` or `bin/rails` and we should use this over the executable installed via `bundle install`.
schneems
force-pushed
the
schneems/add-user-binstubs
branch
from
6f97cb5 to
b17681c
Compare
At runtime, the alphabetical order of the layer name determines the order it is loaded. At build time, the order that the `env` variable is modified determines the order. At both build and runtime we want the bin stubs to come first on the PATH when executing user defined code. This was already working for runtime, but wasn't for build time as the "gems" layer was being prepended to the path after the "venv" layer (because the `venv` layer was being defined first, last definition wins). I originally tried to fix this by defining the PATH inside of the "gems" layer along with the gems path but ran into heroku/libcnb.rs#899. The libcnb.rs project loads the user defined PATH modification last, but I'm unclear if that's spec defined behavior or not https://github.com/buildpacks/spec/blob/main/buildpack.md#layer-paths.
schneems
force-pushed
the
schneems/add-user-binstubs
branch
from
b17681c to
c91cee5
Compare
edmorley
approved these changes
Jan 13, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Superscedes #382. Close #380
rubytobinrubywhich allows executables intalled viabundle installto take precedence/workspace/binto the PATH with a layer name that comes aftergemsandbinrubyCommits are small and in order.
GUS-W-17544084