Skip to content

Commit

Permalink
Merge pull request #7558 from rubygems/update-until-really-updates
Browse files Browse the repository at this point in the history
Make sure `bundle update <specific_gems>` can always update to the latest resolvable version of each requested gem
  • Loading branch information
deivid-rodriguez committed Apr 29, 2024
2 parents 0ecd098 + 01c0bf3 commit d95430f
Show file tree
Hide file tree
Showing 2 changed files with 175 additions and 24 deletions.
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

0 comments on commit d95430f

Please sign in to comment.