Lazily check incomplete lockfile to improve performance

.github/workflows/truffleruby-bundler.yml

@@ -32,3 +32,4 @@ jobs:
         run: |
           bin/parallel_rspec --tag truffleruby_only --tag truffleruby
         working-directory: ./bundler
+    timeout-minutes: 20

Manifest.txt

@@ -80,6 +80,7 @@ bundler/lib/bundler/gem_helpers.rb
 bundler/lib/bundler/gem_tasks.rb
 bundler/lib/bundler/gem_version_promoter.rb
 bundler/lib/bundler/graph.rb
+bundler/lib/bundler/incomplete_specification.rb
 bundler/lib/bundler/index.rb
 bundler/lib/bundler/injector.rb
 bundler/lib/bundler/inline.rb

bundler/lib/bundler.rb

@@ -53,6 +53,7 @@ module Bundler
   autoload :GemHelpers,             File.expand_path("bundler/gem_helpers", __dir__)
   autoload :GemVersionPromoter,     File.expand_path("bundler/gem_version_promoter", __dir__)
   autoload :Graph,                  File.expand_path("bundler/graph", __dir__)
+  autoload :IncompleteSpecification, File.expand_path("bundler/incomplete_specification", __dir__)
   autoload :Index,                  File.expand_path("bundler/index", __dir__)
   autoload :Injector,               File.expand_path("bundler/injector", __dir__)
   autoload :Installer,              File.expand_path("bundler/installer", __dir__)

bundler/lib/bundler/definition.rb

@@ -138,13 +138,13 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
         @unlock[:gems] ||= @dependencies.map(&:name)
       else
         eager_unlock = expand_dependencies(@unlock[:gems] || [], true)
-        @unlock[:gems] = @locked_specs.for(eager_unlock, false, false).map(&:name)
+        @unlock[:gems] = @locked_specs.for(eager_unlock, false, platforms).map(&:name)
       end
 
       @dependency_changes = converge_dependencies
       @local_changes = converge_locals
 
-      @locked_specs_incomplete_for_platform = !@locked_specs.for(requested_dependencies & expand_dependencies(locked_dependencies), true, true)
+      @reresolve = nil
 
       @requires = compute_requires
     end
@@ -279,11 +279,8 @@ def resolve
           end
         end
       else
-        last_resolve = converge_locked_specs
-        # Run a resolve against the locally available gems
         Bundler.ui.debug("Found changes from the lockfile, re-resolving dependencies because #{change_reason}")
-        expanded_dependencies = expand_dependencies(dependencies + metadata_dependencies, true)
-        Resolver.resolve(expanded_dependencies, source_requirements, last_resolve, gem_version_promoter, additional_base_requirements_for_resolve, platforms)
+        @reresolve = reresolve
       end
     end
 
@@ -468,7 +465,7 @@ def most_specific_locked_platform
     private :sources
 
     def nothing_changed?
-      !@source_changes && !@dependency_changes && !@new_platform && !@path_changes && !@local_changes && !@locked_specs_incomplete_for_platform
+      !@source_changes && !@dependency_changes && !@new_platform && !@path_changes && !@local_changes
     end
 
     def unlocking?
@@ -477,8 +474,14 @@ def unlocking?
 
     private
 
+    def reresolve
+      last_resolve = converge_locked_specs
+      expanded_dependencies = expand_dependencies(dependencies + metadata_dependencies, true)
+      Resolver.resolve(expanded_dependencies, source_requirements, last_resolve, gem_version_promoter, additional_base_requirements_for_resolve, platforms)
+    end
+
     def filter_specs(specs, deps)
-      SpecSet.new(specs).for(expand_dependencies(deps, true), false, false)
+      SpecSet.new(specs).for(expand_dependencies(deps, true), false, platforms)
     end
 
     def materialize(dependencies)
@@ -502,6 +505,17 @@ def materialize(dependencies)
         raise GemNotFound, "Could not find #{missing_specs_list.join(" nor ")}"
       end
 
+      if @reresolve.nil?
+        incomplete_specs = specs.incomplete_specs
+
+        if incomplete_specs.any?
+          Bundler.ui.debug("The lockfile does not have all gems needed for the current platform though, Bundler will still re-resolve dependencies")
+          @unlock[:gems].concat(incomplete_specs.map(&:name))
+          @resolve = reresolve
+          specs = resolve.materialize(dependencies)
+        end
+      end
+
       unless specs["bundler"].any?
         bundler = sources.metadata_source.specs.search(Gem::Dependency.new("bundler", VERSION)).last
         specs["bundler"] = bundler
@@ -549,7 +563,6 @@ def change_reason
         [@new_platform, "you added a new platform to your gemfile"],
         [@path_changes, "the gemspecs for path gems changed"],
         [@local_changes, "the gemspecs for git local gems changed"],
-        [@locked_specs_incomplete_for_platform, "the lockfile does not have all gems needed for the current platform"],
       ].select(&:first).map(&:last).join(", ")
     end
 
@@ -725,7 +738,7 @@ def converge_specs(specs)
             # if we won't need the source (according to the lockfile),
             # don't error if the path/git source isn't available
             next if specs.
-                    for(requested_dependencies, false, true).
+                    for(requested_dependencies, false).
                     none? {|locked_spec| locked_spec.source == s.source }
 
             raise

bundler/lib/bundler/incomplete_specification.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module Bundler
+  class IncompleteSpecification
+    attr_reader :name, :platform
+
+    def initialize(name, platform)
+      @name = name
+      @platform = platform
+    end
+  end
+end

bundler/lib/bundler/resolver.rb

@@ -22,7 +22,7 @@ def self.resolve(requirements, source_requirements = {}, base = [], gem_version_
       metadata_requirements, regular_requirements = requirements.partition {|dep| dep.name.end_with?("\0") }
       resolver = new(source_requirements, base, gem_version_promoter, additional_base_requirements, platforms, metadata_requirements)
       result = resolver.start(requirements)
-      SpecSet.new(SpecSet.new(result).for(regular_requirements))
+      SpecSet.new(SpecSet.new(result).for(regular_requirements, false, platforms))
     end
 
     def initialize(source_requirements, base, gem_version_promoter, additional_base_requirements, platforms, metadata_requirements)

bundler/lib/bundler/spec_set.rb

@@ -11,40 +11,35 @@ def initialize(specs)
       @specs = specs
     end
 
-    def for(dependencies, check = false, match_current_platform = false)
-      # dep.name => [list, of, deps]
-      handled = Hash.new {|h, k| h[k] = [] }
-      deps = dependencies.dup
+    def for(dependencies, check = false, platforms = [nil])
+      handled = ["bundler"].product(platforms).map {|k| [k, true] }.to_h
+      deps = dependencies.product(platforms).map {|dep, platform| [dep.name, platform && dep.force_ruby_platform ? Gem::Platform::RUBY : platform] }
       specs = []
 
       loop do
         break unless dep = deps.shift
-        next if handled[dep.name].any? {|d| match_current_platform || d.__platform == dep.__platform } || dep.name == "bundler"
+        next if handled.key?(dep)
 
-        # use a hash here to ensure constant lookup time in the `any?` call above
-        handled[dep.name] << dep
+        handled[dep] = true
 
-        specs_for_dep = specs_for_dependency(dep, match_current_platform)
+        specs_for_dep = specs_for_dependency(*dep)
         if specs_for_dep.any?
           specs.concat(specs_for_dep)
 
           specs_for_dep.first.dependencies.each do |d|
             next if d.type == :development
-            d = DepProxy.get_proxy(Dependency.new(d.name, d.requirement), dep.__platform) unless match_current_platform
-            deps << d
+            deps << [d.name, dep[1]]
           end
         elsif check
-          return false
+          specs << IncompleteSpecification.new(*dep)
         end
       end
 
       if spec = lookup["bundler"].first
         specs << spec
       end
 
-      specs.uniq! unless match_current_platform
-
-      check ? true : specs
+      specs
     end
 
     def [](key)
@@ -71,7 +66,7 @@ def to_hash
     end
 
     def materialize(deps)
-      materialized = self.for(deps, false, true)
+      materialized = self.for(deps, true).uniq
 
       materialized.map! do |s|
         next s unless s.is_a?(LazySpecification)
@@ -99,6 +94,10 @@ def missing_specs
       @specs.select {|s| s.is_a?(LazySpecification) }
     end
 
+    def incomplete_specs
+      @specs.select {|s| s.is_a?(IncompleteSpecification) }
+    end
+
     def merge(set)
       arr = sorted.dup
       set.each do |set_spec|
@@ -173,12 +172,12 @@ def tsort_each_node
       @specs.sort_by(&:name).each {|s| yield s }
     end
 
-    def specs_for_dependency(dep, match_current_platform)
-      specs_for_name = lookup[dep.name]
-      if match_current_platform
+    def specs_for_dependency(name, platform)
+      specs_for_name = lookup[name]
+      if platform.nil?
         GemHelpers.select_best_platform_match(specs_for_name.select {|s| Gem::Platform.match_spec?(s) }, Bundler.local_platform)
       else
-        specs_for_name_and_platform = GemHelpers.select_best_platform_match(specs_for_name, dep.force_ruby_platform ? Gem::Platform::RUBY : dep.__platform)
+        specs_for_name_and_platform = GemHelpers.select_best_platform_match(specs_for_name, platform)
         specs_for_name_and_platform.any? ? specs_for_name_and_platform : specs_for_name
       end
     end

bundler/spec/runtime/platform_spec.rb

@@ -408,6 +408,7 @@
         gem "requires_platform_specific"
       G
 
+      expect(out).to include("lockfile does not have all gems needed for the current platform")
       expect(the_bundle).to include_gem "platform_specific 1.0 x64-mingw32"
     end
   end

bundler/spec/runtime/setup_spec.rb

@@ -634,6 +634,7 @@ def clean_load_path(lp)
 
       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(out).not_to include("lockfile does not have all gems needed for the current platform")
       expect(err).to be_empty
     end