Auto merge of #4992 - chrismo:fix_disable_shared_gems_gem_path, r=chrismo

Fix GEM_PATH regression in 1.13.

_nth time is a charm ... here's the latest latest latest commit message_:

Fix disable_shared_gems bug & keep nested exec
tl;dr `--deployment` will copy _all_ gems to the `--path` now and a
proper fix is in place for nested bundle execs when `--path` is set and
RubyGems >=2.6.2 is being used.

Fixes #4974.

There's two problems blended in here. Let's trace the events here from
the beginning, shall we?

First off, from probably the first versions of Bundler, when
`disable_shared_gems` is true, the `GEM_PATH` gets internally overridden
and initialized to an empty string. This is done to make sure that later
in the `Bundler.setup` process it's expanded to ONLY the Bundler `--path`
setting, otherwise it expands to include the system path.

In 1.12, `bundle exec` was changed to use `Kernel.load` in some cases,
and that new code path calls `require "bundler/setup"`.

Later, issue #4592 was filed, showing that in cases like `--deployment`
where `disable_shared_gems` is true, Bundler couldn't find itself,
because Bundler never lives in the `--path` but only in system gems. And
as it would later be discovered, was not a problem with RubyGems 2.6.1,
but was a problem with >=2.6.2 (and older RubyGems it would seem, though
those weren't as thoroughly investigated).

We fixed #4592 (see PR #4701) in 1.13.0 by changing the oooold code
initializing `GEM_PATH` to be initialized to `nil` instead of empty
string in all `disable_shared_gems` cases. But it created another bug,
filed as #4974, wherein system gems were no longer copied to the
`--path` when `disable_shared_gems` is true. In this commit here (#4992)
we've reverted the change so that `GEM_PATH` is now back to being
initialized to an empty string instead of `nil`.

That fixes #4974, but regresses #4592, and we needed a new way to fix
it.

After a few tortured attempts at this, I ran across issue #4602, a
similar report of nested bundle execs being broken, #4602 itself an
extension of #4381 reporting the same problem. It brought to light the
role the Rubygems version played in the problem.

When the `bundler` gem is installed and the wrapper is generated for any
gem executables, the contents of this wrapper are determined by the
Rubygems version. Up until RubyGems 2.6.1 the last line of this wrapper
calls `Gem.bin_path`. Bundler replaces the Rubygems implementation of
`Gem.bin_path` with its own, and has for a long time made a special
exception for the Bundler gem itself, short-circuiting with the contents
of a special ENV variable called `BUNDLE_BIN_PATH`.

In Rubygems 2.6.2, `bin_path` was superseded by a new
`Gem.activate_bin_path` method which did what `bin_path` did but also
activated the gem. Bundler 1.13 added support for this, but it didn't
include the same short-circuit for bundler itself. (Alert user @taoza
even noticed this here
fcaab35#r56665282).

This commit also includes that short circuit for Rubygems >=2.6.2 now
and nested bundle exec should continue to work.

5 files changed, 60 insertions, 7 deletions

diff --git a/lib/bundler.rb b/lib/bundler.rb
index f5bbd61f57..249c4e2dc6 100644
--- a/lib/bundler.rb
+++ b/lib/bundler.rb
@@ -449,7 +449,10 @@ EOF
     def configure_gem_path(env = ENV, settings = self.settings)
       blank_home = env["GEM_HOME"].nil? || env["GEM_HOME"].empty?
       if settings[:disable_shared_gems]
-        env["GEM_PATH"] = nil
+        # this needs to be empty string to cause
+        # PathSupport.split_gem_path to only load up the
+        # Bundler --path setting as the GEM_PATH.
+        env["GEM_PATH"] = ""
       elsif blank_home || Bundler.rubygems.gem_dir != bundle_path.to_s
         possibles = [Bundler.rubygems.gem_dir, Bundler.rubygems.gem_path]
         paths = possibles.flatten.compact.uniq.reject(&:empty?)
diff --git a/lib/bundler/rubygems_integration.rb b/lib/bundler/rubygems_integration.rb
index 23ae95aa04..1c44ef3ddd 100644
--- a/lib/bundler/rubygems_integration.rb
+++ b/lib/bundler/rubygems_integration.rb
@@ -397,6 +397,17 @@ module Bundler
         spec
       end
 
+      redefine_method(gem_class, :activate_bin_path) do |name, *args|
+        exec_name = args.first
+        return ENV["BUNDLE_BIN_PATH"] if exec_name == "bundle"
+
+        # Copy of Rubygems activate_bin_path impl
+        requirement = args.last
+        spec = find_spec_for_exe name, exec_name, [requirement]
+        Gem::LOADED_SPECS_MUTEX.synchronize { spec.activate }
+        spec.bin_file exec_name
+      end
+
       redefine_method(gem_class, :bin_path) do |name, *args|
         exec_name = args.first
         return ENV["BUNDLE_BIN_PATH"] if exec_name == "bundle"
diff --git a/spec/bundler/bundler_spec.rb b/spec/bundler/bundler_spec.rb
index 2ff9920614..cf6de26ac3 100644
--- a/spec/bundler/bundler_spec.rb
+++ b/spec/bundler/bundler_spec.rb
@@ -143,12 +143,12 @@ describe Bundler do
 
   describe "configuration" do
     context "disable_shared_gems" do
-      it "should unset GEM_PATH with nil" do
+      it "should unset GEM_PATH with empty string" do
         env = {}
         settings = { :disable_shared_gems => true }
         Bundler.send(:configure_gem_path, env, settings)
         expect(env.keys).to include("GEM_PATH")
-        expect(env["GEM_PATH"]).to be_nil
+        expect(env["GEM_PATH"]).to eq ""
       end
     end
   end
diff --git a/spec/commands/exec_spec.rb b/spec/commands/exec_spec.rb
index 42afbd24ba..4dc47919de 100644
--- a/spec/commands/exec_spec.rb
+++ b/spec/commands/exec_spec.rb
@@ -2,8 +2,9 @@
 require "spec_helper"
 
 describe "bundle exec" do
+  let(:system_gems_to_install) { %w(rack-1.0.0 rack-0.9.1) }
   before :each do
-    system_gems "rack-1.0.0", "rack-0.9.1"
+    system_gems(system_gems_to_install)
   end
 
   it "activates the correct gem" do
@@ -627,4 +628,26 @@ __FILE__: #{path.to_s.inspect}
       end
     end
   end
+
+  context "nested bundle exec" do
+    let(:system_gems_to_install) { super() << :bundler }
+    before do
+      gemfile <<-G
+        source "file://#{gem_repo1}"
+        gem "rack"
+      G
+      bundle :install, :system_bundler => true, :path => "vendor/bundler"
+    end
+
+    it "overrides disable_shared_gems so bundler can be found" do
+      file = bundled_app("file_that_bundle_execs.rb")
+      create_file(file, <<-RB)
+        #!#{Gem.ruby}
+        puts `bundle exec echo foo`
+      RB
+      file.chmod(0o777)
+      bundle! "exec #{file}", :system_bundler => true
+      expect(out).to eq("foo")
+    end
+  end
 end
diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb
index b30c44d1cb..6b30afd480 100644
--- a/spec/support/helpers.rb
+++ b/spec/support/helpers.rb
@@ -88,18 +88,28 @@ module Spec
 
       bundle_bin = options.delete("bundle_bin") || File.expand_path("../../../exe/bundle", __FILE__)
 
+      if system_bundler = options.delete(:system_bundler)
+        bundle_bin = "-S bundle"
+      end
+
       requires = options.delete(:requires) || []
       requires << File.expand_path("../fakeweb/" + options.delete(:fakeweb) + ".rb", __FILE__) if options.key?(:fakeweb)
       requires << File.expand_path("../artifice/" + options.delete(:artifice) + ".rb", __FILE__) if options.key?(:artifice)
       requires << "support/hax"
       requires_str = requires.map {|r| "-r#{r}" }.join(" ")
 
+      load_path = []
+      load_path << lib unless system_bundler
+      load_path << spec
+      load_path_str = "-I#{load_path.join(File::PATH_SEPARATOR)}"
+
       env = (options.delete(:env) || {}).map {|k, v| "#{k}='#{v}'" }.join(" ")
+      env["PATH"].gsub!("#{Path.root}/exe", "") if env["PATH"] && system_bundler
       args = options.map do |k, v|
         v == true ? " --#{k}" : " --#{k} #{v}" if v
       end.join
 
-      cmd = "#{env} #{sudo} #{Gem.ruby} -I#{lib}:#{spec} #{requires_str} #{bundle_bin} #{cmd}#{args}"
+      cmd = "#{env} #{sudo} #{Gem.ruby} #{load_path_str} #{requires_str} #{bundle_bin} #{cmd}#{args}"
       sys_exec(cmd) {|i, o, thr| yield i, o, thr if block_given? }
     end
     bang :bundle
@@ -246,11 +256,17 @@ module Spec
 
     def install_gems(*gems)
       gems.each do |g|
-        path = "#{gem_repo1}/gems/#{g}.gem"
+        path = if g == :bundler
+          Dir.chdir(root) { gem_command! :build, "#{root}/bundler.gemspec" }
+          bundler_path = root + "bundler-#{Bundler::VERSION}.gem"
+        else
+          "#{gem_repo1}/gems/#{g}.gem"
+        end
 
         raise "OMG `#{path}` does not exist!" unless File.exist?(path)
 
-        gem_command! :install, "--no-rdoc --no-ri --ignore-dependencies #{path}"
+        gem_command! :install, "--no-rdoc --no-ri --ignore-dependencies '#{path}'"
+        bundler_path && bundler_path.rmtree
       end
     end