From c8606361f7e08dc2025c17318d0c225dbfd86e82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Thu, 21 Dec 2023 15:18:54 +0100 Subject: [PATCH 1/2] Remove indirection a bit It's a bit of a mess right now, just trying to make it a bit easier to parse. --- bundler/lib/bundler/dependency.rb | 4 ++++ bundler/lib/bundler/dsl.rb | 10 +++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/bundler/lib/bundler/dependency.rb b/bundler/lib/bundler/dependency.rb index efc39a12f3ef..77d7a00362b9 100644 --- a/bundler/lib/bundler/dependency.rb +++ b/bundler/lib/bundler/dependency.rb @@ -68,6 +68,10 @@ def should_include? @should_include && current_env? && current_platform? end + def gemspec_dev_dep? + type == :development + end + def current_env? return true unless @env if @env.is_a?(Hash) diff --git a/bundler/lib/bundler/dsl.rb b/bundler/lib/bundler/dsl.rb index a67f7ed8a1ec..a570943bf2bf 100644 --- a/bundler/lib/bundler/dsl.rb +++ b/bundler/lib/bundler/dsl.rb @@ -103,13 +103,13 @@ def gem(name, *args) # if there's already a dependency with this name we try to prefer one if current = @dependencies.find {|d| d.name == dep.name } # Always prefer the dependency from the Gemfile - deleted_dep = @dependencies.delete(current) if current.type == :development + @dependencies.delete(current) if current.gemspec_dev_dep? if current.requirement != dep.requirement current_requirement_open = current.requirements_list.include?(">= 0") - if current.type == :development - unless current_requirement_open || dep.type == :development + if current.gemspec_dev_dep? + unless current_requirement_open || dep.gemspec_dev_dep? Bundler.ui.warn "A gemspec development dependency (#{dep.name}, #{current.requirement}) is being overridden by a Gemfile dependency (#{dep.name}, #{dep.requirement}).\n" \ "This behaviour may change in the future. Please remove either of them, or make sure they both have the same requirement\n" \ end @@ -130,8 +130,8 @@ def gem(name, *args) "You specified: #{current.name} (#{current.requirement}) and #{dep.name} (#{dep.requirement})" \ "#{update_prompt}" end - elsif current.type == :development || dep.type == :development - return if deleted_dep.nil? + elsif current.gemspec_dev_dep? || dep.gemspec_dev_dep? + return if dep.gemspec_dev_dep? elsif current.source != dep.source raise GemfileError, "You cannot specify the same gem twice coming from different sources.\n" \ "You specified that #{dep.name} (#{dep.requirement}) should come from " \ From 48589e44aaea549200f53880bc20067a40a6d02b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Thu, 21 Dec 2023 15:04:29 +0100 Subject: [PATCH 2/2] Fix incorrect error when Gemfile override a gemspec development dependency It should be only a warning. --- bundler/lib/bundler/dsl.rb | 13 ++++++++---- bundler/spec/commands/install_spec.rb | 29 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/bundler/lib/bundler/dsl.rb b/bundler/lib/bundler/dsl.rb index a570943bf2bf..1eca749617a0 100644 --- a/bundler/lib/bundler/dsl.rb +++ b/bundler/lib/bundler/dsl.rb @@ -108,11 +108,16 @@ def gem(name, *args) if current.requirement != dep.requirement current_requirement_open = current.requirements_list.include?(">= 0") - if current.gemspec_dev_dep? - unless current_requirement_open || dep.gemspec_dev_dep? - Bundler.ui.warn "A gemspec development dependency (#{dep.name}, #{current.requirement}) is being overridden by a Gemfile dependency (#{dep.name}, #{dep.requirement}).\n" \ - "This behaviour may change in the future. Please remove either of them, or make sure they both have the same requirement\n" \ + gemspec_dep = [dep, current].find(&:gemspec_dev_dep?) + if gemspec_dep + gemfile_dep = [dep, current].find(&:runtime?) + + unless current_requirement_open + Bundler.ui.warn "A gemspec development dependency (#{gemspec_dep.name}, #{gemspec_dep.requirement}) is being overridden by a Gemfile dependency (#{gemfile_dep.name}, #{gemfile_dep.requirement}).\n" \ + "This behaviour may change in the future. Please remove either of them, or make sure they both have the same requirement\n" end + + return if dep.gemspec_dev_dep? else update_prompt = "" diff --git a/bundler/spec/commands/install_spec.rb b/bundler/spec/commands/install_spec.rb index aa7c54ce4b43..7c016ff3d8a1 100644 --- a/bundler/spec/commands/install_spec.rb +++ b/bundler/spec/commands/install_spec.rb @@ -460,6 +460,35 @@ expect(the_bundle).to include_gems("rubocop 1.37.1") end + it "warns when a Gemfile dependency is overriding a gemspec development dependency, with different requirements" do + build_lib "my-gem", path: bundled_app do |s| + s.add_development_dependency "rails", ">= 5" + end + + build_repo4 do + build_gem "rails", "7.0.8" + end + + gemfile <<~G + source "#{file_uri_for(gem_repo4)}" + + gem "rails", "~> 7.0.8" + + gemspec + G + + bundle :install + + expect(err).to include("A gemspec development dependency (rails, >= 5) is being overridden by a Gemfile dependency (rails, ~> 7.0.8).") + expect(err).to include("This behaviour may change in the future. Please remove either of them, or make sure they both have the same requirement") + + # This is not the best behavior I believe, it would be better if both + # requirements are considered if they are compatible, and a version + # satisfying both is chosen. But not sure about changing it right now, so + # I went with a warning for the time being. + expect(the_bundle).to include_gems("rails 7.0.8") + end + it "does not warn if a gem is added once in Gemfile and also inside a gemspec as a development dependency, with same requirements, and different sources" do build_lib "my-gem", path: bundled_app do |s| s.add_development_dependency "activesupport"