From 336cccc70336d568a5419b50cf9d168fadcfa359 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Fri, 29 Mar 2024 14:43:43 -0600 Subject: [PATCH] fix not syncing dependent gems when their depending gem is synced (#31) --- lib/bundler/multilock.rb | 36 +++++++++++++++----- lib/bundler/multilock/cache.rb | 61 ++++++++++++++++++++++------------ spec/bundler/multilock_spec.rb | 45 +++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 30 deletions(-) diff --git a/lib/bundler/multilock.rb b/lib/bundler/multilock.rb index abf84b2..75ab4d6 100644 --- a/lib/bundler/multilock.rb +++ b/lib/bundler/multilock.rb @@ -234,16 +234,34 @@ def after_install_all(install: true) lockfile = cache.parser(lockfile_name) dependency_changes = false - # replace any duplicate specs with what's in the default lockfile - lockfile.specs.map! do |spec| - parent_spec = parent_specs[[spec.name, spec.platform]] - next spec unless parent_spec - - # they're conflicting on purpose; don't inherit from the parent lockfile - next spec if cache.conflicting_requirements?(lockfile_name, parent_lockfile_name, spec, parent_spec) + forced_inherited_specs = Set.new + + loop do + replaced_any = false + # replace any duplicate specs with what's in the parent lockfile + lockfile.specs.map! do |spec| + parent_spec = parent_specs[[spec.name, spec.platform]] + next spec unless parent_spec + + if cache.reverse_dependencies(lockfile_name)[spec.name].intersect?(forced_inherited_specs) || + cache.reverse_dependencies(parent_lockfile_name)[spec.name].intersect?(forced_inherited_specs) + # a conficting gem that depends on this gem was already replaced with the + # version from the parent lockfile; this gem _must_ be replaced with the parent + # lockfile's version (have to check the dependency chain from both lockfiles, + # dependencies can be introduced or removed with different versions of gems) + elsif cache.conflicting_requirements?(lockfile_name, parent_lockfile_name, spec, parent_spec) + # they're conflicting on purpose; don't inherit from the parent lockfile + next spec + end + + if !replaced_any && !dependency_changes && spec != parent_spec + replaced_any = dependency_changes = true + end + forced_inherited_specs << spec.name + parent_spec + end - dependency_changes ||= spec != parent_spec - parent_spec + break unless replaced_any end missing_specs = parent_specs.each_value.reject do |parent_spec| diff --git a/lib/bundler/multilock/cache.rb b/lib/bundler/multilock/cache.rb index b2fe613..032e8b2 100644 --- a/lib/bundler/multilock/cache.rb +++ b/lib/bundler/multilock/cache.rb @@ -11,6 +11,7 @@ def initialize @parsers = {} @specs = {} @reverse_dependencies = {} + @reverse_requirements = {} @base_checks = {} @deep_checks = {} @base_check_messages = {} @@ -29,6 +30,7 @@ def invalidate_lockfile(lockfile_name) @parsers.delete(lockfile_name) @specs.delete(lockfile_name) @reverse_dependencies.delete(lockfile_name) + @reverse_requirements.delete(lockfile_name) invalidate_checks(lockfile_name) end @@ -59,33 +61,25 @@ def specs(lockfile_name) end # @param lockfile_name [Pathname] - # @return [Hash] hash of gem name to requirement for that gem + # @return [Hash>] hash of gem name to set of gem names that depend on it def reverse_dependencies(lockfile_name) - @reverse_dependencies[lockfile_name] ||= begin - # can use Gem::Requirement.default_prelease when Ruby 2.6 support is dropped - reverse_dependencies = Hash.new { |h, k| h[k] = Gem::Requirement.new(">= 0.a") } - - lockfile = parser(lockfile_name) - - lockfile.dependencies.each_value do |spec| - reverse_dependencies[spec.name].requirements.concat(spec.requirement.requirements) - end - lockfile.specs.each do |spec| - spec.dependencies.each do |dependency| - reverse_dependencies[dependency.name].requirements.concat(dependency.requirement.requirements) - end - end + ensure_reverse_data(lockfile_name) + @reverse_dependencies[lockfile_name] + end - reverse_dependencies - end + # @param lockfile_name [Pathname] + # @return [Hash] hash of gem name to requirement for that gem + def reverse_requirements(lockfile_name) + ensure_reverse_data(lockfile_name) + @reverse_requirements[lockfile_name] end def conflicting_requirements?(lockfile1_name, lockfile2_name, spec1, spec2) - reverse_dependencies1 = reverse_dependencies(lockfile1_name)[spec1.name] - reverse_dependencies2 = reverse_dependencies(lockfile2_name)[spec1.name] + reverse_requirements1 = reverse_requirements(lockfile1_name)[spec1.name] + reverse_requirements2 = reverse_requirements(lockfile2_name)[spec1.name] - !reverse_dependencies1.satisfied_by?(spec2.version) && - !reverse_dependencies2.satisfied_by?(spec1.version) + !reverse_requirements1.satisfied_by?(spec2.version) && + !reverse_requirements2.satisfied_by?(spec1.version) end def log_missing_spec(spec) @@ -113,6 +107,31 @@ def #{type}_check(lockfile_name) end RUBY end + + private + + def ensure_reverse_data(lockfile_name) + return if @reverse_requirements.key?(lockfile_name) + + # can use Gem::Requirement.default_prelease when Ruby 2.6 support is dropped + reverse_requirements = Hash.new { |h, k| h[k] = Gem::Requirement.new(">= 0.a") } + reverse_dependencies = Hash.new { |h, k| h[k] = Set.new } + + lockfile = parser(lockfile_name) + + lockfile.dependencies.each_value do |spec| + reverse_requirements[spec.name].requirements.concat(spec.requirement.requirements) + end + lockfile.specs.each do |spec| + spec.dependencies.each do |dependency| + reverse_requirements[dependency.name].requirements.concat(dependency.requirement.requirements) + reverse_dependencies[dependency.name] << spec.name + end + end + + @reverse_requirements[lockfile_name] = reverse_requirements + @reverse_dependencies[lockfile_name] = reverse_dependencies + end end end end diff --git a/spec/bundler/multilock_spec.rb b/spec/bundler/multilock_spec.rb index 5a44ff9..a2b2be3 100644 --- a/spec/bundler/multilock_spec.rb +++ b/spec/bundler/multilock_spec.rb @@ -814,6 +814,51 @@ end end + it "keeps transitive dependencies in sync, even when the intermediate deps are conflicting" do + orig_gemfile = <<~RUBY + gem "ddtrace", "~> 1.13" + + lockfile do + gem "activesupport", "6.0.0" + end + + lockfile "rails-6.1" do + gem "activesupport", "6.1.0" + end + RUBY + + with_gemfile(orig_gemfile) do + invoke_bundler("install") + + write_gemfile(<<~RUBY) + gem "ddtrace", "~> 1.20.0" + + lockfile do + gem "activesupport", "~> 6.0.0" + end + + lockfile "rails-6.1" do + gem "activesupport", "~> 6.1.0" + end + RUBY + + FileUtils.cp("Gemfile.rails-6.1.lock", "Gemfile.rails-6.1.lock.orig") + # roll back to ddtrace 1.20.0 + invoke_bundler("install") + + # loosen the requirement to allow > 1.20, but with it locked to + # 1.12. But act like the alternate lockfile didn't get updated + write_gemfile(orig_gemfile) + FileUtils.cp("Gemfile.rails-6.1.lock.orig", "Gemfile.rails-6.1.lock") + + # now a plain install should sync the alternate lockfile, rolling it back too + invoke_bundler("install") + + expect(invoke_bundler("info ddtrace")).to include("1.20.0") + expect(invoke_bundler("info ddtrace", env: { "BUNDLE_LOCKFILE" => "rails-6.1" })).to include("1.20.0") + end + end + private def create_local_gem(name, content = "", subdirectory: true)