Merge pull request #6959 from pboling/fix-git-proxy-for-branch-and-ref

(cherry picked from commit c2c0966)

Author: martinemde

bundler/lib/bundler/cli/info.rb

@@ -33,7 +33,7 @@ def spec_for_gem(gem_name)
     def default_gem_spec(gem_name)
       return unless Gem::Specification.respond_to?(:find_all_by_name)
       gem_spec = Gem::Specification.find_all_by_name(gem_name).last
-      return gem_spec if gem_spec&.default_gem?
+      gem_spec if gem_spec&.default_gem?
     end
 
     def spec_not_found(gem_name)

bundler/lib/bundler/retry.rb

@@ -56,7 +56,7 @@ def fail_attempt(e)
     def keep_trying?
       return true  if current_run.zero?
       return false if last_attempt?
-      return true  if @failed
+      true if @failed
     end
 
     def last_attempt?

bundler/lib/bundler/source/git/git_proxy.rb

@@ -43,6 +43,13 @@ def initialize(command, destination_path, ref, repo)
         end
       end
 
+      class AmbiguousGitReference < GitError
+        def initialize(options)
+          msg = "Specification of branch or ref with tag is ambiguous. You specified #{options.inspect}"
+          super msg
+        end
+      end
+
       # The GitProxy is responsible to interact with git repositories.
       # All actions required by the Git source is encapsulated in this
       # object.
@@ -53,10 +60,15 @@ class GitProxy
         def initialize(path, uri, options = {}, revision = nil, git = nil)
           @path     = path
           @uri      = uri
-          @branch   = options["branch"]
           @tag      = options["tag"]
+          @branch   = options["branch"]
           @ref      = options["ref"]
-          @explicit_ref = branch || tag || ref
+          if @tag
+            raise AmbiguousGitReference.new(options) if @branch || @ref
+            @explicit_ref = @tag
+          else
+            @explicit_ref = @ref || @branch
+          end
           @revision = revision
           @git      = git
           @commit_ref = nil

bundler/spec/bundler/source/git/git_proxy_spec.rb

@@ -3,127 +3,177 @@
 RSpec.describe Bundler::Source::Git::GitProxy do
   let(:path) { Pathname("path") }
   let(:uri) { "https://github.com/rubygems/rubygems.git" }
-  let(:ref) { "HEAD" }
+  let(:ref) { nil }
+  let(:branch) { nil }
+  let(:tag) { nil }
+  let(:options) { { "ref" => ref, "branch" => branch, "tag" => tag }.compact }
   let(:revision) { nil }
   let(:git_source) { nil }
   let(:clone_result) { double(Process::Status, :success? => true) }
   let(:base_clone_args) { ["clone", "--bare", "--no-hardlinks", "--quiet", "--no-tags", "--depth", "1", "--single-branch"] }
-  subject { described_class.new(path, uri, ref, revision, git_source) }
+  subject(:git_proxy) { described_class.new(path, uri, options, revision, git_source) }
+
+  context "with explicit ref" do
+    context "with branch only" do
+      let(:branch) { "main" }
+      it "sets explicit ref to branch" do
+        expect(git_proxy.explicit_ref).to eq(branch)
+      end
+    end
+
+    context "with ref only" do
+      let(:ref) { "HEAD" }
+      it "sets explicit ref to ref" do
+        expect(git_proxy.explicit_ref).to eq(ref)
+      end
+    end
+
+    context "with tag only" do
+      let(:tag) { "v1.0" }
+      it "sets explicit ref to ref" do
+        expect(git_proxy.explicit_ref).to eq(tag)
+      end
+    end
+
+    context "with tag and branch" do
+      let(:tag) { "v1.0" }
+      let(:branch) { "main" }
+      it "raises error" do
+        expect { git_proxy }.to raise_error(Bundler::Source::Git::AmbiguousGitReference)
+      end
+    end
+
+    context "with tag and ref" do
+      let(:tag) { "v1.0" }
+      let(:ref) { "HEAD" }
+      it "raises error" do
+        expect { git_proxy }.to raise_error(Bundler::Source::Git::AmbiguousGitReference)
+      end
+    end
+
+    context "with branch and ref" do
+      let(:branch) { "main" }
+      let(:ref) { "HEAD" }
+      it "honors ref over branch" do
+        expect(git_proxy.explicit_ref).to eq(ref)
+      end
+    end
+  end
 
   context "with configured credentials" do
     it "adds username and password to URI" do
       Bundler.settings.temporary(uri => "u:p") do
-        allow(subject).to receive(:git_local).with("--version").and_return("git version 2.14.0")
-        expect(subject).to receive(:capture).with([*base_clone_args, "--", "https://u:[email protected]/rubygems/rubygems.git", path.to_s], nil).and_return(["", "", clone_result])
+        allow(git_proxy).to receive(:git_local).with("--version").and_return("git version 2.14.0")
+        expect(git_proxy).to receive(:capture).with([*base_clone_args, "--", "https://u:[email protected]/rubygems/rubygems.git", path.to_s], nil).and_return(["", "", clone_result])
         subject.checkout
       end
     end
 
     it "adds username and password to URI for host" do
       Bundler.settings.temporary("github.com" => "u:p") do
-        allow(subject).to receive(:git_local).with("--version").and_return("git version 2.14.0")
-        expect(subject).to receive(:capture).with([*base_clone_args, "--", "https://u:[email protected]/rubygems/rubygems.git", path.to_s], nil).and_return(["", "", clone_result])
+        allow(git_proxy).to receive(:git_local).with("--version").and_return("git version 2.14.0")
+        expect(git_proxy).to receive(:capture).with([*base_clone_args, "--", "https://u:[email protected]/rubygems/rubygems.git", path.to_s], nil).and_return(["", "", clone_result])
         subject.checkout
       end
     end
 
     it "does not add username and password to mismatched URI" do
       Bundler.settings.temporary("https://u:[email protected]/rubygems/rubygems-mismatch.git" => "u:p") do
-        allow(subject).to receive(:git_local).with("--version").and_return("git version 2.14.0")
-        expect(subject).to receive(:capture).with([*base_clone_args, "--", uri, path.to_s], nil).and_return(["", "", clone_result])
+        allow(git_proxy).to receive(:git_local).with("--version").and_return("git version 2.14.0")
+        expect(git_proxy).to receive(:capture).with([*base_clone_args, "--", uri, path.to_s], nil).and_return(["", "", clone_result])
         subject.checkout
       end
     end
 
     it "keeps original userinfo" do
       Bundler.settings.temporary("github.com" => "u:p") do
         original = "https://orig:[email protected]/rubygems/rubygems.git"
-        subject = described_class.new(Pathname("path"), original, "HEAD")
-        allow(subject).to receive(:git_local).with("--version").and_return("git version 2.14.0")
-        expect(subject).to receive(:capture).with([*base_clone_args, "--", original, path.to_s], nil).and_return(["", "", clone_result])
-        subject.checkout
+        git_proxy = described_class.new(Pathname("path"), original, options)
+        allow(git_proxy).to receive(:git_local).with("--version").and_return("git version 2.14.0")
+        expect(git_proxy).to receive(:capture).with([*base_clone_args, "--", original, path.to_s], nil).and_return(["", "", clone_result])
+        git_proxy.checkout
       end
     end
   end
 
   describe "#version" do
     context "with a normal version number" do
       before do
-        expect(subject).to receive(:git_local).with("--version").
+        expect(git_proxy).to receive(:git_local).with("--version").
           and_return("git version 1.2.3")
       end
 
       it "returns the git version number" do
-        expect(subject.version).to eq("1.2.3")
+        expect(git_proxy.version).to eq("1.2.3")
       end
 
       it "does not raise an error when passed into Gem::Version.create" do
-        expect { Gem::Version.create subject.version }.not_to raise_error
+        expect { Gem::Version.create git_proxy.version }.not_to raise_error
       end
     end
 
     context "with a OSX version number" do
       before do
-        expect(subject).to receive(:git_local).with("--version").
+        expect(git_proxy).to receive(:git_local).with("--version").
           and_return("git version 1.2.3 (Apple Git-BS)")
       end
 
       it "strips out OSX specific additions in the version string" do
-        expect(subject.version).to eq("1.2.3")
+        expect(git_proxy.version).to eq("1.2.3")
       end
 
       it "does not raise an error when passed into Gem::Version.create" do
-        expect { Gem::Version.create subject.version }.not_to raise_error
+        expect { Gem::Version.create git_proxy.version }.not_to raise_error
       end
     end
 
     context "with a msysgit version number" do
       before do
-        expect(subject).to receive(:git_local).with("--version").
+        expect(git_proxy).to receive(:git_local).with("--version").
           and_return("git version 1.2.3.msysgit.0")
       end
 
       it "strips out msysgit specific additions in the version string" do
-        expect(subject.version).to eq("1.2.3")
+        expect(git_proxy.version).to eq("1.2.3")
       end
 
       it "does not raise an error when passed into Gem::Version.create" do
-        expect { Gem::Version.create subject.version }.not_to raise_error
+        expect { Gem::Version.create git_proxy.version }.not_to raise_error
       end
     end
   end
 
   describe "#full_version" do
     context "with a normal version number" do
       before do
-        expect(subject).to receive(:git_local).with("--version").
+        expect(git_proxy).to receive(:git_local).with("--version").
           and_return("git version 1.2.3")
       end
 
       it "returns the git version number" do
-        expect(subject.full_version).to eq("1.2.3")
+        expect(git_proxy.full_version).to eq("1.2.3")
       end
     end
 
     context "with a OSX version number" do
       before do
-        expect(subject).to receive(:git_local).with("--version").
+        expect(git_proxy).to receive(:git_local).with("--version").
           and_return("git version 1.2.3 (Apple Git-BS)")
       end
 
       it "does not strip out OSX specific additions in the version string" do
-        expect(subject.full_version).to eq("1.2.3 (Apple Git-BS)")
+        expect(git_proxy.full_version).to eq("1.2.3 (Apple Git-BS)")
       end
     end
 
     context "with a msysgit version number" do
       before do
-        expect(subject).to receive(:git_local).with("--version").
+        expect(git_proxy).to receive(:git_local).with("--version").
           and_return("git version 1.2.3.msysgit.0")
       end
 
       it "does not strip out msysgit specific additions in the version string" do
-        expect(subject.full_version).to eq("1.2.3.msysgit.0")
+        expect(git_proxy.full_version).to eq("1.2.3.msysgit.0")
       end
     end
   end

bundler/spec/commands/add_spec.rb

@@ -63,7 +63,7 @@
     it "adds multiple version constraints when specified" do
       requirements = ["< 3.0", "> 1.0"]
       bundle "add 'foo' --version='#{requirements.join(", ")}'"
-      expect(bundled_app_gemfile.read).to match(/gem "foo", #{Gem::Requirement.new(requirements).as_list.map(&:dump).join(', ')}/)
+      expect(bundled_app_gemfile.read).to match(/gem "foo", #{Gem::Requirement.new(requirements).as_list.map(&:dump).join(", ")}/)
       expect(the_bundle).to include_gems "foo 2.0"
     end
   end

bundler/spec/commands/open_spec.rb

@@ -104,7 +104,7 @@
         input.puts "2"
       end
 
-      expect(out).to match(%r{bundler_editor #{default_bundle_path('gems', 'activerecord-2.3.2')}/CHANGELOG\.md\z})
+      expect(out).to match(%r{bundler_editor #{default_bundle_path("gems", "activerecord-2.3.2")}/CHANGELOG\.md\z})
     end
 
     it "opens deep subpath of the selected matching gem", :readline do
@@ -113,7 +113,7 @@
         input.puts "2"
       end
 
-      expect(out).to match(%r{bundler_editor #{default_bundle_path('gems', 'activerecord-2.3.2')}/lib/activerecord/version\.rb\z})
+      expect(out).to match(%r{bundler_editor #{default_bundle_path("gems", "activerecord-2.3.2")}/lib/activerecord/version\.rb\z})
     end
 
     it "select the gem from many match gems", :readline do
@@ -122,7 +122,7 @@
         input.puts "2"
       end
 
-      expect(out).to match(/bundler_editor #{default_bundle_path('gems', 'activerecord-2.3.2')}\z/)
+      expect(out).to match(/bundler_editor #{default_bundle_path("gems", "activerecord-2.3.2")}\z/)
     end
 
     it "allows selecting exit from many match gems", :readline do

bundler/spec/install/gemfile/gemspec_spec.rb

@@ -85,7 +85,7 @@
       source "#{file_uri_for(gem_repo2)}"
       gemspec :path => '#{tmp.join("foo")}'
     G
-    expect(err).to match(/There are no gemspecs at #{tmp.join('foo')}/)
+    expect(err).to match(/There are no gemspecs at #{tmp.join("foo")}/)
   end
 
   it "should raise if there are too many gemspecs available" do
@@ -97,7 +97,7 @@
       source "#{file_uri_for(gem_repo2)}"
       gemspec :path => '#{tmp.join("foo")}'
     G
-    expect(err).to match(/There are multiple gemspecs at #{tmp.join('foo')}/)
+    expect(err).to match(/There are multiple gemspecs at #{tmp.join("foo")}/)
   end
 
   it "should pick a specific gemspec" do

bundler/spec/install/gemfile/git_spec.rb

@@ -572,7 +572,7 @@
 
       bundle %(config set local.rack #{lib_path("local-rack")})
       bundle :install, :raise_on_error => false
-      expect(err).to match(/Cannot use local override for rack-0.8 because #{Regexp.escape(lib_path('local-rack').to_s)} does not exist/)
+      expect(err).to match(/Cannot use local override for rack-0.8 because #{Regexp.escape(lib_path("local-rack").to_s)} does not exist/)
 
       solution = "config unset local.rack"
       expect(err).to match(/Run `bundle #{solution}` to remove the local override/)
@@ -594,7 +594,7 @@
 
       bundle %(config set local.rack #{lib_path("local-rack")})
       bundle :install, :raise_on_error => false
-      expect(err).to match(/Cannot use local override for rack-0.8 at #{Regexp.escape(lib_path('local-rack').to_s)} because :branch is not specified in Gemfile/)
+      expect(err).to match(/Cannot use local override for rack-0.8 at #{Regexp.escape(lib_path("local-rack").to_s)} because :branch is not specified in Gemfile/)
 
       solution = "config unset local.rack"
       expect(err).to match(/Specify a branch or run `bundle #{solution}` to remove the local override/)

bundler/spec/runtime/setup_spec.rb

@@ -410,7 +410,7 @@ def clean_load_path(lp)
 
     it "provides a useful exception when the git repo is not checked out yet" do
       run "1", :raise_on_error => false
-      expect(err).to match(/the git source #{lib_path('rack-1.0.0')} is not yet checked out. Please run `bundle install`/i)
+      expect(err).to match(/the git source #{lib_path("rack-1.0.0")} is not yet checked out. Please run `bundle install`/i)
     end
 
     it "does not hit the git binary if the lockfile is available and up to date" do
@@ -497,7 +497,7 @@ def clean_load_path(lp)
 
       FileUtils.rm_rf(lib_path("local-rack"))
       run "require 'rack'", :raise_on_error => false
-      expect(err).to match(/Cannot use local override for rack-0.8 because #{Regexp.escape(lib_path('local-rack').to_s)} does not exist/)
+      expect(err).to match(/Cannot use local override for rack-0.8 because #{Regexp.escape(lib_path("local-rack").to_s)} does not exist/)
     end
 
     it "explodes if branch is not given on runtime" do

tool/bundler/dev_gems.rb.lock

@@ -39,6 +39,8 @@ PLATFORMS
   x64-mingw-ucrt
   x64-mingw32
   x86_64-darwin-20
+  x86_64-darwin-21
+  x86_64-darwin-22
   x86_64-linux
 
 DEPENDENCIES