Merge pull request #7307 from rubygems/fix-incorrect-lockfile

Fix incorrect lockfiles being generated in some situations

Author: deivid-rodriguez

bundler/lib/bundler/definition.rb

@@ -722,17 +722,14 @@ def check_lockfile
       @locked_spec_with_invalid_deps = nil
       @locked_spec_with_missing_deps = nil
 
-      locked_names = @locked_specs.map(&:name)
       missing = []
       invalid = []
 
       @locked_specs.each do |s|
-        s.dependencies.each do |dep|
-          next if dep.name == "bundler"
+        validation = @locked_specs.validate_deps(s)
 
-          missing << s unless locked_names.include?(dep.name)
-          invalid << s if @locked_specs.none? {|spec| dep.matches_spec?(spec) }
-        end
+        missing << s if validation == :missing
+        invalid << s if validation == :invalid
       end
 
       if missing.any?

bundler/lib/bundler/lazy_specification.rb

@@ -10,6 +10,8 @@ class LazySpecification
     attr_reader :name, :version, :platform
     attr_accessor :source, :remote, :force_ruby_platform, :dependencies, :required_ruby_version, :required_rubygems_version
 
+    alias_method :runtime_dependencies, :dependencies
+
     def self.from_spec(s)
       lazy_spec = new(s.name, s.version, s.platform, s.source)
       lazy_spec.dependencies = s.dependencies

bundler/lib/bundler/remote_specification.rb

@@ -88,6 +88,10 @@ def dependencies
       end
     end
 
+    def runtime_dependencies
+      dependencies.select(&:runtime?)
+    end
+
     def git_version
       return unless loaded_from && source.is_a?(Bundler::Source::Git)
       " #{source.revision[0..6]}"

bundler/lib/bundler/spec_set.rb

@@ -37,15 +37,15 @@ def for(dependencies, check = false, platforms = [nil])
 
           specs_for_dep.first.dependencies.each do |d|
             next if d.type == :development
-            incomplete = true if d.name != "bundler" && lookup[d.name].empty?
+            incomplete = true if d.name != "bundler" && lookup[d.name].nil?
             deps << [d, dep[1]]
           end
         else
           incomplete = true
         end
 
         if incomplete && check
-          @incomplete_specs += lookup[name].any? ? lookup[name] : [LazySpecification.new(name, nil, nil)]
+          @incomplete_specs += lookup[name] || [LazySpecification.new(name, nil, nil)]
         end
       end
 
@@ -64,7 +64,9 @@ def complete_platforms!(platforms)
         valid_platform = lookup.all? do |_, specs|
           spec = specs.first
           matching_specs = spec.source.specs.search([spec.name, spec.version])
-          platform_spec = GemHelpers.select_best_platform_match(matching_specs, platform).find(&:matches_current_metadata?)
+          platform_spec = GemHelpers.select_best_platform_match(matching_specs, platform).find do |s|
+            s.matches_current_metadata? && valid_dependencies?(s)
+          end
 
           if platform_spec
             new_specs << LazySpecification.from_spec(platform_spec)
@@ -90,9 +92,20 @@ def complete_platforms!(platforms)
       platforms
     end
 
+    def validate_deps(s)
+      s.runtime_dependencies.each do |dep|
+        next if dep.name == "bundler"
+
+        return :missing unless names.include?(dep.name)
+        return :invalid if none? {|spec| dep.matches_spec?(spec) }
+      end
+
+      :valid
+    end
+
     def [](key)
       key = key.name if key.respond_to?(:name)
-      lookup[key].reverse
+      lookup[key]&.reverse || []
     end
 
     def []=(key, value)
@@ -167,7 +180,7 @@ def delete_by_name(name)
     end
 
     def what_required(spec)
-      unless req = find {|s| s.dependencies.any? {|d| d.type == :runtime && d.name == spec.name } }
+      unless req = find {|s| s.runtime_dependencies.any? {|d| d.name == spec.name } }
         return [spec]
       end
       what_required(req) << spec
@@ -193,8 +206,16 @@ def each(&b)
       sorted.each(&b)
     end
 
+    def names
+      lookup.keys
+    end
+
     private
 
+    def valid_dependencies?(s)
+      validate_deps(s) == :valid
+    end
+
     def sorted
       rake = @specs.find {|s| s.name == "rake" }
       begin
@@ -213,8 +234,9 @@ def extract_circular_gems(error)
 
     def lookup
       @lookup ||= begin
-        lookup = Hash.new {|h, k| h[k] = [] }
+        lookup = {}
         @specs.each do |s|
+          lookup[s.name] ||= []
           lookup[s.name] << s
         end
         lookup
@@ -228,6 +250,8 @@ def tsort_each_node
 
     def specs_for_dependency(dep, platform)
       specs_for_name = lookup[dep.name]
+      return [] unless specs_for_name
+
       matching_specs = if dep.force_ruby_platform
         GemHelpers.force_ruby_platform(specs_for_name)
       else
@@ -240,7 +264,11 @@ def specs_for_dependency(dep, platform)
     def tsort_each_child(s)
       s.dependencies.sort_by(&:name).each do |d|
         next if d.type == :development
-        lookup[d.name].each {|s2| yield s2 }
+
+        specs_for_name = lookup[d.name]
+        next unless specs_for_name
+
+        specs_for_name.each {|s2| yield s2 }
       end
     end
   end

bundler/spec/install/gemfile/specific_platform_spec.rb

@@ -1220,6 +1220,48 @@
     end
   end
 
+  it "does not add ruby platform gem if it brings extra dependencies not resolved originally" do
+    build_repo4 do
+      build_gem "nokogiri", "1.15.5" do |s|
+        s.add_dependency "mini_portile2", "~> 2.8.2"
+      end
+
+      build_gem "nokogiri", "1.15.5" do |s|
+        s.platform = "x86_64-linux"
+      end
+    end
+
+    gemfile <<~G
+      source "#{file_uri_for(gem_repo4)}"
+
+      gem "nokogiri"
+    G
+
+    checksums = checksums_section_when_existing do |c|
+      c.checksum gem_repo4, "nokogiri", "1.15.5", "x86_64-linux"
+    end
+
+    simulate_platform "x86_64-linux" do
+      bundle "install --verbose", artifice: "compact_index", env: { "BUNDLER_SPEC_GEM_REPO" => gem_repo4.to_s }
+
+      expect(lockfile).to eq(<<~L)
+        GEM
+          remote: #{file_uri_for(gem_repo4)}/
+          specs:
+            nokogiri (1.15.5-x86_64-linux)
+
+        PLATFORMS
+          x86_64-linux
+
+        DEPENDENCIES
+          nokogiri
+        #{checksums}
+        BUNDLED WITH
+           #{Bundler::VERSION}
+      L
+    end
+  end
+
   private
 
   def setup_multiplatform_gem

bundler/spec/support/artifice/helpers/compact_index.rb

@@ -77,7 +77,7 @@ def gems(gem_repo = default_gem_repo)
 
         specs.group_by(&:name).map do |name, versions|
           gem_versions = versions.map do |spec|
-            deps = spec.dependencies.select {|d| d.type == :runtime }.map do |d|
+            deps = spec.runtime_dependencies.map do |d|
               reqs = d.requirement.requirements.map {|r| r.join(" ") }.join(", ")
               CompactIndex::Dependency.new(d.name, reqs)
             end

bundler/spec/support/artifice/helpers/endpoint.rb

@@ -72,7 +72,7 @@ def dependencies_for(gem_names, gem_repo = default_gem_repo)
           name: spec.name,
           number: spec.version.version,
           platform: spec.platform.to_s,
-          dependencies: spec.dependencies.select {|dep| dep.type == :runtime }.map do |dep|
+          dependencies: spec.runtime_dependencies.map do |dep|
             [dep.name, dep.requirement.requirements.map {|a| a.join(" ") }.join(", ")]
           end,
         }