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

Cannot require gem if BUNDLE_PATH is set #3298

Closed
pocke opened this issue Jun 11, 2019 · 9 comments
Closed

Cannot require gem if BUNDLE_PATH is set #3298

pocke opened this issue Jun 11, 2019 · 9 comments

Comments

@pocke
Copy link
Contributor

pocke commented Jun 11, 2019

I found the problem while developing RuboCop.
rubocop/rubocop#7124

Problem

Ruby does not find a gem that is installed by bundler with BUNDLE_PATH since ruby/ruby@8f37629 . This commit updated bundler code in ruby/ruby repository.

Reproducing

FROM ubuntu:18.04

ENV BUNDLER_MERGE_COMMIT 8f37629519ad330032a38ac0e871b2912ed38a1b
ENV WORKING_DIR /work

RUN apt-get update -y
RUN apt-get install -y autoconf bison build-essential libssl-dev libyaml-dev libreadline6-dev zlib1g-dev libncurses5-dev libffi-dev libgdbm5 libgdbm-dev git ruby
RUN git clone --depth=100 https://github.com/ruby/ruby

RUN mkdir $WORKING_DIR && echo "source 'https://rubygems.org'; gem 'pry'" > "$WORKING_DIR/Gemfile"

RUN mkdir /before
RUN mkdir /after

RUN cd ruby/ && git checkout "${BUNDLER_MERGE_COMMIT}~" && autoreconf && ./configure --prefix=/before && make -j && make install
RUN cd ruby/ && git checkout "${BUNDLER_MERGE_COMMIT}"  && autoreconf && ./configure --prefix=/after  && make -j && make install

RUN echo 'Modify me to remove cache'

ENV GEM_HOME /usr/local/bundle
ENV BUNDLE_PATH="$GEM_HOME"

RUN echo "After merge bundler"
RUN cd $WORKING_DIR && /after/bin/bundle install && /after/bin/ruby -v -r pry -e '' || true

RUN echo "Before merge bundler"
RUN cd $WORKING_DIR && /before/bin/bundle install && /before/bin/ruby -v -r pry -e '' || true

I use a Dockerfile to reproduce.
It dispplays cannot load such file -- pry .

$ docker build .
(snip)

Step 15/18 : RUN echo "After merge bundler"
 ---> Running in 2cd4e88549b1
After merge bundler
Removing intermediate container 2cd4e88549b1
 ---> 5903c6252630
Step 16/18 : RUN cd $WORKING_DIR && /after/bin/bundle install && /after/bin/ruby -v -r pry -e '' || true
 ---> Running in fbfa538b00ea
Don't run Bundler as root. Bundler can ask for sudo if it is needed, and
installing your bundle as root will break this application for all non-root
users on this machine.
Fetching gem metadata from https://rubygems.org/.........
Resolving dependencies...
Using bundler 2.1.0.pre.1
Fetching coderay 1.1.2
Installing coderay 1.1.2
Fetching method_source 0.9.2
Installing method_source 0.9.2
Fetching pry 0.12.2
Installing pry 0.12.2
Bundle complete! 1 Gemfile dependency, 4 gems now installed.
Bundled gems are installed into `/usr/local/bundle`
ruby 2.7.0dev (2019-06-09T12:44:10+09:00 trunk 8f37629519) [x86_64-linux]
/after/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:54:in `require': cannot load such file -- pry (LoadError)
        from /after/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:54:in `require'
Removing intermediate container fbfa538b00ea
 ---> 2c0821585bda
Step 17/18 : RUN echo "Before merge bundler"
 ---> Running in 976aee50b0c0
Before merge bundler
Removing intermediate container 976aee50b0c0
 ---> 16582b269efa
Step 18/18 : RUN cd $WORKING_DIR && /before/bin/bundle install && /before/bin/ruby -v -r pry -e '' || true
 ---> Running in f11a252a1f4d
Don't run Bundler as root. Bundler can ask for sudo if it is needed, and
installing your bundle as root will break this application for all non-root
users on this machine.
Fetching gem metadata from https://rubygems.org/.........
Using bundler 2.1.0.pre.1
Fetching coderay 1.1.2
Installing coderay 1.1.2
Fetching method_source 0.9.2
Installing method_source 0.9.2
Fetching pry 0.12.2
Installing pry 0.12.2
Bundle complete! 1 Gemfile dependency, 4 gems now installed.
Bundled gems are installed into `/usr/local/bundle`
ruby 2.7.0dev (2019-06-09T12:31:12+09:00 trunk 6650899248) [x86_64-linux]
Removing intermediate container f11a252a1f4d
 ---> 5257e284fe9e
Successfully built 5257e284fe9e
@deivid-rodriguez
Copy link
Member

Thanks so much for the clean report, and for finding out this. I'll investigate this and report back! 👍

@deivid-rodriguez
Copy link
Member

deivid-rodriguez commented Jun 11, 2019

This seems like a regression of rubygems/bundler#7163, and @indirect was right about rubygems/bundler#7163 (comment)... 😅 although probably not in the way we expected.

In this case, it sounds like the intention is that bundler uses system gems. The way the current code expects people to do that is by setting BUNDLE_PATH__SYSTEM=true because setting BUNDLE_PATH directly will append the ruby scope to the path, so gems are not really installed to $GEM_HOME, but to $GEM_HOME/ruby/2.7.0.

So:

  • The immediate fix would be to change the BUNDLE_PATH="GEM_HOME" line to BUNDLE_PATH__SYSTEM=true (or alternatively to prepend bundle exec to your ruby command, so that the "custom" $GEM_HOME/ruby/2.7.0 path is used).

  • We should update the official ruby images with that change as well.

Another debate is whether this distinction makes sense and whether we need it or not. The relevant code is here:

https://github.com/bundler/bundler/blob/e6fd423bbdf6fc2aa1032a5e35df7b4079c45ddd/lib/bundler/settings.rb#L222

The ruby scope is only appended to the path if we're not using system gems. Maybe if we adapted the behavior to only append the ruby scope if it's not already present in the BUNDLE_PATH, this would just work as expected for every case.

@indirect Thougths?

@deivid-rodriguez
Copy link
Member

The fix I suggest at the end of my previous message would be something like

diff --git a/lib/bundler/settings.rb b/lib/bundler/settings.rb
index 3888ac51d..945bac76f 100644
--- a/lib/bundler/settings.rb
+++ b/lib/bundler/settings.rb
@@ -219,7 +219,7 @@ module Bundler
     Path = Struct.new(:explicit_path, :system_path, :default_install_uses_path) do
       def path
         path = base_path
-        path = File.join(path, Bundler.ruby_scope) unless use_system_gems?
+        path = File.join(path, Bundler.ruby_scope) unless path.end_with?(Bundler.ruby_scope)
         path
       end
 

@pocke
Copy link
Contributor Author

pocke commented Jun 12, 2019

Thank you for investigation!

Just FYI, I tried adding bundle exec, but it does not work for me.
rubocop/rubocop#7135
https://circleci.com/gh/rubocop-hq/rubocop/58232?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

@deivid-rodriguez
Copy link
Member

Thanks @pocke! In principle that error is also unexpected to me, so I'll have a look at that too 👍.

@deivid-rodriguez
Copy link
Member

I investigated why prepending bundle exec doesn't work here.

Prepending bundle exec to a command, among other things, puts -rbundle/setup at the front of RUBYOPT. However, ruby command line flags still take precedence over RUBYOPT so by the time pry is required, the bundler environment has not yet been properly setup.

We could add custom code to bundle exec to handle the special case of bundle exec'ing to ruby but for now the recommendation would be to not use bundle exec ruby, but do ruby -rbundle/setup instead.

ghost referenced this issue in rubygems/bundler Dec 17, 2019
7501: Delay appending `ruby/<ABI_VERSION>` to `$BUNDLE_PATH` r=indirect a=deivid-rodriguez

### What was the end-user problem that led to this PR?

The problem was that #7163 broke several things regarding official docker ruby images.

### What was your diagnosis of the problem?

My diagnosis was that appending `ruby/<ABI_VERSION>` to `$BUNDLE_PATH` break things there because gems nor executables are no longer installed at a location that `rubygems` now about.

### What is your fix for the problem, implemented in this PR?

My fix is to revert the offending PR for the time being.

### Why did you choose this fix out of the possible options?

I chose this fix because it's quick.

I think the better fix would be to get `BUNDLE_PATH__SYSTEM=true` working as expected and upstream that change to the docker images, because I think that's exactly the use case for the docker images. But I need more time for that, and I want to restore working behavior.

/cc @indirect Sorry, you were right about this being dangerous 😳

Closes #7197.
Closes #7494.

Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
@hsbt hsbt transferred this issue from rubygems/bundler Mar 14, 2020
@akostadinov
Copy link

@deivid-rodriguez , when BUNDLE_PATH is set in global config or environment variable causes Bundler NOT to append ruby scope, while setting this option on command line or in project config cause it to DO append that. Just in case that matters for your fix.

You can see more about what I found at https://stackoverflow.com/a/64163511/520567

@deivid-rodriguez
Copy link
Member

Hei, I lost track of this issue, but #3933 removed the special logic you point out in the stackoverflow thread, so it might've fixed this as a side effect. Could you try if this is reproducible on the latest default branch.

@deivid-rodriguez
Copy link
Member

I believe this particular issue has been fixed, but I opened #4025 for a separate problem discovered while investigating this. I'll close this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants
@colby-swandale @akostadinov @deivid-rodriguez @pocke @bronzdoc and others