Skip to content

Commit

Permalink
improve inferences on if mismatched dependencies are intentional
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 committed Oct 10, 2023
1 parent e104b9b commit 79fa76a
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 @@ -172,7 +169,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 79fa76a

Please sign in to comment.