-
Notifications
You must be signed in to change notification settings - Fork 8
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
Example Ruby buildpack actually uses system Ruby #398
Comments
edmorley
added a commit
that referenced
this issue
Jun 10, 2022
Since it supersedes `heroku/buildpacks:20`: https://github.com/heroku/builder#heroku-builder-images Whilst updating the example build output in the README, I noticed that the specified commands were incorrect (eg wrong buildpack path), so those have been fixed. In addition, I've switched the hello world example to use the wildcard stack, since the code is not stack-specific, and it will save us having to remember to update it in the future. Lastly, updating the Ruby example buildpack to `heroku/builder:22` revealed that the buildpack was actually broken and relying on system Ruby, so that buildpack has been excluded from using the new builder until that is resolved: #398
Good spot, thank you :-) |
edmorley
changed the title
Example Ruby buildpack is broken
Example Ruby buildpack actually uses system Ruby
Jan 2, 2024
In addition to the issue mentioned in the OP here, there are multiple other problems with the Ruby CNB example in this repo:
IMO we should just delete this example, and instead:
|
Merged
edmorley
added a commit
that referenced
this issue
Jan 3, 2024
Fixes the following CI failures on `main`: 1. Lint failures on the newly released Rust 1.75 (see #756) (plus pre-emptively fixes another for Rust 1.76 beta). 2. Integration test failures due to the Ruby example having picked up newer bundler, which is not compatible with the Ruby 2.7 it uses. I've opted to delete the Ruby example entirely, since: 1. This is currently blocking libcnb.rs PRs/development. 2. It has multiple issues that really require a complete rewrite: - #398 - #479 - #746 - #755 3. IMO including a full language CNB example in this repo is not something we should do, since it's both never going to fully support the language in a best practice way, and also will be too complicated to demonstrate libcnb concepts in the simplified way that we should be using in an example. Instead, we should stick to simple examples of concepts, and then link out to our real-world CNB repos for users who want to do further reading. As an added bonus, the removal will speed up CI a fair amount, since the Ruby integration test was very slow (due to it bundler plus the test using a different builder image, so another docker pull). Fixes #756. Fixes #755. Closes #398. Closes #479. Closes #746. GUS-W-14739082. GUS-W-14739086.
Wontfix since the Ruby example has been removed in #757. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When I updated all usages of
heroku/buildpacks:20
in this repository to instead useheroku/builder:22
the integration test for the example Ruby buildpack started failing.After investigation, it turns out that the example Ruby buildpack is actually pretty broken at the moment.
For example, when the test runs, the image actually uses system Ruby, and so when run under
heroku/builder:22
(which is based on Heroku-22, which no longer has system Ruby), the image fails at runtime.The reason it's falling back to system Ruby seems to be due to the manual
PATH
definition here, since it's missing the delimiter:libcnb.rs/examples/ruby-sample/src/layers/ruby.rs
Lines 46 to 57 in 6452900
This then causes a broken
PATH
:(See also buildpacks/spec#285)
I removed this manual
PATH
definition (which was also referencing Ruby 2.6, so completely wrong?), along with the seemingly also redundantLD_LIBRARY_CONFIG
env var.However then ran into another issue:
As such, I'm going to exclude this example buildpack from the PR for
heroku/builder:22
-- we'll have to fix this example buildpack first.cc @schneems @Malax
The text was updated successfully, but these errors were encountered: