Skip to content

Commit

Permalink
fix conflicting gems from alternate lockfiles updating unexpectedly
Browse files Browse the repository at this point in the history
  • Loading branch information
ccutrer committed Mar 20, 2024
1 parent 8d45782 commit 740e986
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 19 deletions.
36 changes: 22 additions & 14 deletions lib/bundler/multilock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,14 @@ def after_install_all(install: true)
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]
# we already wrote the default lockfile
next if lockfile_definition[:lockfile] == Bundler.default_lockfile(force_original: true)
next if lockfile_name == Bundler.default_lockfile(force_original: true)

# root needs to be set so that paths are output relative to the correct root in the lockfile
Bundler.root = lockfile_definition[:gemfile].dirname

relative_lockfile = lockfile_definition[:lockfile].relative_path_from(Dir.pwd)
relative_lockfile = lockfile_name.relative_path_from(Dir.pwd)

# already up to date?
up_to_date = false
Expand All @@ -182,26 +183,27 @@ def after_install_all(install: true)

if Bundler.frozen_bundle?
# if we're frozen, you have to use the pre-existing lockfile
unless lockfile_definition[:lockfile].exist?
unless lockfile_name.exist?
Bundler.ui.error("The bundle is locked, but #{relative_lockfile} is missing. " \
"Please make sure you have checked #{relative_lockfile} " \
"into version control before deploying.")
exit 1
end

Bundler.ui.info("Installing gems for #{relative_lockfile}...")
write_lockfile(lockfile_definition, lockfile_definition[:lockfile], install: install)
write_lockfile(lockfile_definition, lockfile_name, install: install)
else
Bundler.ui.info("Syncing to #{relative_lockfile}...") if attempts == 1
synced_any = true

parent = lockfile_definition[:parent]
parent_root = parent.dirname
parent_specs = cache.specs(parent)
specs = lockfile_name.exist? ? cache.specs(lockfile_name) : {}
parent_lockfile_name = lockfile_definition[:parent]
parent_root = parent_lockfile_name.dirname
parent_specs = cache.specs(parent_lockfile_name)

# adjust locked paths from the parent lockfile to be relative to _this_ gemfile
adjusted_parent_lockfile_contents =
cache.contents(parent).gsub(/PATH\n remote: ([^\n]+)\n/) do |remote|
cache.contents(parent_lockfile_name).gsub(/PATH\n remote: ([^\n]+)\n/) do |remote|
remote_path = Pathname.new($1)
next remote if remote_path.absolute?

Expand All @@ -221,27 +223,33 @@ def after_install_all(install: true)
TEXT
end

if lockfile_definition[:lockfile].exist?
if lockfile_name.exist?
# if the lockfile already exists, "merge" it together
parent_lockfile = if adjusted_parent_lockfile_contents == cache.contents(lockfile_definition[:lockfile])
cache.parser(parent)
parent_lockfile = if adjusted_parent_lockfile_contents == cache.contents(lockfile_name)
cache.parser(parent_lockfile_name)
else
local_parser_cache[adjusted_parent_lockfile_contents] ||=
LockfileParser.new(adjusted_parent_lockfile_contents)
end
lockfile = cache.parser(lockfile_definition[:lockfile])
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)

dependency_changes ||= spec != parent_spec
parent_spec
end

lockfile.specs.replace(parent_lockfile.specs + lockfile.specs).uniq!
missing_specs = parent_specs.each_value.reject do |parent_spec|
specs.include?([parent_spec.name, parent_spec.platform])
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
Expand Down Expand Up @@ -277,7 +285,7 @@ def after_install_all(install: true)
dependency_changes: dependency_changes,
unlocking_bundler: unlocking_bundler)
end
cache.invalidate_lockfile(lockfile_definition[:lockfile]) if had_changes
cache.invalidate_lockfile(lockfile_name) if had_changes

# if we had changes, bundler may have updated some common
# dependencies beyond the default lockfile, so re-run it
Expand Down
8 changes: 8 additions & 0 deletions lib/bundler/multilock/cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,14 @@ def reverse_dependencies(lockfile_name)
end
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_dependencies1.satisfied_by?(spec2.version) &&
!reverse_dependencies2.satisfied_by?(spec1.version)
end

def log_missing_spec(spec)
return if @missing_specs.include?(spec)

Expand Down
6 changes: 1 addition & 5 deletions lib/bundler/multilock/check.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,6 @@ def deep_check(lockfile_definition)
success = false
end

reverse_dependencies = @cache.reverse_dependencies(lockfile_name)
parent_reverse_dependencies = @cache.reverse_dependencies(parent_lockfile_name)

# look through top-level explicit dependencies for pinned requirements
if lockfile_definition[:enforce_pinned_additional_dependencies]
find_pinned_dependencies(proven_pinned, parser.dependencies.each_value)
Expand Down Expand Up @@ -142,8 +139,7 @@ def deep_check(lockfile_definition)

# 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)
if @cache.conflicting_requirements?(lockfile_name, parent_lockfile_name, spec, parent_spec)
# 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
Expand Down
48 changes: 48 additions & 0 deletions spec/bundler/multilock_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,54 @@
end
end

it "doesn't update versions in alternate lockfiles when syncing" do
# first
with_gemfile(<<~RUBY) do
gem "rubocop", "1.62.0"
lockfile do
gem "activesupport", "7.0.0"
end
lockfile "rails-7.1" do
gem "activesupport", "7.1.3"
end
RUBY
invoke_bundler("install")

write_gemfile(<<~RUBY)
gem "rubocop", "~> 1.62"
lockfile do
gem "activesupport", "~> 7.0.0"
end
lockfile "rails-7.1" do
gem "activesupport", "~> 7.1.0"
end
RUBY

# first, unpin, but ensure no gems update during this process
invoke_bundler("install")

expect(invoke_bundler("info rubocop")).to include("1.62.0")
expect(invoke_bundler("info activesupport")).to include("7.0.0")
expect(invoke_bundler("info rubocop", env: { "BUNDLE_LOCKFILE" => "rails-7.1" })).to include("1.62.0")
expect(invoke_bundler("info activesupport", env: { "BUNDLE_LOCKFILE" => "rails-7.1" })).to include("7.1.3")
expect(invoke_bundler("info activesupport", env: { "BUNDLE_LOCKFILE" => "rails-7.1" })).not_to include("7.1.3.")

# now, update an unrelated gem, but _only_ that gem
# this should not update other gems in the alternate lockfiles
invoke_bundler("update rubocop --conservative")

expect(invoke_bundler("info rubocop")).not_to include("1.62.0")
expect(invoke_bundler("info activesupport")).to include("7.0.0")
expect(invoke_bundler("info rubocop", env: { "BUNDLE_LOCKFILE" => "rails-7.1" })).not_to include("1.62.0")
expect(invoke_bundler("info activesupport", env: { "BUNDLE_LOCKFILE" => "rails-7.1" })).to include("7.1.3")
expect(invoke_bundler("info activesupport", env: { "BUNDLE_LOCKFILE" => "rails-7.1" })).not_to include("7.1.3.")
end
end

private

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

0 comments on commit 740e986

Please sign in to comment.