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

improve inferences on if mismatched dependencies are intentional #12

Merged
merged 1 commit into from
Oct 10, 2023
Merged

improve inferences on if mismatched dependencies are intentional #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 5 additions & 8 deletions lib/bundler/multilock.rb
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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