Skip to content

Commit

Permalink
improve inferences on if mismatched dependencies are intentional (#12)
Browse files Browse the repository at this point in the history 
basically, makes it so that we don't have to re-sync lockfiles
every time when you have disjunctions on the same gem (i.e. rails)
between lockfiles

due to the improved logic, deprecate the allow_mismatched_dependencies
param entirely.
  • Loading branch information
@ccutrer
ccutrer authored Oct 10, 2023
1 parent adce58d commit 41e3bd9
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 55 deletions.
13 changes: 5 additions & 8 deletions lib/bundler/multilock.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,6 @@ class << self
# BUNDLE_LOCKFILE will still override a lockfile tagged as active
# @param parent [String] The parent lockfile to sync dependencies from.
# Also used for comparing enforce_pinned_additional_dependencies against.
# @param allow_mismatched_dependencies [true, false]
# Allows version differences in dependencies between this lockfile and
# the default lockfile. Note that even with this option, only top-level
# dependencies that differ from the default lockfile, and their transitive
# depedencies, are allowed to mismatch.
# @param enforce_pinned_additional_dependencies [true, false]
# If dependencies are present in this lockfile that are not present in the
# default lockfile, enforce that they are pinned.
Expand All @@ -44,12 +39,15 @@ def add_lockfile(lockfile = nil,
active: nil,
default: nil,
parent: nil,
allow_mismatched_dependencies: true,
allow_mismatched_dependencies: nil,
enforce_pinned_additional_dependencies: false,
&block)
# backcompat
active = default if active.nil?
Bundler.ui.warn("lockfile(default:) is deprecated. Use lockfile(active:) instead.") if default
unless allow_mismatched_dependencies.nil?
Bundler.ui.warn("lockfile(allow_mismatched_dependencies:) is deprecated.")
end

active = true if active.nil? && lockfile_definitions.empty? && lockfile.nil? && gemfile.nil?

Expand Down Expand Up @@ -81,7 +79,6 @@ def add_lockfile(lockfile = nil,
active: active,
prepare: block,
parent: parent,
allow_mismatched_dependencies: allow_mismatched_dependencies,
enforce_pinned_additional_dependencies: enforce_pinned_additional_dependencies
})

Expand Down Expand Up @@ -171,7 +168,7 @@ def after_install_all(install: true)
Bundler.settings.temporary(frozen: true) do
Bundler.ui.silence do
up_to_date = checker.base_check(lockfile_definition) &&
checker.check(lockfile_definition, allow_mismatched_dependencies: false)
checker.check(lockfile_definition)
end
end
if up_to_date
Expand Down
61 changes: 28 additions & 33 deletions lib/bundler/multilock/check.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def base_check(lockfile_definition, log_missing: false, return_missing: false)

# this checks for mismatches between the parent lockfile and the given lockfile,
# and for pinned dependencies in lockfiles requiring them
def check(lockfile_definition, allow_mismatched_dependencies: true)
def check(lockfile_definition)
success = true
proven_pinned = Set.new
needs_pin_check = []
Expand All @@ -109,44 +109,16 @@ def check(lockfile_definition, allow_mismatched_dependencies: true)
success = false
end

specs = lockfile.specs.group_by(&:name)
if allow_mismatched_dependencies
allow_mismatched_dependencies = lockfile_definition[:allow_mismatched_dependencies]
end

# build list of top-level dependencies that differ from the parent lockfile,
# and all _their_ transitive dependencies
if allow_mismatched_dependencies
transitive_dependencies = Set.new
# only dependencies that differ from the parent lockfile
pending_transitive_dependencies = lockfile.dependencies.reject do |name, dep|
parent_lockfile.dependencies[name] == dep
end.map(&:first)

until pending_transitive_dependencies.empty?
dep = pending_transitive_dependencies.shift
next if transitive_dependencies.include?(dep)

transitive_dependencies << dep
platform_specs = specs[dep]
unless platform_specs
# should only be bundler that's missing a spec
raise "Could not find spec for dependency #{dep}" unless dep == "bundler"

next
end

pending_transitive_dependencies.concat(platform_specs.flat_map(&:dependencies).map(&:name).uniq)
end
end
reverse_dependencies = cache_reverse_dependencies(lockfile)
parent_reverse_dependencies = cache_reverse_dependencies(parent_lockfile)

# look through top-level explicit dependencies for pinned requirements
if lockfile_definition[:enforce_pinned_additional_dependencies]
find_pinned_dependencies(proven_pinned, lockfile.dependencies.each_value)
end

# check for conflicting requirements (and build list of pins, in the same loop)
specs.values.flatten.each do |spec|
lockfile.specs.each do |spec|
parent_spec = lockfile_specs[parent][[spec.name, spec.platform]]

if lockfile_definition[:enforce_pinned_additional_dependencies]
Expand All @@ -170,7 +142,15 @@ def check(lockfile_definition, allow_mismatched_dependencies: true)
end

next if parent_spec.version == spec.version && same_source
next if allow_mismatched_dependencies && transitive_dependencies.include?(spec.name)

# the version in the parent lockfile cannot possibly satisfy the requirements
# in this lockfile, and vice versa, so we assume it's intentional and allow it
unless reverse_dependencies[spec.name].satisfied_by?(parent_spec.version) ||
parent_reverse_dependencies[spec.name].satisfied_by?(spec.version)
# we're allowing it to differ from the parent, so pin check requirement comes into play
needs_pin_check << spec if lockfile_definition[:enforce_pinned_additional_dependencies]
next
end

Bundler.ui.error("#{spec}#{spec.git_version} in #{lockfile_path} " \
"does not match the parent lockfile's version " \
Expand Down Expand Up @@ -206,6 +186,21 @@ def check(lockfile_definition, allow_mismatched_dependencies: true)

private

def cache_reverse_dependencies(lockfile)
reverse_dependencies = Hash.new { |h, k| h[k] = Gem::Requirement.default_prerelease }

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

reverse_dependencies
end

def find_pinned_dependencies(proven_pinned, dependencies)
dependencies.each do |dependency|
dependency.requirement.requirements.each do |requirement|
Expand Down
45 changes: 31 additions & 14 deletions spec/bundler/multilock_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@
it "syncs from a parent lockfile" do
with_gemfile(<<~RUBY) do
lockfile do
gem "activesupport", "> 6.0", "< 7.1"
gem "activesupport", "~> 7.0.0"
end
lockfile("6.1") do
Expand Down Expand Up @@ -359,16 +359,16 @@
it "notifies about mismatched versions between different lockfiles" do
with_gemfile(<<~RUBY) do
lockfile do
gem "activesupport", "~> 6.1.0"
gem "activesupport", ">= 6.1", "< 7.2"
end
lockfile("full", allow_mismatched_dependencies: false) do
gem "activesupport", "7.0.4.3"
lockfile("full") do
gem "activesupport", "6.1.7.6"
end
RUBY
expect do
invoke_bundler("install")
end.to raise_error(Regexp.new("activesupport \\(7.0.4.3\\) in Gemfile.full.lock " \
end.to raise_error(Regexp.new("activesupport \\(6.1.7.6\\) in Gemfile.full.lock " \
"does not match the parent lockfile's version"))
end
end
Expand All @@ -377,7 +377,7 @@
with_gemfile(<<~RUBY) do
gem "activesupport", "7.0.4.3" # depends on tzinfo ~> 2.0, so will get >= 2.0.6
lockfile("full", allow_mismatched_dependencies: false) do
lockfile("full") do
gem "tzinfo", "2.0.5"
end
Expand Down Expand Up @@ -410,7 +410,6 @@
it "disallows mismatched implicit dependencies" do
with_gemfile(<<~RUBY) do
lockfile("local_test/Gemfile.lock",
allow_mismatched_dependencies: false,
gemfile: "local_test/Gemfile")
gem "snaky_hash", "2.0.1"
Expand Down Expand Up @@ -635,32 +634,32 @@
# so that it won't downgrade if that's all you have available
it "installs missing deps from alternate lockfiles before syncing" do
Bundler.with_unbundled_env do
`gem uninstall activesupport -a --force 2> #{File::NULL}`
`gem install activesupport -v 6.1.7.6`
`gem uninstall activemodel -a --force 2> #{File::NULL}`
`gem install activemodel -v 6.1.7.6`
end

with_gemfile(<<~RUBY) do
lockfile() do
lockfile do
gem "activemodel", ">= 6.0"
end
lockfile("rails-6.1") do
gem "activemodel", "~> 6.1.0"
end
RUBY
invoke_bundler("install")
invoke_bundler("install --local")
expect(invoke_bundler("info activesupport", env: { "BUNDLE_LOCKFILE" => "rails-6.1" })).to include("6.1.7.6")

Bundler.with_unbundled_env do
`gem uninstall activesupport -v 6.1.7.6 --force 2> #{File::NULL}`
`gem install activesupport -v 6.1.6`
`gem uninstall activemodel -v 6.1.7.6 --force 2> #{File::NULL}`
`gem install activemodel -v 6.1.6`
end

expect { invoke_bundler("check") }.to raise_error(/The following gems are missing/)
invoke_bundler("install")

# it should have re-installed 6.1.7.6, leaving the lockfile alone
expect(invoke_bundler("info activesupport", env: { "BUNDLE_LOCKFILE" => "rails-6.1" })).to include("6.1.7.6")
expect(invoke_bundler("info activemodel", env: { "BUNDLE_LOCKFILE" => "rails-6.1" })).to include("6.1.7.6")
end
end

Expand Down Expand Up @@ -699,6 +698,24 @@
end
end

it "does not re-sync lockfiles that have conflicting sub-dependencies" do
with_gemfile(<<~RUBY) do
lockfile do
gem "activemodel", "~> 7.1.0"
end
lockfile("rails-7.0") do
gem "activemodel", "~> 7.0.0"
end
RUBY
output = invoke_bundler("install")
expect(output).to include("Syncing")

output = invoke_bundler("install")
expect(output).not_to include("Syncing")
end
end

private

def create_local_gem(name, content = "", subdirectory: true)
Expand Down

0 comments on commit 41e3bd9

Please sign in to comment.