From 78f68cf4b753eacfaaefa827456ec135601aa439 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Fri, 9 Jul 2021 20:38:29 +0200 Subject: [PATCH 1/7] Improve `SpecSet#for` uniqueness Short-circuit duplications earlier. --- bundler/lib/bundler/spec_set.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bundler/lib/bundler/spec_set.rb b/bundler/lib/bundler/spec_set.rb index 13cee734c3e5..2e8ae7424877 100644 --- a/bundler/lib/bundler/spec_set.rb +++ b/bundler/lib/bundler/spec_set.rb @@ -18,13 +18,13 @@ def for(dependencies, check = false, match_current_platform = false, raise_on_mi loop do break unless dep = deps.shift - next if handled.include?(dep) + next if handled.any?{|d| d.name == dep.name && (match_current_platform || d.__platform == dep.__platform) } handled << dep specs_for_dep = spec_for_dependency(dep, match_current_platform) if specs_for_dep.any? - specs |= specs_for_dep + specs += specs_for_dep specs_for_dep.first.dependencies.each do |d| next if d.type == :development From a6e2b723709ec89a31837743a84bcb6a64553fe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Fri, 9 Jul 2021 23:03:18 +0200 Subject: [PATCH 2/7] Refactor group manipulation inside definition --- bundler/lib/bundler/definition.rb | 11 +++-------- bundler/lib/bundler/installer/standalone.rb | 2 +- bundler/lib/bundler/runtime.rb | 2 -- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/bundler/lib/bundler/definition.rb b/bundler/lib/bundler/definition.rb index 486f469bf8ee..1ab730406028 100644 --- a/bundler/lib/bundler/definition.rb +++ b/bundler/lib/bundler/definition.rb @@ -240,17 +240,11 @@ def missing_specs? end def requested_specs - @requested_specs ||= begin - groups = requested_groups - groups.map!(&:to_sym) - specs_for(groups) - end + @requested_specs ||= specs_for(requested_groups) end def requested_dependencies - groups = requested_groups - groups.map!(&:to_sym) - dependencies_for(groups) + dependencies_for(requested_groups) end def current_dependencies @@ -265,6 +259,7 @@ def specs_for(groups) end def dependencies_for(groups) + groups.map!(&:to_sym) current_dependencies.reject do |d| (d.groups & groups).empty? end diff --git a/bundler/lib/bundler/installer/standalone.rb b/bundler/lib/bundler/installer/standalone.rb index 2a3f5cfe3585..4f5aa93a4a4e 100644 --- a/bundler/lib/bundler/installer/standalone.rb +++ b/bundler/lib/bundler/installer/standalone.rb @@ -3,7 +3,7 @@ module Bundler class Standalone def initialize(groups, definition) - @specs = groups.empty? ? definition.requested_specs : definition.specs_for(groups.map(&:to_sym)) + @specs = groups.empty? ? definition.requested_specs : definition.specs_for(groups) end def generate diff --git a/bundler/lib/bundler/runtime.rb b/bundler/lib/bundler/runtime.rb index e259b590bfc7..a8f97b3fbcbc 100644 --- a/bundler/lib/bundler/runtime.rb +++ b/bundler/lib/bundler/runtime.rb @@ -12,8 +12,6 @@ def initialize(root, definition) def setup(*groups) @definition.ensure_equivalent_gemfile_and_lockfile if Bundler.frozen_bundle? - groups.map!(&:to_sym) - # Has to happen first clean_load_path From 043ab2faf894e08944f940a91b4573aadb402d87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Sun, 11 Jul 2021 14:00:36 +0200 Subject: [PATCH 3/7] Restore special handling for bundler when materializing Removing it was causing unnecessary re-resolves if you have a pre-release version of bundler installed. --- bundler/lib/bundler/spec_set.rb | 4 ++-- bundler/spec/runtime/setup_spec.rb | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/bundler/lib/bundler/spec_set.rb b/bundler/lib/bundler/spec_set.rb index 2e8ae7424877..1a8906c47ee4 100644 --- a/bundler/lib/bundler/spec_set.rb +++ b/bundler/lib/bundler/spec_set.rb @@ -18,7 +18,7 @@ def for(dependencies, check = false, match_current_platform = false, raise_on_mi loop do break unless dep = deps.shift - next if handled.any?{|d| d.name == dep.name && (match_current_platform || d.__platform == dep.__platform) } + next if handled.any?{|d| d.name == dep.name && (match_current_platform || d.__platform == dep.__platform) } || dep.name == "bundler" handled << dep @@ -42,7 +42,7 @@ def for(dependencies, check = false, match_current_platform = false, raise_on_mi end if spec = lookup["bundler"].first - specs |= [spec] + specs << spec end check ? true : specs diff --git a/bundler/spec/runtime/setup_spec.rb b/bundler/spec/runtime/setup_spec.rb index d8ba569f0abb..380db991364d 100644 --- a/bundler/spec/runtime/setup_spec.rb +++ b/bundler/spec/runtime/setup_spec.rb @@ -644,6 +644,25 @@ def clean_load_path(lp) expect(err).to be_empty end + it "doesn't re-resolve when a pre-release bundler is used and a dependency includes a dependency on bundler" do + system_gems "bundler-9.99.9.beta1" + + build_repo4 do + build_gem "depends_on_bundler", "1.0" do |s| + s.add_dependency "bundler", ">= 1.5.0" + end + end + + install_gemfile <<~G + source "#{file_uri_for(gem_repo4)}" + gem "depends_on_bundler" + G + + ruby "require '#{system_gem_path("gems/bundler-9.99.9.beta1/lib/bundler.rb")}'; Bundler.setup", :env => { "DEBUG" => "1" } + expect(out).to include("Found no changes, using resolution from the lockfile") + expect(err).to be_empty + end + it "remembers --without and does not include groups passed to Bundler.setup" do bundle "config set --local without rails" install_gemfile <<-G From 88350fb44fdd55c5224c49d5ad62395f45771260 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Sun, 11 Jul 2021 19:48:09 +0200 Subject: [PATCH 4/7] No need to memoize requested specs The method is called at most once per invocation. --- bundler/lib/bundler/definition.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundler/lib/bundler/definition.rb b/bundler/lib/bundler/definition.rb index 1ab730406028..47b67bea3fbe 100644 --- a/bundler/lib/bundler/definition.rb +++ b/bundler/lib/bundler/definition.rb @@ -240,7 +240,7 @@ def missing_specs? end def requested_specs - @requested_specs ||= specs_for(requested_groups) + specs_for(requested_groups) end def requested_dependencies From 051ab488b3ce105cff3ea5555eb7dd427f94ec2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Sun, 11 Jul 2021 20:07:27 +0200 Subject: [PATCH 5/7] Move group selection logic to `Definition#specs_for` --- bundler/lib/bundler/definition.rb | 1 + bundler/lib/bundler/installer/standalone.rb | 2 +- bundler/lib/bundler/runtime.rb | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/bundler/lib/bundler/definition.rb b/bundler/lib/bundler/definition.rb index 47b67bea3fbe..7e6331e613e3 100644 --- a/bundler/lib/bundler/definition.rb +++ b/bundler/lib/bundler/definition.rb @@ -254,6 +254,7 @@ def current_dependencies end def specs_for(groups) + groups = requested_groups if groups.empty? deps = dependencies_for(groups) SpecSet.new(specs.for(expand_dependencies(deps))) end diff --git a/bundler/lib/bundler/installer/standalone.rb b/bundler/lib/bundler/installer/standalone.rb index 4f5aa93a4a4e..f16135cb481d 100644 --- a/bundler/lib/bundler/installer/standalone.rb +++ b/bundler/lib/bundler/installer/standalone.rb @@ -3,7 +3,7 @@ module Bundler class Standalone def initialize(groups, definition) - @specs = groups.empty? ? definition.requested_specs : definition.specs_for(groups) + @specs = definition.specs_for(groups) end def generate diff --git a/bundler/lib/bundler/runtime.rb b/bundler/lib/bundler/runtime.rb index a8f97b3fbcbc..287fa1cfe94b 100644 --- a/bundler/lib/bundler/runtime.rb +++ b/bundler/lib/bundler/runtime.rb @@ -15,7 +15,7 @@ def setup(*groups) # Has to happen first clean_load_path - specs = groups.any? ? @definition.specs_for(groups) : requested_specs + specs = @definition.specs_for(groups) SharedHelpers.set_bundle_environment Bundler.rubygems.replace_entrypoints(specs) From 1b22ccb100b9d81d5e5e8dc44d4f0debe5ca7413 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Sun, 11 Jul 2021 20:21:28 +0200 Subject: [PATCH 6/7] Extract method to add bundler to set of materialized specs To be reused later. --- bundler/lib/bundler/definition.rb | 37 +++++++++++++++---------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/bundler/lib/bundler/definition.rb b/bundler/lib/bundler/definition.rb index 7e6331e613e3..0b682905b54b 100644 --- a/bundler/lib/bundler/definition.rb +++ b/bundler/lib/bundler/definition.rb @@ -190,25 +190,15 @@ def resolve_remotely! # # @return [Bundler::SpecSet] def specs - @specs ||= begin - begin - specs = resolve.materialize(requested_dependencies) - rescue GemNotFound => e # Handle yanked gem - gem_name, gem_version = extract_gem_info(e) - locked_gem = @locked_specs[gem_name].last - raise if locked_gem.nil? || locked_gem.version.to_s != gem_version || !@remote - raise GemNotFound, "Your bundle is locked to #{locked_gem} from #{locked_gem.source}, but that version can " \ - "no longer be found in that source. That means the author of #{locked_gem} has removed it. " \ - "You'll need to update your bundle to a version other than #{locked_gem} that hasn't been " \ - "removed in order to install." - end - unless specs["bundler"].any? - bundler = sources.metadata_source.specs.search(Gem::Dependency.new("bundler", VERSION)).last - specs["bundler"] = bundler - end - - specs - end + @specs ||= add_bundler_to(resolve.materialize(requested_dependencies)) + rescue GemNotFound => e # Handle yanked gem + gem_name, gem_version = extract_gem_info(e) + locked_gem = @locked_specs[gem_name].last + raise if locked_gem.nil? || locked_gem.version.to_s != gem_version || !@remote + raise GemNotFound, "Your bundle is locked to #{locked_gem} from #{locked_gem.source}, but that version can " \ + "no longer be found in that source. That means the author of #{locked_gem} has removed it. " \ + "You'll need to update your bundle to a version other than #{locked_gem} that hasn't been " \ + "removed in order to install." end def new_specs @@ -510,6 +500,15 @@ def unlocking? private + def add_bundler_to(specs) + unless specs["bundler"].any? + bundler = sources.metadata_source.specs.search(Gem::Dependency.new("bundler", VERSION)).last + specs["bundler"] = bundler + end + + specs + end + def precompute_source_requirements_for_indirect_dependencies? sources.non_global_rubygems_sources.all?(&:dependency_api_available?) && !sources.aggregate_global_source? end From eb548357029d7705a0343596f265884ff0944258 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Sun, 11 Jul 2021 15:13:42 +0200 Subject: [PATCH 7/7] Do less work when setting up bundler Previous version was first materializing the full set of specs, then re-materializing the "filtered" set of specs needed. We can do the latter directly. --- bundler/lib/bundler/definition.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundler/lib/bundler/definition.rb b/bundler/lib/bundler/definition.rb index 0b682905b54b..b585b06df102 100644 --- a/bundler/lib/bundler/definition.rb +++ b/bundler/lib/bundler/definition.rb @@ -246,7 +246,7 @@ def current_dependencies def specs_for(groups) groups = requested_groups if groups.empty? deps = dependencies_for(groups) - SpecSet.new(specs.for(expand_dependencies(deps))) + add_bundler_to(resolve.materialize(expand_dependencies(deps))) end def dependencies_for(groups)