Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid extraneous unlocks #34

Merged
merged 2 commits into from
Mar 29, 2024
Merged

avoid extraneous unlocks #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b780077
fix source convergence when syncing
ccutrer Mar 29, 2024
b8a46cf
simplify intermediate lockfile creation during syncing
ccutrer Mar 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 51 additions & 29 deletions lib/bundler/multilock.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -175,6 +187,7 @@ def after_install_all(install: true)
end
if up_to_date
attempts = 1
previous_contents.clear
next
end

Expand Down Expand Up @@ -231,41 +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
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)

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
forced_inherited_specs << spec.name
parent_spec
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|
Expand Down Expand Up @@ -306,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
Expand Down
14 changes: 8 additions & 6 deletions lib/bundler/multilock/cache.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions spec/bundler/multilock_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
Loading