Merge #7303

7303: Refactor ruby core integration r=hsbt a=deivid-rodriguez

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

The problem was that sometimes we break specs when integrating bundler changes into core.

### What was your diagnosis of the problem?

My diagnosis was that sometimes we use paths dependent on the structure of this repo, but that break under ruby-core's structure.

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

My fix is only some refactoring so that usage of structure independent helpers is encouraged. After this set of changes, if you grep the repo for `ruby_core?`, the only result will be `spec/support/path.rb`. That means that all logic dealing with repo folder structure lives in a single place. 

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

I chose this fix because it makes the integration in core cleaner.


Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>

11 files changed, 87 insertions, 111 deletions

diff --git a/lib/bundler/shared_helpers.rb b/lib/bundler/shared_helpers.rb
index af1836009f..a9bfd57fae 100644
--- a/lib/bundler/shared_helpers.rb
+++ b/lib/bundler/shared_helpers.rb
@@ -304,7 +304,7 @@ module Bundler
       exe_file = File.expand_path("../../../exe/bundle", __FILE__)
 
       # for Ruby core repository testing
-      exe_file = File.expand_path("../../../bin/bundle", __FILE__) unless File.exist?(exe_file)
+      exe_file = File.expand_path("../../../libexec/bundle", __FILE__) unless File.exist?(exe_file)
 
       # bundler is a default gem, exe path is separate
       exe_file = Bundler.rubygems.bin_path("bundler", "bundle", VERSION) unless File.exist?(exe_file)
diff --git a/spec/bundler/shared_helpers_spec.rb b/spec/bundler/shared_helpers_spec.rb
index 910854fe46..0340cb7bae 100644
--- a/spec/bundler/shared_helpers_spec.rb
+++ b/spec/bundler/shared_helpers_spec.rb
@@ -223,14 +223,6 @@ RSpec.describe Bundler::SharedHelpers do
       ENV["BUNDLE_GEMFILE"] = "Gemfile"
     end
 
-    let(:setup_path) do
-      if ruby_core?
-        File.expand_path("../../../lib/bundler/setup", __dir__)
-      else
-        File.expand_path("../../lib/bundler/setup", __dir__)
-      end
-    end
-
     shared_examples_for "ENV['PATH'] gets set correctly" do
       before { Dir.mkdir ".bundle" }
 
@@ -244,7 +236,7 @@ RSpec.describe Bundler::SharedHelpers do
     shared_examples_for "ENV['RUBYOPT'] gets set correctly" do
       it "ensures -rbundler/setup is at the beginning of ENV['RUBYOPT']" do
         subject.set_bundle_environment
-        expect(ENV["RUBYOPT"].split(" ")).to start_with("-r#{setup_path}")
+        expect(ENV["RUBYOPT"].split(" ")).to start_with("-r#{lib}/bundler/setup")
       end
     end
 
@@ -402,9 +394,8 @@ RSpec.describe Bundler::SharedHelpers do
 
       it "sets BUNDLE_BIN_PATH to the bundle executable file" do
         subject.set_bundle_environment
-        bundle_exe = ruby_core? ? "../../../../bin/bundle" : "../../../exe/bundle"
         bin_path = ENV["BUNDLE_BIN_PATH"]
-        expect(bin_path).to eq(File.expand_path(bundle_exe, __FILE__))
+        expect(bin_path).to eq(bindir.join("bundle").to_s)
         expect(File.exist?(bin_path)).to be true
       end
     end
diff --git a/spec/commands/clean_spec.rb b/spec/commands/clean_spec.rb
index 2243a72b3d..d3161c3adb 100644
--- a/spec/commands/clean_spec.rb
+++ b/spec/commands/clean_spec.rb
@@ -361,8 +361,7 @@ RSpec.describe "bundle clean" do
       gem "rack"
     G
 
-    gem = ruby_core? ? ENV["BUNDLE_GEM"] : "gem"
-    sys_exec! "#{gem} list"
+    gem_command! :list
     expect(out).to include("rack (1.0.0)").and include("thin (1.0)")
   end
 
@@ -484,8 +483,7 @@ RSpec.describe "bundle clean" do
     end
     bundle! :update, :all => true
 
-    gem = ruby_core? ? ENV["BUNDLE_GEM"] : "gem"
-    sys_exec! "#{gem} list"
+    gem_command! :list
     expect(out).to include("foo (1.0.1, 1.0)")
   end
 
@@ -509,8 +507,7 @@ RSpec.describe "bundle clean" do
     bundle "clean --force"
 
     expect(out).to include("Removing foo (1.0)")
-    gem = ruby_core? ? ENV["BUNDLE_GEM"] : "gem"
-    sys_exec "#{gem} list"
+    gem_command! :list
     expect(out).not_to include("foo (1.0)")
     expect(out).to include("rack (1.0.0)")
   end
@@ -544,8 +541,7 @@ RSpec.describe "bundle clean" do
       expect(err).to include(system_gem_path.to_s)
       expect(err).to include("grant write permissions")
 
-      gem = ruby_core? ? ENV["BUNDLE_GEM"] : "gem"
-      sys_exec "#{gem} list"
+      gem_command! :list
       expect(out).to include("foo (1.0)")
       expect(out).to include("rack (1.0.0)")
     end
diff --git a/spec/commands/exec_spec.rb b/spec/commands/exec_spec.rb
index bb13b1b086..c68a9c4f43 100644
--- a/spec/commands/exec_spec.rb
+++ b/spec/commands/exec_spec.rb
@@ -279,12 +279,7 @@ RSpec.describe "bundle exec" do
     G
 
     rubyopt = ENV["RUBYOPT"]
-    setup_path = if ruby_core?
-      File.expand_path("../../../lib/bundler/setup", __dir__)
-    else
-      File.expand_path("../../lib/bundler/setup", __dir__)
-    end
-    rubyopt = "-r#{setup_path} #{rubyopt}"
+    rubyopt = "-r#{lib}/bundler/setup #{rubyopt}"
 
     bundle "exec 'echo $RUBYOPT'"
     expect(out).to have_rubyopts(rubyopt)
@@ -299,7 +294,7 @@ RSpec.describe "bundle exec" do
     G
 
     rubylib = ENV["RUBYLIB"]
-    rubylib = rubylib.to_s.split(File::PATH_SEPARATOR).unshift bundler_path.to_s
+    rubylib = rubylib.to_s.split(File::PATH_SEPARATOR).unshift lib.to_s
     rubylib = rubylib.uniq.join(File::PATH_SEPARATOR)
 
     bundle "exec 'echo $RUBYLIB'"
diff --git a/spec/commands/newgem_spec.rb b/spec/commands/newgem_spec.rb
index f4cd29edbc..beb21f9c2b 100644
--- a/spec/commands/newgem_spec.rb
+++ b/spec/commands/newgem_spec.rb
@@ -217,9 +217,7 @@ RSpec.describe "bundle gem" do
     prepare_gemspec(bundled_app("newgem", "newgem.gemspec"))
 
     Dir.chdir(bundled_app("newgem")) do
-      gems = ["rake-12.3.2", :bundler]
-      # for Ruby core repository, Ruby 2.6+ has bundler as standard library.
-      gems.delete(:bundler) if ruby_core?
+      gems = ["rake-12.3.2"]
       system_gems gems, :path => :bundle_path
       bundle! "exec rake build"
     end
@@ -311,8 +309,7 @@ RSpec.describe "bundle gem" do
     end
 
     it "sets a minimum ruby version" do
-      gemspec_path = ruby_core? ? "../../../lib/bundler" : "../.."
-      bundler_gemspec = Bundler::GemHelper.new(File.expand_path(gemspec_path, __dir__)).gemspec
+      bundler_gemspec = Bundler::GemHelper.new(gemspec_dir).gemspec
 
       expect(bundler_gemspec.required_ruby_version).to eq(generated_gemspec.required_ruby_version)
     end
diff --git a/spec/quality_spec.rb b/spec/quality_spec.rb
index 7cd04fdc75..40c0b0366e 100644
--- a/spec/quality_spec.rb
+++ b/spec/quality_spec.rb
@@ -108,8 +108,7 @@ RSpec.describe "The library itself" do
     exempt = /\.gitmodules|fixtures|vendor|LICENSE|vcr_cassettes|rbreadline\.diff|\.txt$/
     error_messages = []
     Dir.chdir(root) do
-      files = ruby_core? ? `git ls-files -z -- lib/bundler lib/bundler.rb spec/bundler` : `git ls-files -z`
-      files.split("\x0").each do |filename|
+      tracked_files.split("\x0").each do |filename|
         next if filename =~ exempt
         error_messages << check_for_tab_characters(filename)
         error_messages << check_for_extra_spaces(filename)
@@ -122,8 +121,7 @@ RSpec.describe "The library itself" do
     exempt = /vendor|vcr_cassettes|LICENSE|rbreadline\.diff/
     error_messages = []
     Dir.chdir(root) do
-      files = ruby_core? ? `git ls-files -z -- lib/bundler lib/bundler.rb spec/bundler` : `git ls-files -z`
-      files.split("\x0").each do |filename|
+      tracked_files.split("\x0").each do |filename|
         next if filename =~ exempt
         error_messages << check_for_straneous_quotes(filename)
       end
@@ -135,8 +133,7 @@ RSpec.describe "The library itself" do
     exempt = %r{quality_spec.rb|support/helpers|vcr_cassettes|\.md|\.ronn|\.txt|\.5|\.1}
     error_messages = []
     Dir.chdir(root) do
-      files = ruby_core? ? `git ls-files -z -- lib/bundler lib/bundler.rb spec/bundler` : `git ls-files -z`
-      files.split("\x0").each do |filename|
+      tracked_files.split("\x0").each do |filename|
         next if filename =~ exempt
         error_messages << check_for_debugging_mechanisms(filename)
       end
@@ -148,8 +145,7 @@ RSpec.describe "The library itself" do
     error_messages = []
     exempt = %r{lock/lockfile_spec|quality_spec|vcr_cassettes|\.ronn|lockfile_parser\.rb}
     Dir.chdir(root) do
-      files = ruby_core? ? `git ls-files -z -- lib/bundler lib/bundler.rb spec/bundler` : `git ls-files -z`
-      files.split("\x0").each do |filename|
+      tracked_files.split("\x0").each do |filename|
         next if filename =~ exempt
         error_messages << check_for_git_merge_conflicts(filename)
       end
@@ -174,8 +170,7 @@ RSpec.describe "The library itself" do
     error_messages = []
     exempt = /vendor|vcr_cassettes/
     Dir.chdir(root) do
-      lib_files = ruby_core? ? `git ls-files -z -- lib/bundler lib/bundler.rb` : `git ls-files -z -- lib`
-      lib_files.split("\x0").each do |filename|
+      lib_tracked_files.split("\x0").each do |filename|
         next if filename =~ exempt
         error_messages << check_for_expendable_words(filename)
         error_messages << check_for_specific_pronouns(filename)
@@ -204,8 +199,7 @@ RSpec.describe "The library itself" do
 
     Dir.chdir(root) do
       key_pattern = /([a-z\._-]+)/i
-      lib_files = ruby_core? ? `git ls-files -z -- lib/bundler lib/bundler.rb` : `git ls-files -z -- lib`
-      lib_files.split("\x0").each do |filename|
+      lib_tracked_files.split("\x0").each do |filename|
         each_line(filename) do |line, number|
           line.scan(/Bundler\.settings\[:#{key_pattern}\]/).flatten.each {|s| all_settings[s] << "referenced at `#{filename}:#{number.succ}`" }
         end
@@ -231,23 +225,8 @@ RSpec.describe "The library itself" do
   end
 
   it "can still be built" do
-    Dir.chdir(root) do
-      begin
-        if ruby_core?
-          spec = Gem::Specification.load(gemspec.to_s)
-          spec.bindir = "libexec"
-          File.open(root.join("bundler.gemspec").to_s, "w") {|f| f.write spec.to_ruby }
-          gem_command! :build, root.join("bundler.gemspec").to_s
-          FileUtils.rm(root.join("bundler.gemspec").to_s)
-        else
-          gem_command! :build, gemspec
-        end
-
-        expect(err).to be_empty, "bundler should build as a gem without warnings, but\n#{err}"
-      ensure
-        # clean up the .gem generated
-        FileUtils.rm("bundler-#{Bundler::VERSION}.gem")
-      end
+    with_built_bundler do |_gem_path|
+      expect(err).to be_empty, "bundler should build as a gem without warnings, but\n#{err}"
     end
   end
 
@@ -271,12 +250,11 @@ RSpec.describe "The library itself" do
         lib/bundler/vlad.rb
         lib/bundler/templates/gems.rb
       ]
-      lib_files = ruby_core? ? `git ls-files -z -- lib/bundler lib/bundler.rb` : `git ls-files -z -- lib`
-      lib_files = lib_files.split("\x0").grep(/\.rb$/) - exclusions
-      lib_files.reject! {|f| f.start_with?("lib/bundler/vendor") }
-      lib_files.map! {|f| f.chomp(".rb") }
+      files_to_require = lib_tracked_files.split("\x0").grep(/\.rb$/) - exclusions
+      files_to_require.reject! {|f| f.start_with?("lib/bundler/vendor") }
+      files_to_require.map! {|f| f.chomp(".rb") }
       sys_exec!("ruby -w -Ilib") do |input, _, _|
-        lib_files.each do |f|
+        files_to_require.each do |f|
           input.puts "require '#{f.sub(%r{\Alib/}, "")}'"
         end
       end
@@ -293,8 +271,7 @@ RSpec.describe "The library itself" do
     Dir.chdir(root) do
       exempt = %r{templates/|vendor/}
       all_bad_requires = []
-      lib_files = ruby_core? ? `git ls-files -z -- lib/bundler lib/bundler.rb` : `git ls-files -z -- lib`
-      lib_files.split("\x0").each do |filename|
+      lib_tracked_files.split("\x0").each do |filename|
         next if filename =~ exempt
         each_line(filename) do |line, number|
           line.scan(/^ *require "bundler/).each { all_bad_requires << "#{filename}:#{number.succ}" }
diff --git a/spec/runtime/gem_tasks_spec.rb b/spec/runtime/gem_tasks_spec.rb
index eb9db56ead..af645c8ef1 100644
--- a/spec/runtime/gem_tasks_spec.rb
+++ b/spec/runtime/gem_tasks_spec.rb
@@ -11,7 +11,7 @@ RSpec.describe "require 'bundler/gem_tasks'" do
     end
     bundled_app("Rakefile").open("w") do |f|
       f.write <<-RAKEFILE
-        $:.unshift("#{bundler_path}")
+        $:.unshift("#{lib}")
         require "bundler/gem_tasks"
       RAKEFILE
     end
@@ -19,7 +19,7 @@ RSpec.describe "require 'bundler/gem_tasks'" do
 
   it "includes the relevant tasks" do
     with_gem_path_as(Spec::Path.base_system_gems.to_s) do
-      sys_exec "#{rake} -T", "RUBYOPT" => "-I#{bundler_path}"
+      sys_exec "#{rake} -T", "RUBYOPT" => "-I#{lib}"
     end
 
     expect(err).to eq("")
diff --git a/spec/runtime/setup_spec.rb b/spec/runtime/setup_spec.rb
index 866f4862c5..65cb006edd 100644
--- a/spec/runtime/setup_spec.rb
+++ b/spec/runtime/setup_spec.rb
@@ -114,7 +114,7 @@ RSpec.describe "Bundler.setup" do
     def clean_load_path(lp)
       without_bundler_load_path = ruby!("puts $LOAD_PATH").split("\n")
       lp -= without_bundler_load_path
-      lp.map! {|p| p.sub(/^#{Regexp.union system_gem_path.to_s, default_bundle_path.to_s, bundler_path.to_s}/i, "") }
+      lp.map! {|p| p.sub(/^#{Regexp.union system_gem_path.to_s, default_bundle_path.to_s, lib.to_s}/i, "") }
     end
 
     it "puts loaded gems after -I and RUBYLIB", :ruby_repo do
@@ -773,7 +773,7 @@ end
     full_gem_name = gem_name + "-1.0"
     ext_dir = File.join(tmp("extensions", full_gem_name))
 
-    install_gem full_gem_name
+    install_gems full_gem_name
 
     install_gemfile <<-G
       source "#{file_uri_for(gem_repo1)}"
@@ -804,7 +804,6 @@ end
   context "with bundler is located in symlinked GEM_HOME" do
     let(:gem_home) { Dir.mktmpdir }
     let(:symlinked_gem_home) { Tempfile.new("gem_home").path }
-    let(:bundler_dir) { ruby_core? ? File.expand_path("../../../..", __FILE__) : File.expand_path("../../..", __FILE__) }
     let(:full_name) { "bundler-#{Bundler::VERSION}" }
 
     before do
@@ -814,15 +813,14 @@ end
       Dir.mkdir(gems_dir)
       Dir.mkdir(specifications_dir)
 
-      FileUtils.ln_s(bundler_dir, File.join(gems_dir, full_name))
+      FileUtils.ln_s(root, File.join(gems_dir, full_name))
 
-      gemspec_file = ruby_core? ? "#{bundler_dir}/lib/bundler/bundler.gemspec" : "#{bundler_dir}/bundler.gemspec"
-      gemspec = File.binread(gemspec_file).
-                sub("Bundler::VERSION", %("#{Bundler::VERSION}"))
-      gemspec = gemspec.lines.reject {|line| line =~ %r{lib/bundler/version} }.join
+      gemspec_content = File.binread(gemspec).
+                sub("Bundler::VERSION", %("#{Bundler::VERSION}")).
+                lines.reject {|line| line =~ %r{lib/bundler/version} }.join
 
       File.open(File.join(specifications_dir, "#{full_name}.gemspec"), "wb") do |f|
-        f.write(gemspec)
+        f.write(gemspec_content)
       end
     end
 
diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb
index 02ec3c790b..9cd468dfa1 100644
--- a/spec/support/helpers.rb
+++ b/spec/support/helpers.rb
@@ -95,10 +95,6 @@ module Spec
       run(cmd, *args)
     end
 
-    def lib
-      root.join("lib")
-    end
-
     def spec
       spec_dir.to_s
     end
@@ -196,7 +192,6 @@ module Spec
     end
 
     def gembin(cmd)
-      lib = File.expand_path("../../../lib", __FILE__)
       old = ENV["RUBYOPT"]
       ENV["RUBYOPT"] = "#{ENV["RUBYOPT"]} -I#{lib}"
       cmd = bundled_app("bin/#{cmd}") unless cmd.to_s.include?("/")
@@ -205,13 +200,8 @@ module Spec
       ENV["RUBYOPT"] = old
     end
 
-    def gem_command(command, args = "", options = {})
-      if command == :exec && !options[:no_quote]
-        args = args.gsub(/(?=")/, "\\")
-        args = %("#{args}")
-      end
-      gem = ENV["BUNDLE_GEM"] || "#{Gem.ruby} -rrubygems -S gem --backtrace"
-      sys_exec("#{gem} #{command} #{args}")
+    def gem_command(command, args = "")
+      sys_exec("#{Path.gem_bin} #{command} #{args}")
     end
     bang :gem_command
 
@@ -307,32 +297,35 @@ module Spec
       options = gems.last.is_a?(Hash) ? gems.pop : {}
       gem_repo = options.fetch(:gem_repo) { gem_repo1 }
       gems.each do |g|
-        path = if g == :bundler
-          if ruby_core?
-            spec = Gem::Specification.load(gemspec.to_s)
-            spec.bindir = "libexec"
-            File.open(root.join("bundler.gemspec").to_s, "w") {|f| f.write spec.to_ruby }
-            Dir.chdir(root) { gem_command! :build, root.join("bundler.gemspec").to_s }
-            FileUtils.rm(root.join("bundler.gemspec"))
-          else
-            Dir.chdir(root) { gem_command! :build, gemspec.to_s }
-          end
-          bundler_path = root + "bundler-#{Bundler::VERSION}.gem"
+        if g == :bundler
+          with_built_bundler {|gem_path| install_gem(gem_path) }
         elsif g.to_s =~ %r{\A(?:[A-Z]:)?/.*\.gem\z}
-          g
+          install_gem(g)
         else
-          "#{gem_repo}/gems/#{g}.gem"
+          install_gem("#{gem_repo}/gems/#{g}.gem")
         end
+      end
+    end
 
-        raise "OMG `#{path}` does not exist!" unless File.exist?(path)
+    def install_gem(path)
+      raise "OMG `#{path}` does not exist!" unless File.exist?(path)
 
-        gem_command! :install, "--no-document --ignore-dependencies '#{path}'"
+      gem_command! :install, "--no-document --ignore-dependencies '#{path}'"
+    end
 
-        bundler_path && bundler_path.rmtree
+    def with_built_bundler
+      with_root_gemspec do |gemspec|
+        Dir.chdir(root) { gem_command! :build, gemspec.to_s }
       end
-    end
 
-    alias_method :install_gem, :install_gems
+      bundler_path = root + "bundler-#{Bundler::VERSION}.gem"
+
+      begin
+        yield(bundler_path)
+      ensure
+        bundler_path.rmtree
+      end
+    end
 
     def with_gem_path_as(path)
       backup = ENV.to_hash
diff --git a/spec/support/path.rb b/spec/support/path.rb
index b722ec498b..9493c5422a 100644
--- a/spec/support/path.rb
+++ b/spec/support/path.rb
@@ -13,14 +13,30 @@ module Spec
       @gemspec ||= root.join(ruby_core? ? "lib/bundler/bundler.gemspec" : "bundler.gemspec")
     end
 
+    def gemspec_dir
+      @gemspec_dir ||= gemspec.parent
+    end
+
     def bindir
       @bindir ||= root.join(ruby_core? ? "libexec" : "exe")
     end
 
+    def gem_bin
+      @gem_bin ||= ruby_core? ? ENV["BUNDLE_GEM"] : "#{Gem.ruby} -S gem --backtrace"
+    end
+
     def spec_dir
       @spec_dir ||= root.join(ruby_core? ? "spec/bundler" : "spec")
     end
 
+    def tracked_files
+      @tracked_files ||= ruby_core? ? `git ls-files -z -- lib/bundler lib/bundler.rb spec/bundler` : `git ls-files -z`
+    end
+
+    def lib_tracked_files
+      @lib_tracked_files ||= ruby_core? ? `git ls-files -z -- lib/bundler lib/bundler.rb` : `git ls-files -z -- lib`
+    end
+
     def tmp(*path)
       root.join("tmp", *path)
     end
@@ -104,7 +120,7 @@ module Spec
       tmp("libs", *args)
     end
 
-    def bundler_path
+    def lib
       root.join("lib")
     end
 
@@ -120,6 +136,19 @@ module Spec
       tmp "tmpdir", *args
     end
 
+    def with_root_gemspec
+      if ruby_core?
+        root_gemspec = root.join("bundler.gemspec")
+        spec = Gem::Specification.load(gemspec.to_s)
+        spec.bindir = "libexec"
+        File.open(root_gemspec.to_s, "w") {|f| f.write spec.to_ruby }
+        yield(root_gemspec)
+        FileUtils.rm(root_gemspec)
+      else
+        yield(gemspec)
+      end
+    end
+
     def ruby_core?
       # avoid to wornings
       @ruby_core ||= nil
diff --git a/spec/support/rubygems_ext.rb b/spec/support/rubygems_ext.rb
index 6ca51bee16..aa04c3d00e 100644
--- a/spec/support/rubygems_ext.rb
+++ b/spec/support/rubygems_ext.rb
@@ -81,7 +81,7 @@ module Spec
       no_reqs.map!(&:first)
       reqs.map! {|name, req| "'#{name}:#{req}'" }
       deps = reqs.concat(no_reqs).join(" ")
-      gem = Spec::Path.ruby_core? ? ENV["BUNDLE_GEM"] : "#{Gem.ruby} -S gem"
+      gem = Path.gem_bin
       cmd = "#{gem} install #{deps} --no-document --conservative"
       puts cmd
       system(cmd) || raise("Installing gems #{deps} for the tests to use failed!")