Auto merge of #5559 - colby-swandale:gem-branch-exception, r=segiddins

print an error message when a non-git gem is given a `branch` option

When the user supplies the `branch` option to a non-git gem an error message should be printed.

Relates to #5530.

2 files changed, 24 insertions, 0 deletions

diff --git a/lib/bundler/dsl.rb b/lib/bundler/dsl.rb
index cdbae076f0..9bbabfee19 100644
--- a/lib/bundler/dsl.rb
+++ b/lib/bundler/dsl.rb
@@ -384,6 +384,12 @@ module Bundler
 
     def validate_keys(command, opts, valid_keys)
       invalid_keys = opts.keys - valid_keys
+
+      git_source = opts.keys & @git_sources.keys.map(&:to_s)
+      if opts["branch"] && !(opts["git"] || opts["github"] || git_source.any?)
+        raise GemfileError, %(The `branch` option for `#{command}` is not allowed. Only gems with a git source can specify a branch)
+      end
+
       if invalid_keys.any?
         message = String.new
         message << "You passed #{invalid_keys.map {|k| ":" + k }.join(", ")} "
diff --git a/spec/bundler/dsl_spec.rb b/spec/bundler/dsl_spec.rb
index 0561cb7ddc..4f5eb6dc92 100644
--- a/spec/bundler/dsl_spec.rb
+++ b/spec/bundler/dsl_spec.rb
@@ -144,6 +144,24 @@ RSpec.describe Bundler::Dsl do
       expect { subject.gem(:foo) }.
         to raise_error(Bundler::GemfileError, /You need to specify gem names as Strings. Use 'gem "foo"' instead/)
     end
+
+    it "rejects branch option on non-git gems" do
+      expect { subject.gem("foo", :branch => "test") }.
+        to raise_error(Bundler::GemfileError, /The `branch` option for `gem 'foo'` is not allowed. Only gems with a git source can specify a branch/)
+    end
+
+    it "allows specifiying a branch on git gems" do
+      subject.gem("foo", :branch => "test", :git => "http://mytestrepo")
+      dep = subject.dependencies.last
+      expect(dep.name).to eq "foo"
+    end
+
+    it "allows specifiying a branch on git gems with a git_source" do
+      subject.git_source(:test_source) {|n| "https://github.com/#{n}" }
+      subject.gem("foo", :branch => "test", :test_source => "bundler/bundler")
+      dep = subject.dependencies.last
+      expect(dep.name).to eq "foo"
+    end
   end
 
   describe "#gemspec" do