diff options
author | The Bundler Bot <bot@bundler.io> | 2018-07-03 03:44:34 +0000 |
---|---|---|
committer | The Bundler Bot <bot@bundler.io> | 2018-07-03 03:44:34 +0000 |
commit | 25b76e5e12f861c4f75e726d993d5ba55810deef (patch) | |
tree | 301af257b349e2485870bfdaeaac963ef7186187 | |
parent | 233dc13e6ccafcb5b15fafe35e96c598b9ffad6d (diff) | |
parent | aacba5022f87ec0c8c3da1c8a7a9f60e7e6d2fcb (diff) | |
download | bundler-25b76e5e12f861c4f75e726d993d5ba55810deef.tar.gz |
Auto merge of #6447 - agrim123:agr-update-error-message, r=segiddins
Update error message on bundle add in case of duplication
### What was the end-user problem that led to this PR?
If the user tries to add a gem giving version requirement then bundler throws an error if the gem is already present.
### What was your diagnosis of the problem?
The error displayed is very descriptive but if the user is specifying a gem version maybe he wants to update the gem.
### What is your fix for the problem, implemented in this PR?
Added an instructive message to inform the user that gem can also be updated.
### Why did you choose this fix out of the possible options?
This seemed to be the best informative message.
Closes #6341
-rw-r--r-- | lib/bundler/dsl.rb | 17 | ||||
-rw-r--r-- | lib/bundler/injector.rb | 11 | ||||
-rw-r--r-- | spec/commands/add_spec.rb | 42 | ||||
-rw-r--r-- | spec/commands/install_spec.rb | 50 |
4 files changed, 115 insertions, 5 deletions
diff --git a/lib/bundler/dsl.rb b/lib/bundler/dsl.rb index 7467b5a537..80d208e463 100644 --- a/lib/bundler/dsl.rb +++ b/lib/bundler/dsl.rb @@ -107,13 +107,28 @@ module Bundler if current.requirement != dep.requirement unless deleted_dep return if dep.type == :development + + update_prompt = "" + + if File.basename(@gemfile) == Injector::INJECTED_GEMS + if dep.requirements_list.include?(">= 0") && !current.requirements_list.include?(">= 0") + update_prompt = ". Gem already added" + else + update_prompt = ". If you want to update the gem version, run `bundle update #{current.name}`" + + update_prompt += ". You may also need to change the version requirement specified in the Gemfile if it's too restrictive." unless current.requirements_list.include?(">= 0") + end + end + raise GemfileError, "You cannot specify the same gem twice with different version requirements.\n" \ - "You specified: #{current.name} (#{current.requirement}) and #{dep.name} (#{dep.requirement})" + "You specified: #{current.name} (#{current.requirement}) and #{dep.name} (#{dep.requirement})" \ + "#{update_prompt}" end else Bundler.ui.warn "Your Gemfile lists the gem #{current.name} (#{current.requirement}) more than once.\n" \ "You should probably keep only one of them.\n" \ + "Remove any duplicate entries and specify the gem only once (per group).\n" \ "While it's not a problem now, it could cause errors if you change the version of one of them later." end diff --git a/lib/bundler/injector.rb b/lib/bundler/injector.rb index 5bbb6aeae0..0eae0c6ece 100644 --- a/lib/bundler/injector.rb +++ b/lib/bundler/injector.rb @@ -2,8 +2,10 @@ module Bundler class Injector - def self.inject(deps, options = {}) - injector = new(deps, options) + INJECTED_GEMS = "injected gems".freeze + + def self.inject(new_deps, options = {}) + injector = new(new_deps, options) injector.inject(Bundler.default_gemfile, Bundler.default_lockfile) end @@ -36,8 +38,9 @@ module Bundler @deps -= builder.dependencies # add new deps to the end of the in-memory Gemfile - # Set conservative versioning to false because we want to let the resolver resolve the version first - builder.eval_gemfile("injected gems", build_gem_lines(false)) if @deps.any? + # Set conservative versioning to false because + # we want to let the resolver resolve the version first + builder.eval_gemfile(INJECTED_GEMS, build_gem_lines(false)) if @deps.any? # resolve to see if the new deps broke anything @definition = builder.to_definition(lockfile_path, {}) diff --git a/spec/commands/add_spec.rb b/spec/commands/add_spec.rb index 5d7e5a30a6..2e9ef6b923 100644 --- a/spec/commands/add_spec.rb +++ b/spec/commands/add_spec.rb @@ -172,4 +172,46 @@ RSpec.describe "bundle add" do expect(out).to include("You specified: weakling (~> 0.0.1) and weakling (>= 0).") end end + + describe "when a gem is added which is already specified in Gemfile with version" do + it "shows an error when added with different version requirement" do + install_gemfile <<-G + source "file://#{gem_repo2}" + gem "rack", "1.0" + G + + bundle "add 'rack' --version=1.1" + + expect(out).to include("You cannot specify the same gem twice with different version requirements") + expect(out).to include("If you want to update the gem version, run `bundle update rack`. You may also need to change the version requirement specified in the Gemfile if it's too restrictive") + end + + it "shows error when added without version requirements" do + install_gemfile <<-G + source "file://#{gem_repo2}" + gem "rack", "1.0" + G + + bundle "add 'rack'" + + expect(out).to include("Gem already added.") + expect(out).to include("You cannot specify the same gem twice with different version requirements") + expect(out).not_to include("If you want to update the gem version, run `bundle update rack`. You may also need to change the version requirement specified in the Gemfile if it's too restrictive") + end + end + + describe "when a gem is added which is already specified in Gemfile without version" do + it "shows an error when added with different version requirement" do + install_gemfile <<-G + source "file://#{gem_repo2}" + gem "rack" + G + + bundle "add 'rack' --version=1.1" + + expect(out).to include("You cannot specify the same gem twice with different version requirements") + expect(out).to include("If you want to update the gem version, run `bundle update rack`.") + expect(out).not_to include("You may also need to change the version requirement specified in the Gemfile if it's too restrictive") + end + end end diff --git a/spec/commands/install_spec.rb b/spec/commands/install_spec.rb index 85593ee0ff..394f672fef 100644 --- a/spec/commands/install_spec.rb +++ b/spec/commands/install_spec.rb @@ -321,6 +321,56 @@ RSpec.describe "bundle install with gem sources" do expect(File.exist?(bundled_app("Gemfile.lock"))).to eq(true) end + context "throws a warning if a gem is added twice in Gemfile" do + it "without version requirements" do + install_gemfile <<-G + source "file://#{gem_repo2}" + gem "rack" + gem "rack" + G + + expect(out).to include("Your Gemfile lists the gem rack (>= 0) more than once.") + expect(out).to include("Remove any duplicate entries and specify the gem only once (per group).") + expect(out).to include("While it's not a problem now, it could cause errors if you change the version of one of them later.") + end + + it "with same versions" do + install_gemfile <<-G + source "file://#{gem_repo2}" + gem "rack", "1.0" + gem "rack", "1.0" + G + + expect(out).to include("Your Gemfile lists the gem rack (= 1.0) more than once.") + expect(out).to include("Remove any duplicate entries and specify the gem only once (per group).") + expect(out).to include("While it's not a problem now, it could cause errors if you change the version of one of them later.") + end + end + + context "throws an error if a gem is added twice in Gemfile" do + it "when version of one dependency is not specified" do + install_gemfile <<-G + source "file://#{gem_repo2}" + gem "rack" + gem "rack", "1.0" + G + + expect(out).to include("You cannot specify the same gem twice with different version requirements") + expect(out).to include("You specified: rack (>= 0) and rack (= 1.0).") + end + + it "when different versions of both dependencies are specified" do + install_gemfile <<-G + source "file://#{gem_repo2}" + gem "rack", "1.0" + gem "rack", "1.1" + G + + expect(out).to include("You cannot specify the same gem twice with different version requirements") + expect(out).to include("You specified: rack (= 1.0) and rack (= 1.1).") + end + end + it "gracefully handles error when rubygems server is unavailable" do install_gemfile <<-G, :artifice => nil source "file://#{gem_repo1}" |