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

Stop setting BUNDLE_PATH #306

Merged
merged 3 commits into from
Jan 4, 2020
Merged

Stop setting BUNDLE_PATH #306

merged 3 commits into from
Jan 4, 2020

Conversation

deivid-rodriguez
Copy link
Contributor

@deivid-rodriguez deivid-rodriguez commented Jan 3, 2020

Newer bundler versions install gems to $GEM_HOME/ruby/<ruby_version>, instead of directly to $GEM_HOME.

So we need to add the proper paths to GEM_PATH and PATH so that gems
and their executables are properly found.

This is the proposal to fix rubygems/bundler#7494.

These changes are expected to be fully backwards compatible and should pass all the smoke tests to be added to https://github.com/docker-library/official-images:

I don't think that folder exists :/
Newer bundler versions install gems to `$GEM_HOME/ruby/<ruby_version>`,
instead of directly to `$GEM_HOME`.

So we need to add the proper paths to `GEM_PATH` and `PATH` so that gems
and their executables are properly found.
Stop setting `BUNDLE_PATH`.

All default bundler versions shipped with all supported rubies install
gems to `GEM_HOME` by default, so this shouldn't be a breaking change
and it's a less surprising behavior because it does not deviate from how
bundler is configured by default.

Future versions will probably install locally by default, but that
hasn't happened yet.

I think leaving bundler "unconfigured" also matches the direction of the
recent change where the images no longer explicitly install `bundler` but
simply leave whatever version comes with ruby. Similarly, it makes sense
to me to leave whatever configuration comes by default with `bundler` in
place.
@deivid-rodriguez
Copy link
Contributor Author

deivid-rodriguez commented Jan 3, 2020

Ok, so I was working on this and I realized removing BUNDLE_PATH might be the better option.

On one hand, it fixes the issue, since it no longer hits the particularly problematic behaviour and restores the previous behaviour.

On the other hand, It's simpler and I think deviating from bundler's default configuration has historically confused users. It's also simpler in docker images to set an environment variable than to unset it, so removing BUNDLE_PATH is more flexible. See for example #297 or #42.

Finally, all bundler versions shipped with all supported rubies install gems to GEM_HOME by default, so this should be backwards compatible and not change any behavior. It looks like this was originally introduced in #21, and intended to fix precisely the issue we're having now (gem list not matching bundle list). In particular, see #42 (comment). But what's explained in that comment should be the default behavior, so maybe the problem back then was a bug in an old bundler version or something like that.

Other bundler configurations meant to avoid the creation of a .bundle folder local to the working directory are preserved, and nothing should change regarding that.

Also, note that the behavior of installing gems globally is expected to change in the future, but it won't happen any time soon, so we'll deal with that when it happens.

@deivid-rodriguez deivid-rodriguez changed the title Add paths scoped to ruby version to PATH and GEM_PATH Stop setting BUNDLE_PATH Jan 3, 2020
@deivid-rodriguez
Copy link
Contributor Author

Changed the title to reflect the new approach.

Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

LGTM 👍

(Really appreciate the detailed summary and the time spent figuring out the best path forward!! 🤘)

@yosifkit yosifkit merged commit e30082f into docker-library:master Jan 4, 2020
@deivid-rodriguez deivid-rodriguez deleted the add_path_scoped_to_ruby_version_to_path_and_gem_path branch January 4, 2020 00:56
@deivid-rodriguez
Copy link
Contributor Author

Thanks both for your patience! 💜

docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Jan 4, 2020
Changes:

- docker-library/ruby@e30082f: Merge pull request docker-library/ruby#306 from deivid-rodriguez/add_path_scoped_to_ruby_version_to_path_and_gem_path
- docker-library/ruby@82eecb7: Alternative fix
- docker-library/ruby@7df74ad: Add paths scoped to ruby version to PATH and GEM_PATH
- docker-library/ruby@8f3650c: Remove unnecessary PATH modification
bquorning added a commit to rubocop/circleci-ruby-snapshot-image that referenced this pull request Jan 4, 2020
With changes from docker-library/ruby#306

This commit also reverts a999d22
@jimeh
Copy link

jimeh commented Jan 6, 2020

@tianon do you happen know when we might expect to see this change in the images on Docker Hub?

My understanding is a PR must be raised against docker-library/official-images. If there's no plan to do that at the moment, and it's open for anyone to do, I'd be happy to volunteer myself.

@jimbali
Copy link

jimbali commented Jan 6, 2020

Thanks for all your work on this @deivid-rodriguez! :)

@deivid-rodriguez
Copy link
Contributor Author

You're welcome! Also thanks @jimeh for the super useful input :)

@jimeh
Copy link

jimeh commented Jan 7, 2020

@deivid-rodriguez you did all the heavy lifting, I just butted in with annoying questions from time to time :D... thanks for all your effort :)

@slayer
Copy link

slayer commented Jan 8, 2020

This PR broke our CI builds :(

@deivid-rodriguez
Copy link
Contributor Author

I'm sorry that happened @slayer. I wasn't expecting anything to break, but this is not a perfect world as experience keeps proving.

Can you elaborate a bit more what happened? I'm pretty confident this is the right direction to take for the official images, but I'd still like to hear about your situation to see if there's something we should be changing.

@slayer
Copy link

slayer commented Jan 9, 2020

It is non critical, I have already fixed our Dockerfile, it just was a surprise when build suddenly fails

We used chown -r ${APP_USER} ${BUNDLE_PATH} to be able setup bundle gems under non-root user.

@deivid-rodriguez
Copy link
Contributor Author

Oh, I see. So the command became invalid when removing the BUNDLE_PATH environment variable... Yeah, I think this is an acceptable incompatibility that it's better to fix in the end users images like you did, I'm sorry it bit you though.

@NobodysNightmare
Copy link

@tianon do you happen know when we might expect to see this change in the images on Docker Hub?

My understanding is a PR must be raised against docker-library/official-images. If there's no plan to do that at the moment, and it's open for anyone to do, I'd be happy to volunteer myself.

So I understand it correct that these changes are not yet live in the official images, are they?

I can still see the issue with ruby:2.6.4-alpine, so now I was wondering, whether I still have a mistake on my side or whether the official images are just not yet updated.

@deivid-rodriguez
Copy link
Contributor Author

They were updated a while ago.

The issue you're having is that only the latest patch level version of each ruby is officially maintained and receives patches. So in addition to the mentioned workarounds, you could upgrade to ruby-2.6.5.

$ docker run --rm -it ruby:2.6.4-alpine sh -c "echo \$BUNDLE_PATH"
Unable to find image 'ruby:2.6.4-alpine' locally
2.6.4-alpine: Pulling from library/ruby
9d48c3bd43c5: Already exists 
9ce9598067e7: Pull complete 
278f4c997324: Pull complete 
a3a302efd712: Pull complete 
9d45a498a618: Pull complete 
Digest: sha256:f3eeb2b71ae7c004ca57ddb12618fde07dece33e6a0f0a50b53f03b83be76174
Status: Downloaded newer image for ruby:2.6.4-alpine
/usr/local/bundle

$ docker run --rm -it ruby:2.6.5-alpine sh -c "echo \$BUNDLE_PATH"
Unable to find image 'ruby:2.6.5-alpine' locally
2.6.5-alpine: Pulling from library/ruby
c9b1b535fdd9: Pull complete 
68ee169f7fe0: Pull complete 
c0dce75739cf: Pull complete 
8c71c542757d: Pull complete 
93d37870039a: Pull complete 
Digest: sha256:deaf1c8117ad605b45f879a84389afcd3030f302af3c9cffd8f827077882ad4d
Status: Downloaded newer image for ruby:2.6.5-alpine

$

Earlopain added a commit to Earlopain/official-images-docs that referenced this pull request Jul 5, 2024
tianon pushed a commit to docker-library/docs that referenced this pull request Jul 8, 2024
* Update examples from "ruby:3.0" to "ruby:3.3"

Previous: #2051

3.0 images are not build/updated anymore

* Drop mentions of unused bundler env variables for ruby

Dropped in:
* docker-library/ruby#209
* docker-library/ruby#306
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bundler 2.1.0 does not expose executables from gems without "bundle exec", while 2.0.2 does
7 participants