[rubygems/rubygems] 🐛 Specification of branch or ref with tag is ambiguous

- Specs for GitProxy were incorrect and insufficient
- Specs are now correct and less insufficient

https://github.com/rubygems/rubygems/commit/63d0a8cfd0

lib/bundler/source/git/git_proxy.rb

@@ -43,6 +43,13 @@ module Bundler
         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 @@ module Bundler
         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

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

@@ -3,34 +3,84 @@
 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:p@github.com/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:p@github.com/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:p@github.com/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:p@github.com/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:p@github.com/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
@@ -38,10 +88,10 @@ RSpec.describe Bundler::Source::Git::GitProxy do
     it "keeps original userinfo" do
       Bundler.settings.temporary("github.com" => "u:p") do
         original = "https://orig:info@github.com/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
@@ -49,46 +99,46 @@ RSpec.describe Bundler::Source::Git::GitProxy do
   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
@@ -96,34 +146,34 @@ RSpec.describe Bundler::Source::Git::GitProxy do
   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