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.

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

Make sure bundle update <specific_gems> can always update to the latest resolvable version of each requested gem #7558

Merged
merged 3 commits into from Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
77 changes: 53 additions & 24 deletions bundler/lib/bundler/definition.rb
Expand Up @@ -132,7 +132,7 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
@sources.merged_gem_lockfile_sections!(locked_gem_sources.first)
end

@unlock[:sources] ||= []
@sources_to_unlock = @unlock.delete(:sources) || []
@unlock[:ruby] ||= if @ruby_version && locked_ruby_version_object
@ruby_version.diff(locked_ruby_version_object)
end
Expand All @@ -144,11 +144,13 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
@path_changes = converge_paths
@source_changes = converge_sources

@explicit_unlocks = @unlock.delete(:gems) || []

if @unlock[:conservative]
@unlock[:gems] ||= @dependencies.map(&:name)
@gems_to_unlock = @explicit_unlocks.any? ? @explicit_unlocks : @dependencies.map(&:name)
else
eager_unlock = (@unlock[:gems] || []).map {|name| Dependency.new(name, ">= 0") }
@unlock[:gems] = @locked_specs.for(eager_unlock, false, platforms).map(&:name).uniq
eager_unlock = @explicit_unlocks.map {|name| Dependency.new(name, ">= 0") }
@gems_to_unlock = @locked_specs.for(eager_unlock, false, platforms).map(&:name).uniq
end

@dependency_changes = converge_dependencies
Expand Down Expand Up @@ -227,7 +229,6 @@ def missing_specs?
@resolver = nil
@resolution_packages = nil
@specs = nil
@gem_version_promoter = nil

Bundler.ui.debug "The definition is missing dependencies, failed to resolve & materialize locally (#{e})"
true
Expand Down Expand Up @@ -568,8 +569,10 @@ def resolution_packages
@resolution_packages ||= begin
last_resolve = converge_locked_specs
remove_invalid_platforms!(current_dependencies)
packages = Resolver::Base.new(source_requirements, expanded_dependencies, last_resolve, @platforms, locked_specs: @originally_locked_specs, unlock: @unlock[:gems], prerelease: gem_version_promoter.pre?)
additional_base_requirements_for_resolve(packages, last_resolve)
packages = Resolver::Base.new(source_requirements, expanded_dependencies, last_resolve, @platforms, locked_specs: @originally_locked_specs, unlock: @gems_to_unlock, prerelease: gem_version_promoter.pre?)
packages = additional_base_requirements_to_prevent_downgrades(packages, last_resolve)
packages = additional_base_requirements_to_force_updates(packages)
packages
end
end

Expand Down Expand Up @@ -673,14 +676,18 @@ def add_current_platform

def change_reason
if unlocking?
unlock_reason = @unlock.reject {|_k, v| Array(v).empty? }.map do |k, v|
if v == true
k.to_s
else
v = Array(v)
"#{k}: (#{v.join(", ")})"
end
end.join(", ")
unlock_targets = if @gems_to_unlock.any?
["gems", @gems_to_unlock]
elsif @sources_to_unlock.any?
["sources", @sources_to_unlock]
end

unlock_reason = if unlock_targets
"#{unlock_targets.first}: (#{unlock_targets.last.join(", ")})"
else
@unlock[:ruby] ? "ruby" : ""
end

return "bundler is unlocking #{unlock_reason}"
end
[
Expand Down Expand Up @@ -735,15 +742,15 @@ def converge_locals
spec = @dependencies.find {|s| s.name == k }
source = spec&.source
if source&.respond_to?(:local_override!)
source.unlock! if @unlock[:gems].include?(spec.name)
source.unlock! if @gems_to_unlock.include?(spec.name)
locals << [source, source.local_override!(v)]
end
end

sources_with_changes = locals.select do |source, changed|
changed || specs_changed?(source)
end.map(&:first)
!sources_with_changes.each {|source| @unlock[:sources] << source.name }.empty?
!sources_with_changes.each {|source| @sources_to_unlock << source.name }.empty?
end

def check_lockfile
Expand Down Expand Up @@ -820,7 +827,7 @@ def converge_sources
# gem), unlock it. For git sources, this means to unlock the revision, which
# will cause the `ref` used to be the most recent for the branch (or master) if
# an explicit `ref` is not used.
if source.respond_to?(:unlock!) && @unlock[:sources].include?(source.name)
if source.respond_to?(:unlock!) && @sources_to_unlock.include?(source.name)
source.unlock!
changes = true
end
Expand Down Expand Up @@ -864,7 +871,7 @@ def converge_dependencies
def converge_locked_specs
converged = converge_specs(@locked_specs)

resolve = SpecSet.new(converged.reject {|s| @unlock[:gems].include?(s.name) })
resolve = SpecSet.new(converged.reject {|s| @gems_to_unlock.include?(s.name) })

diff = nil

Expand Down Expand Up @@ -897,7 +904,7 @@ def converge_specs(specs)

@specs_that_changed_sources << s if gemfile_source != lockfile_source
deps << dep if !dep.source || lockfile_source.include?(dep.source)
@unlock[:gems] << name if lockfile_source.include?(dep.source) && lockfile_source != gemfile_source
@gems_to_unlock << name if lockfile_source.include?(dep.source) && lockfile_source != gemfile_source

# Replace the locked dependency's source with the equivalent source from the Gemfile
s.source = gemfile_source
Expand All @@ -906,7 +913,7 @@ def converge_specs(specs)
s.source = default_source unless sources.get(lockfile_source)
end

next if @unlock[:sources].include?(s.source.name)
next if @sources_to_unlock.include?(s.source.name)

# Path sources have special logic
if s.source.instance_of?(Source::Path) || s.source.instance_of?(Source::Gemspec)
Expand All @@ -928,12 +935,12 @@ def converge_specs(specs)
else
# If the spec is no longer in the path source, unlock it. This
# commonly happens if the version changed in the gemspec
@unlock[:gems] << name
@gems_to_unlock << name
end
end

if dep.nil? && requested_dependencies.find {|d| name == d.name }
@unlock[:gems] << s.name
@gems_to_unlock << s.name
else
converged << s
end
Expand Down Expand Up @@ -1010,7 +1017,7 @@ def lockfiles_equal?(current, proposed, preserve_unknown_sections)
current == proposed
end

def additional_base_requirements_for_resolve(resolution_packages, last_resolve)
def additional_base_requirements_to_prevent_downgrades(resolution_packages, last_resolve)
return resolution_packages unless @locked_gems && !sources.expired_sources?(@locked_gems.sources)
converge_specs(@originally_locked_specs - last_resolve).each do |locked_spec|
next if locked_spec.source.is_a?(Source::Path)
Expand All @@ -1019,6 +1026,28 @@ def additional_base_requirements_for_resolve(resolution_packages, last_resolve)
resolution_packages
end

def additional_base_requirements_to_force_updates(resolution_packages)
return resolution_packages if @explicit_unlocks.empty?
full_update = dup_for_full_unlock.resolve
@explicit_unlocks.each do |name|
version = full_update[name].first&.version
resolution_packages.base_requirements[name] = Gem::Requirement.new("= #{version}") if version
end
resolution_packages
end

def dup_for_full_unlock
unlocked_definition = self.class.new(@lockfile, @dependencies, @sources, true, @ruby_version, @optional_groups, @gemfiles)
unlocked_definition.resolution_mode = { "local" => !@remote }
unlocked_definition.setup_sources_for_resolve
unlocked_definition.gem_version_promoter.tap do |gvp|
gvp.level = gem_version_promoter.level
gvp.strict = gem_version_promoter.strict
gvp.pre = gem_version_promoter.pre
end
unlocked_definition
end

def remove_invalid_platforms!(dependencies)
return if Bundler.frozen_bundle?

Expand Down
122 changes: 122 additions & 0 deletions bundler/spec/commands/lock_spec.rb
Expand Up @@ -252,6 +252,128 @@
expect(read_lockfile).to eq(remove_checksums_from_lockfile(@lockfile, "(2.3.2)", "(#{rake_version})"))
end

it "updates specific gems using --update, even if that requires unlocking other top level gems" do
build_repo4 do
build_gem "prism", "0.15.1"
build_gem "prism", "0.24.0"

build_gem "ruby-lsp", "0.12.0" do |s|
s.add_dependency "prism", "< 0.24.0"
end

build_gem "ruby-lsp", "0.16.1" do |s|
s.add_dependency "prism", ">= 0.24.0"
end

build_gem "tapioca", "0.11.10" do |s|
s.add_dependency "prism", "< 0.24.0"
end

build_gem "tapioca", "0.13.1" do |s|
s.add_dependency "prism", ">= 0.24.0"
end
end

gemfile <<~G
source "#{file_uri_for(gem_repo4)}"
gem "tapioca"
gem "ruby-lsp"
G

lockfile <<~L
GEM
remote: #{file_uri_for(gem_repo4)}
specs:
prism (0.15.1)
ruby-lsp (0.12.0)
prism (< 0.24.0)
tapioca (0.11.10)
prism (< 0.24.0)
PLATFORMS
#{lockfile_platforms}
DEPENDENCIES
ruby-lsp
tapioca
BUNDLED WITH
#{Bundler::VERSION}
L

bundle "lock --update tapioca --verbose"

expect(lockfile).to include("tapioca (0.13.1)")
end

it "updates specific gems using --update, even if that requires unlocking other top level gems, but only as few as possible" do
build_repo4 do
build_gem "prism", "0.15.1"
build_gem "prism", "0.24.0"

build_gem "ruby-lsp", "0.12.0" do |s|
s.add_dependency "prism", "< 0.24.0"
end

build_gem "ruby-lsp", "0.16.1" do |s|
s.add_dependency "prism", ">= 0.24.0"
end

build_gem "tapioca", "0.11.10" do |s|
s.add_dependency "prism", "< 0.24.0"
end

build_gem "tapioca", "0.13.1" do |s|
s.add_dependency "prism", ">= 0.24.0"
end

build_gem "other-prism-dependent", "1.0.0" do |s|
s.add_dependency "prism", ">= 0.15.1"
end

build_gem "other-prism-dependent", "1.1.0" do |s|
s.add_dependency "prism", ">= 0.15.1"
end
end

gemfile <<~G
source "#{file_uri_for(gem_repo4)}"
gem "tapioca"
gem "ruby-lsp"
gem "other-prism-dependent"
G

lockfile <<~L
GEM
remote: #{file_uri_for(gem_repo4)}
specs:
other-prism-dependent (1.0.0)
prism (>= 0.15.1)
prism (0.15.1)
ruby-lsp (0.12.0)
prism (< 0.24.0)
tapioca (0.11.10)
prism (< 0.24.0)
PLATFORMS
#{lockfile_platforms}
DEPENDENCIES
ruby-lsp
tapioca
BUNDLED WITH
#{Bundler::VERSION}
L

bundle "lock --update tapioca"

expect(lockfile).to include("tapioca (0.13.1)")
expect(lockfile).to include("other-prism-dependent (1.0.0)")
end

it "preserves unknown checksum algorithms" do
lockfile @lockfile.gsub(/(sha256=[a-f0-9]+)$/, "constant=true,\\1,xyz=123")

Expand Down