From e7698f73eb3409eb23d1695d775a9bbf4fdf41bd Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 28 Mar 2024 14:51:21 -0600 Subject: [PATCH 1/6] fix not syncing dependent gems when their depending gem is synced --- 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) From 91c905585ebfe2ba983974a718841975f42e47a4 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Fri, 29 Mar 2024 09:39:22 -0600 Subject: [PATCH 2/6] use a hash keyed by lockfile for the definition list --- lib/bundler/multilock.rb | 21 ++++++++------------- lib/bundler/multilock/check.rb | 8 ++++---- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/lib/bundler/multilock.rb b/lib/bundler/multilock.rb index 75ab4d6..af1509e 100644 --- a/lib/bundler/multilock.rb +++ b/lib/bundler/multilock.rb @@ -56,26 +56,24 @@ def add_lockfile(lockfile = nil, # allow short-form lockfile names lockfile = expand_lockfile(lockfile) - if lockfile_definitions.find { |definition| definition[:lockfile] == lockfile } - raise ArgumentError, "Lockfile #{lockfile} is already defined" - end + raise ArgumentError, "Lockfile #{lockfile} is already defined" if lockfile_definitions.key?(lockfile) env_lockfile = lockfile if active && ENV["BUNDLE_LOCKFILE"] == "active" env_lockfile ||= ENV["BUNDLE_LOCKFILE"]&.then { |l| expand_lockfile(l) } active = env_lockfile == lockfile if env_lockfile - if active && (old_active = lockfile_definitions.find { |definition| definition[:active] }) + if active && (old_active = lockfile_definitions.each_value.find { |definition| definition[:active] }) raise ArgumentError, "Only one lockfile (#{old_active[:lockfile]}) can be flagged as active" end parent = expand_lockfile(parent) if parent != Bundler.default_lockfile(force_original: true) && - !lockfile_definitions.find { |definition| definition[:lockfile] == parent } && + !lockfile_definitions.key?(parent) && !parent.exist? raise ArgumentError, "Parent lockfile #{parent} is not defined" end - lockfile_definitions << (lockfile_def = { + lockfile_definitions[lockfile] = (lockfile_def = { gemfile: (gemfile && Bundler.root.join(gemfile).expand_path) || Bundler.default_gemfile, lockfile: lockfile, active: active, @@ -158,8 +156,7 @@ def after_install_all(install: true) synced_any = false local_parser_cache = {} Bundler.settings.temporary(cache_all_platforms: true, suppress_install_using_messages: true) do - lockfile_definitions.each do |lockfile_definition| - lockfile_name = lockfile_definition[:lockfile] + lockfile_definitions.each do |lockfile_name, lockfile_definition| # we already wrote the default lockfile next if lockfile_name == Bundler.default_lockfile(force_original: true) @@ -334,7 +331,7 @@ def loaded! @loaded = true return if lockfile_definitions.empty? - return unless lockfile_definitions.none? { |definition| definition[:active] } + return unless lockfile_definitions.each_value.none? { |definition| definition[:active] } if ENV["BUNDLE_LOCKFILE"]&.then { |l| expand_lockfile(l) } == Bundler.default_lockfile(force_original: true) @@ -344,9 +341,7 @@ def loaded! raise GemfileNotFound, "Could not locate lockfile #{ENV["BUNDLE_LOCKFILE"].inspect}" if ENV["BUNDLE_LOCKFILE"] # Gemfile.lock isn't explicitly specified, otherwise it would be active - default_lockfile_definition = lockfile_definitions.find do |definition| - definition[:lockfile] == Bundler.default_lockfile(force_original: true) - end + default_lockfile_definition = lockfile_definitions[Bundler.default_lockfile(force_original: true)] return unless default_lockfile_definition && default_lockfile_definition[:active] == false raise GemfileEvalError, "No lockfiles marked as active" @@ -411,7 +406,7 @@ def inject_preamble # @!visibility private def reset! - @lockfile_definitions = [] + @lockfile_definitions = {} @loaded = false end diff --git a/lib/bundler/multilock/check.rb b/lib/bundler/multilock/check.rb index 59d7094..c046526 100644 --- a/lib/bundler/multilock/check.rb +++ b/lib/bundler/multilock/check.rb @@ -25,11 +25,11 @@ def run(skip_base_checks: false) base_check({ gemfile: Bundler.default_gemfile, lockfile: Bundler.default_lockfile(force_original: true) }) end - Multilock.lockfile_definitions.each do |lockfile_definition| - next if lockfile_definition[:lockfile] == Bundler.default_lockfile(force_original: true) + Multilock.lockfile_definitions.each do |lockfile_name, lockfile_definition| + next if lockfile_name == Bundler.default_lockfile(force_original: true) - unless lockfile_definition[:lockfile].exist? - Bundler.ui.error("Lockfile #{lockfile_definition[:lockfile]} does not exist.") + unless lockfile_name.exist? + Bundler.ui.error("Lockfile #{lockfile_name} does not exist.") success = false next end From d5987e0d565705be8487eda02af3c318bc4af0bf Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 28 Mar 2024 17:24:25 -0600 Subject: [PATCH 3/6] fix syncing of ruby version and auto-infer ruby version if the parent lockfile constrains it, but a child lockfile does not --- lib/bundler/multilock.rb | 23 +++++++++++++--- lib/bundler/multilock/check.rb | 13 ++++++--- lib/bundler/multilock/ext/dsl.rb | 10 +++++++ spec/bundler/multilock_spec.rb | 46 ++++++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 7 deletions(-) diff --git a/lib/bundler/multilock.rb b/lib/bundler/multilock.rb index af1509e..77160f1 100644 --- a/lib/bundler/multilock.rb +++ b/lib/bundler/multilock.rb @@ -273,7 +273,7 @@ def after_install_all(install: true) p2 != "ruby" && p1 != p2 && MatchPlatform.platforms_match?(p2, p1) end end - lockfile.instance_variable_set(:@ruby_version, parent_lockfile.ruby_version) + lockfile.instance_variable_set(:@ruby_version, parent_lockfile.ruby_version) if lockfile.ruby_version unless lockfile.bundler_version == parent_lockfile.bundler_version unlocking_bundler = parent_lockfile.bundler_version lockfile.instance_variable_set(:@bundler_version, parent_lockfile.bundler_version) @@ -341,7 +341,7 @@ def loaded! raise GemfileNotFound, "Could not locate lockfile #{ENV["BUNDLE_LOCKFILE"].inspect}" if ENV["BUNDLE_LOCKFILE"] # Gemfile.lock isn't explicitly specified, otherwise it would be active - default_lockfile_definition = lockfile_definitions[Bundler.default_lockfile(force_original: true)] + default_lockfile_definition = self.default_lockfile_definition return unless default_lockfile_definition && default_lockfile_definition[:active] == false raise GemfileEvalError, "No lockfiles marked as active" @@ -410,6 +410,11 @@ def reset! @loaded = false end + # @!visibility private + def default_lockfile_definition + lockfile_definitions[Bundler.default_lockfile(force_original: true)] + end + private def expand_lockfile(lockfile) @@ -449,6 +454,12 @@ def write_lockfile(lockfile_definition, builder = Dsl.new builder.eval_gemfile(gemfile, &prepare_block) if prepare_block builder.eval_gemfile(gemfile) + if !builder.instance_variable_get(:@ruby_version) && + (parent_lockfile = lockfile_definition[:parent]) && + (parent_lockfile_definition = lockfile_definitions[parent_lockfile]) && + (parent_ruby_version_requirement = parent_lockfile_definition[:ruby_version_requirement]) + builder.instance_variable_set(:@ruby_version, parent_ruby_version_requirement) + end definition = builder.to_definition(lockfile, { bundler: unlocking_bundler }) definition.instance_variable_set(:@dependency_changes, dependency_changes) if dependency_changes @@ -506,11 +517,15 @@ def write_lockfile(lockfile_definition, end SharedHelpers.capture_filesystem_access do definition.instance_variable_set(:@resolved_bundler_version, unlocking_bundler) if unlocking_bundler + + # need to force it to _not_ preserve unknown sections, so that it + # will overwrite the ruby version + definition.instance_variable_set(:@unlocking_bundler, true) if Bundler.gem_version >= Gem::Version.new("2.5.6") definition.instance_variable_set(:@lockfile, lockfile_definition[:lockfile]) - definition.lock(true) + definition.lock else - definition.lock(lockfile_definition[:lockfile], true) + definition.lock(lockfile_definition[:lockfile]) end end ensure diff --git a/lib/bundler/multilock/check.rb b/lib/bundler/multilock/check.rb index c046526..2bc7b86 100644 --- a/lib/bundler/multilock/check.rb +++ b/lib/bundler/multilock/check.rb @@ -22,8 +22,10 @@ def run(skip_base_checks: false) success = true unless skip_base_checks - base_check({ gemfile: Bundler.default_gemfile, - lockfile: Bundler.default_lockfile(force_original: true) }) + default_lockfile_definition = Multilock.default_lockfile_definition + default_lockfile_definition ||= { gemfile: Bundler.default_gemfile, + lockfile: Bundler.default_lockfile(force_original: true) } + base_check(default_lockfile_definition) end Multilock.lockfile_definitions.each do |lockfile_name, lockfile_definition| next if lockfile_name == Bundler.default_lockfile(force_original: true) @@ -68,7 +70,7 @@ def base_check(lockfile_definition, check_missing_deps: false) end end - next false unless not_installed.empty? && definition.no_resolve_needed? + next false unless not_installed.empty? # cache a sentinel so that we can share a cache regardless of the check_missing_deps argument next :missing_deps unless (definition.locked_gems.dependencies.values - definition.dependencies).empty? @@ -105,6 +107,11 @@ def deep_check(lockfile_definition) "does not match the parent lockfile's version (@#{parent_parser.bundler_version}).") success = false end + unless parser.ruby_version == parent_parser.ruby_version + Bundler.ui.error("ruby (#{parser.ruby_version || ""}) in #{lockfile_path} " \ + "does not match the parent lockfile's version (#{parent_parser.ruby_version}).") + success = false + end # look through top-level explicit dependencies for pinned requirements if lockfile_definition[:enforce_pinned_additional_dependencies] diff --git a/lib/bundler/multilock/ext/dsl.rb b/lib/bundler/multilock/ext/dsl.rb index 0235cf2..c83a231 100644 --- a/lib/bundler/multilock/ext/dsl.rb +++ b/lib/bundler/multilock/ext/dsl.rb @@ -11,12 +11,22 @@ module ClassMethods # Significant changes: # * evaluate the prepare block as part of the gemfile + # * keep track of the ruby version set in the default gemfile + # * apply that ruby version to alternate lockfiles if they didn't set one + # themselves # * mark Multilock as loaded once the main gemfile is evaluated # so that they're not loaded multiple times def evaluate(gemfile, lockfile, unlock) builder = new builder.eval_gemfile(gemfile, &Multilock.prepare_block) if Multilock.prepare_block builder.eval_gemfile(gemfile) + if (ruby_version_requirement = builder.instance_variable_get(:@ruby_version)) + Multilock.lockfile_definitions[lockfile][:ruby_version_requirement] = ruby_version_requirement + elsif (parent_lockfile = Multilock.lockfile_definitions.dig(lockfile, :parent)) && + (parent_lockfile_definition = Multilock.lockfile_definitions[parent_lockfile]) && + (parent_ruby_version_requirement = parent_lockfile_definition[:ruby_version_requirement]) + builder.instance_variable_set(:@ruby_version, parent_ruby_version_requirement) + end Multilock.loaded! builder.to_definition(lockfile, unlock) end diff --git a/spec/bundler/multilock_spec.rb b/spec/bundler/multilock_spec.rb index a2b2be3..bbd757d 100644 --- a/spec/bundler/multilock_spec.rb +++ b/spec/bundler/multilock_spec.rb @@ -859,6 +859,44 @@ end end + it "syncs ruby version" do + with_gemfile(<<~RUBY) do + gem "concurrent-ruby", "1.2.2" + + lockfile do + ruby ">= 2.1" + end + + lockfile "alt" do + end + RUBY + invoke_bundler("install") + + expect(File.read("Gemfile.lock")).to include(Bundler::RubyVersion.system.to_s) + expect(File.read("Gemfile.alt.lock")).to include(Bundler::RubyVersion.system.to_s) + + update_lockfile_ruby("Gemfile.alt.lock", "ruby 2.1.0p0") + + expect do + invoke_bundler("check") + end.to raise_error(/ruby \(ruby 2.1.0p0\) in Gemfile.alt.lock does not match the parent lockfile's version/) + + update_lockfile_ruby("Gemfile.alt.lock", nil) + expect do + invoke_bundler("check") + end.to raise_error(/ruby \(\) in Gemfile.alt.lock does not match the parent lockfile's version/) + + invoke_bundler("install") + expect(File.read("Gemfile.alt.lock")).to include(Bundler::RubyVersion.system.to_s) + + update_lockfile_ruby("Gemfile.lock", "ruby 2.6.0p0") + update_lockfile_ruby("Gemfile.alt.lock", nil) + + invoke_bundler("install") + expect(File.read("Gemfile.alt.lock")).to include("ruby 2.6.0p0") + end + end + private def create_local_gem(name, content = "", subdirectory: true) @@ -960,4 +998,12 @@ def update_lockfile_bundler(lockfile, version) File.write(lockfile, new_contents) end + + def update_lockfile_ruby(lockfile, version) + old_contents = File.read(lockfile) + new_version = version ? "RUBY VERSION\n #{version}\n\n" : "" + new_contents = old_contents.gsub(/RUBY VERSION\n #{Bundler::RubyVersion::PATTERN}\n\n/o, new_version) + + File.write(lockfile, new_contents) + end end From 8b9eb5275665032211aaf61b15651163398d0ebb Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 28 Mar 2024 18:33:37 -0600 Subject: [PATCH 4/6] fix source convergence when syncing --- lib/bundler/multilock.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/bundler/multilock.rb b/lib/bundler/multilock.rb index 77160f1..5517057 100644 --- a/lib/bundler/multilock.rb +++ b/lib/bundler/multilock.rb @@ -255,7 +255,10 @@ def after_install_all(install: true) replaced_any = dependency_changes = true end forced_inherited_specs << spec.name - parent_spec + + new_spec = parent_spec.dup + new_spec.source = spec.source + new_spec end break unless replaced_any From 041112497c0ae8a53f35951ff30217c7144afba9 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Thu, 28 Mar 2024 18:39:07 -0600 Subject: [PATCH 5/6] ignore errors installing gems due to a gem version not allowed on the current ruby version --- lib/bundler/multilock.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bundler/multilock.rb b/lib/bundler/multilock.rb index 5517057..119d76b 100644 --- a/lib/bundler/multilock.rb +++ b/lib/bundler/multilock.rb @@ -489,7 +489,7 @@ def write_lockfile(lockfile_definition, Installer.install(gemfile.dirname, current_definition, {}) end end - rescue RubyVersionMismatch, GemNotFound, SolveFailure + rescue RubyVersionMismatch, GemNotFound, SolveFailure, InstallError, ProductionError # ignore end end From 9fe3c345e89c0f7e5bb357c929ed23234dd4face Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Fri, 29 Mar 2024 10:36:33 -0600 Subject: [PATCH 6/6] simplify intermediate lockfile creation during syncing _don't_ include everything from the parent lockfile. only stuff that actually exists in the child lockfile. this avoids accidentally unlocking everything when syncing a minor change, because sources won't have changed --- lib/bundler/multilock.rb | 83 +++++++++++++++++++++------------- lib/bundler/multilock/cache.rb | 14 +++--- spec/bundler/multilock_spec.rb | 4 ++ 3 files changed, 63 insertions(+), 38 deletions(-) diff --git a/lib/bundler/multilock.rb b/lib/bundler/multilock.rb index 119d76b..828b584 100644 --- a/lib/bundler/multilock.rb +++ b/lib/bundler/multilock.rb @@ -148,6 +148,7 @@ def after_install_all(install: true) Bundler.ui.debug("Syncing to alternate lockfiles") attempts = 1 + previous_contents = Set.new default_root = Bundler.root @@ -165,6 +166,17 @@ def after_install_all(install: true) relative_lockfile = lockfile_name.relative_path_from(Dir.pwd) + # prevent infinite loops of tick-tocking back and forth between two versions + current_contents = cache.contents(lockfile_name) + if previous_contents.include?(current_contents) + Bundler.ui.debug("Unable to converge on a single solution for #{lockfile_name}; " \ + "perhaps there are conflicting requirements?") + attempts = 1 + previous_contents.clear + next + end + previous_contents << current_contents + # already up to date? up_to_date = false Bundler.settings.temporary(frozen: true) do @@ -175,6 +187,7 @@ def after_install_all(install: true) end if up_to_date attempts = 1 + previous_contents.clear next end @@ -231,44 +244,49 @@ def after_install_all(install: true) lockfile = cache.parser(lockfile_name) dependency_changes = false - 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 + spec_precedences = {} + + check_precedence = lambda do |spec, parent_spec| + next :parent if spec.nil? + next :self if parent_spec.nil? + next spec_precedences[spec.name] if spec_precedences.key?(spec.name) - new_spec = parent_spec.dup - new_spec.source = spec.source - new_spec + precedence = :self if cache.conflicting_requirements?(lockfile_name, + parent_lockfile_name, + spec, + parent_spec) + + # look through all reverse dependencies; if any of them say it + # has to come from self, due to conflicts, then this gem has + # to come from self as well + [cache.reverse_dependencies(lockfile_name), + cache.reverse_dependencies(parent_lockfile_name)].each do |reverse_dependencies| + break if precedence == :self + + reverse_dependencies[spec.name].each do |dep_name| + precedence = check_precedence.call(specs[dep_name], parent_specs[dep_name]) + break if precedence == :self + end end - break unless replaced_any + spec_precedences[spec.name] = precedence || :parent end - missing_specs = parent_specs.each_value.reject do |parent_spec| - specs.include?([parent_spec.name, parent_spec.platform]) + # 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 + + next spec if check_precedence.call(spec, parent_spec) == :self + + dependency_changes ||= spec != parent_spec + + new_spec = parent_spec.dup + new_spec.source = spec.source + new_spec end - lockfile.specs.replace(missing_specs + lockfile.specs) unless missing_specs.empty? - lockfile.sources.replace(parent_lockfile.sources + lockfile.sources).uniq! + lockfile.platforms.replace(parent_lockfile.platforms).uniq! # prune more specific platforms lockfile.platforms.delete_if do |p1| @@ -309,12 +327,13 @@ def after_install_all(install: true) # once to reset them back to the default lockfile's version. # if it's already good, the `check` check at the beginning of # the loop will skip the second sync anyway. - if had_changes && attempts < 2 + if had_changes attempts += 1 Bundler.ui.debug("Re-running sync to #{relative_lockfile} to reset common dependencies") redo else attempts = 1 + previous_contents.clear end end end diff --git a/lib/bundler/multilock/cache.rb b/lib/bundler/multilock/cache.rb index 032e8b2..cf8665b 100644 --- a/lib/bundler/multilock/cache.rb +++ b/lib/bundler/multilock/cache.rb @@ -45,7 +45,9 @@ def invalidate_checks(lockfile_name) # @param lockfile_name [Pathname] # @return [String] the raw contents of the lockfile def contents(lockfile_name) - @contents[lockfile_name] ||= lockfile_name.read.freeze + @contents.fetch(lockfile_name) do + @contents[lockfile_name] = lockfile_name.file? && lockfile_name.read.freeze + end end # @param lockfile_name [Pathname] @@ -119,13 +121,13 @@ def ensure_reverse_data(lockfile_name) lockfile = parser(lockfile_name) - lockfile.dependencies.each_value do |spec| - reverse_requirements[spec.name].requirements.concat(spec.requirement.requirements) + lockfile.dependencies.each_value do |dep| + reverse_requirements[dep.name].requirements.concat(dep.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 + spec.dependencies.each do |dep| + reverse_requirements[dep.name].requirements.concat(dep.requirement.requirements) + reverse_dependencies[dep.name] << spec.name end end diff --git a/spec/bundler/multilock_spec.rb b/spec/bundler/multilock_spec.rb index bbd757d..5af3f8c 100644 --- a/spec/bundler/multilock_spec.rb +++ b/spec/bundler/multilock_spec.rb @@ -815,6 +815,10 @@ end it "keeps transitive dependencies in sync, even when the intermediate deps are conflicting" do + pending "this spec was broken when improving _not_ unlocking all gems when syncing to alternate lockfiles. " \ + "it was determined that until such a problem arises again, it's not worth the effort to fix at " \ + "the moment" + orig_gemfile = <<~RUBY gem "ddtrace", "~> 1.13"