ruby-changes:72841
From: David <ko1@a...>
Date: Sat, 6 Aug 2022 15:42:03 +0900 (JST)
Subject: [ruby-changes:72841] 466a760e18 (master): [rubygems/rubygems] Fix yanked gems being unintentionally update when other gems are unlocked
https://git.ruby-lang.org/ruby.git/commit/?id=466a760e18 From 466a760e1807e629d0ec9f9ebf160d3c3f649d04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@r...> Date: Wed, 3 Aug 2022 19:03:50 +0200 Subject: [rubygems/rubygems] Fix yanked gems being unintentionally update when other gems are unlocked This is a regression from a change intended to raise errors when user puts a gem under an incorrect source in the Gemfile by mistake. To fix the issue, we revert the change that caused it and implement it in a different way that restores the resolver independency from real specifications. Now it deals only with names and versions and does not try to materialize anything into real specifications before resolving. https://github.com/rubygems/rubygems/commit/d2bf1b86eb --- lib/bundler/definition.rb | 22 +++++++++++++- lib/bundler/lazy_specification.rb | 8 ------ lib/bundler/resolver.rb | 3 +- lib/bundler/spec_set.rb | 9 ------ spec/bundler/install/yanked_spec.rb | 57 +++++++++++++++++++++++++++++++++++++ spec/bundler/resolver/basic_spec.rb | 2 +- 6 files changed, 81 insertions(+), 20 deletions(-) diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb index 536099a20a..38bd01f08f 100644 --- a/lib/bundler/definition.rb +++ b/lib/bundler/definition.rb @@ -739,11 +739,22 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/lib/bundler/definition.rb#L739 specs[dep].any? {|s| s.satisfies?(dep) && (!dep.source || s.source.include?(dep.source)) } end + @specs_that_changed_sources = [] + specs.each do |s| dep = @dependencies.find {|d| s.satisfies?(d) } # Replace the locked dependency's source with the equivalent source from the Gemfile - s.source = (dep && dep.source) || sources.get_with_fallback(s.source) + s.source = if dep && dep.source + gemfile_source = dep.source + lockfile_source = s.source + + @specs_that_changed_sources << s if gemfile_source != lockfile_source + + gemfile_source + else + sources.get_with_fallback(s.source) + end next if @unlock[:sources].include?(s.source.name) @@ -821,9 +832,18 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/lib/bundler/definition.rb#L832 end source_requirements[:default_bundler] = source_requirements["bundler"] || sources.default_source source_requirements["bundler"] = sources.metadata_source # needs to come last to override + verify_changed_sources! source_requirements end + def verify_changed_sources! + @specs_that_changed_sources.each do |s| + if s.source.specs.search(s.name).empty? + raise GemNotFound, "Could not find gem '#{s.name}' in #{s.source}" + end + end + end + def requested_groups values = groups - Bundler.settings[:without] - @optional_groups + Bundler.settings[:with] values &= Bundler.settings[:only] unless Bundler.settings[:only].empty? diff --git a/lib/bundler/lazy_specification.rb b/lib/bundler/lazy_specification.rb index 9f75c7bab2..5b40bec5a8 100644 --- a/lib/bundler/lazy_specification.rb +++ b/lib/bundler/lazy_specification.rb @@ -93,14 +93,6 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/lib/bundler/lazy_specification.rb#L93 __materialize__(candidates) end - def materialize_for_resolution - return self unless Gem::Platform.match_spec?(self) - - candidates = source.specs.search(self) - - __materialize__(candidates) - end - def __materialize__(candidates) @specification = begin search = candidates.reverse.find do |spec| diff --git a/lib/bundler/resolver.rb b/lib/bundler/resolver.rb index 40bc247b32..ca1bdbda7b 100644 --- a/lib/bundler/resolver.rb +++ b/lib/bundler/resolver.rb @@ -28,10 +28,11 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/lib/bundler/resolver.rb#L28 def initialize(source_requirements, base, gem_version_promoter, additional_base_requirements, platforms, metadata_requirements) @source_requirements = source_requirements @metadata_requirements = metadata_requirements + @base = base @resolver = Molinillo::Resolver.new(self, self) @search_for = {} @base_dg = Molinillo::DependencyGraph.new - @base = base.materialized_for_resolution do |ls| + base.each do |ls| dep = Dependency.new(ls.name, ls.version) @base_dg.add_vertex(ls.name, DepProxy.get_proxy(dep, ls.platform), true) end diff --git a/lib/bundler/spec_set.rb b/lib/bundler/spec_set.rb index d7239f5c8c..14733269d6 100644 --- a/lib/bundler/spec_set.rb +++ b/lib/bundler/spec_set.rb @@ -82,15 +82,6 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/lib/bundler/spec_set.rb#L82 end end - def materialized_for_resolution - materialized = @specs.map do |s| - spec = s.materialize_for_resolution - yield spec if spec - spec - end.compact - SpecSet.new(materialized) - end - def incomplete_ruby_specs?(deps) self.class.new(self.for(deps, true, [Gem::Platform::RUBY])).incomplete_specs.any? end diff --git a/spec/bundler/install/yanked_spec.rb b/spec/bundler/install/yanked_spec.rb index 44fbb0bda3..09a5ba0be1 100644 --- a/spec/bundler/install/yanked_spec.rb +++ b/spec/bundler/install/yanked_spec.rb @@ -43,6 +43,63 @@ RSpec.context "when installing a bundle that includes yanked gems" do https://github.com/ruby/ruby/blob/trunk/spec/bundler/install/yanked_spec.rb#L43 end end +RSpec.context "when resolving a bundle that includes yanked gems, but unlocking an unrelated gem" do + before(:each) do + build_repo4 do + build_gem "foo", "10.0.0" + + build_gem "bar", "1.0.0" + build_gem "bar", "2.0.0" + end + + lockfile <<-L + GEM + remote: #{file_uri_for(gem_repo4)} + specs: + foo (9.0.0) + bar (1.0.0) + + PLATFORMS + #{lockfile_platforms} + + DEPENDENCIES + foo + bar + + BUNDLED WITH + #{Bundler::VERSION} + L + + gemfile <<-G + source "#{file_uri_for(gem_repo4)}" + gem "foo" + gem "bar" + G + end + + it "does not update the yanked gem" do + bundle "lock --update bar" + + expect(lockfile).to eq <<~L + GEM + remote: #{file_uri_for(gem_repo4)}/ + specs: + bar (2.0.0) + foo (9.0.0) + + PLATFORMS + #{lockfile_platforms} + + DEPENDENCIES + bar + foo + + BUNDLED WITH + #{Bundler::VERSION} + L + end +end + RSpec.context "when using gem before installing" do it "does not suggest the author has yanked the gem" do gemfile <<-G diff --git a/spec/bundler/resolver/basic_spec.rb b/spec/bundler/resolver/basic_spec.rb index 7182d1e29c..ee62dc3577 100644 --- a/spec/bundler/resolver/basic_spec.rb +++ b/spec/bundler/resolver/basic_spec.rb @@ -233,7 +233,7 @@ Bundler could not find compatible versions for gem "a": https://github.com/ruby/ruby/blob/trunk/spec/bundler/resolver/basic_spec.rb#L233 it "resolves foo only to latest patch - changing dependency declared case" do # bar is locked AND a declared dependency in the Gemfile, so it will not move, and therefore # foo can only move up to 1.4.4. - @base << Bundler::LazySpecification.new("bar", "2.0.3", nil) + @base << build_spec("bar", "2.0.3").first should_conservative_resolve_and_include :patch, ["foo"], %w[foo-1.4.4 bar-2.0.3] end -- cgit v1.2.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/