Merge pull request #8630 from rubygems/deivid-rodriguez/refuse-to-add-invalid-platforms

Fix `bundle lock` sometimes allowing invalid platforms into the lockfile

(cherry picked from commit 89b3776)

Author: deivid-rodriguez

bundler/lib/bundler/definition.rb

@@ -257,7 +257,7 @@ def missing_specs?
     rescue BundlerError => e
       @resolve = nil
       @resolver = nil
-      @resolution_packages = nil
+      @resolution_base = nil
       @source_requirements = nil
       @specs = nil
 
@@ -614,7 +614,7 @@ def write_lock(file, preserve_unknown_sections)
     end
 
     def resolver
-      @resolver ||= Resolver.new(resolution_packages, gem_version_promoter, @most_specific_locked_platform)
+      @resolver ||= Resolver.new(resolution_base, gem_version_promoter, @most_specific_locked_platform)
     end
 
     def expanded_dependencies
@@ -628,15 +628,15 @@ def dependencies_with_bundler
       [Dependency.new("bundler", @unlocking_bundler)] + dependencies
     end
 
-    def resolution_packages
-      @resolution_packages ||= begin
+    def resolution_base
+      @resolution_base ||= begin
         last_resolve = converge_locked_specs
         remove_invalid_platforms!
         new_resolution_platforms = @current_platform_missing ? @new_platforms + [local_platform] : @new_platforms
-        packages = Resolver::Base.new(source_requirements, expanded_dependencies, last_resolve, @platforms, locked_specs: @originally_locked_specs, unlock: @unlocking_all || @gems_to_unlock, prerelease: gem_version_promoter.pre?, prefer_local: @prefer_local, new_platforms: new_resolution_platforms)
-        packages = additional_base_requirements_to_prevent_downgrades(packages)
-        packages = additional_base_requirements_to_force_updates(packages)
-        packages
+        base = Resolver::Base.new(source_requirements, expanded_dependencies, last_resolve, @platforms, locked_specs: @originally_locked_specs, unlock: @unlocking_all || @gems_to_unlock, prerelease: gem_version_promoter.pre?, prefer_local: @prefer_local, new_platforms: new_resolution_platforms)
+        base = additional_base_requirements_to_prevent_downgrades(base)
+        base = additional_base_requirements_to_force_updates(base)
+        base
       end
     end
 
@@ -711,8 +711,7 @@ def materialize(dependencies)
         still_incomplete_specs = resolve.incomplete_specs
 
         if still_incomplete_specs == incomplete_specs
-          package = resolution_packages.get_package(incomplete_specs.first.name)
-          resolver.raise_not_found! package
+          resolver.raise_incomplete! incomplete_specs
         end
 
         incomplete_specs = still_incomplete_specs
@@ -734,7 +733,7 @@ def materialize(dependencies)
     end
 
     def reresolve_without(incomplete_specs)
-      resolution_packages.delete(incomplete_specs)
+      resolution_base.delete(incomplete_specs)
       @resolve = start_resolution
     end
 
@@ -747,6 +746,14 @@ def start_resolution
 
       @resolved_bundler_version = result.find {|spec| spec.name == "bundler" }&.version
 
+      @new_platforms.each do |platform|
+        incomplete_specs = result.incomplete_specs_for_platform(current_dependencies, platform)
+
+        if incomplete_specs.any?
+          resolver.raise_incomplete! incomplete_specs
+        end
+      end
+
       if @most_specific_non_local_locked_platform
         if spec_set_incomplete_for_platform?(result, @most_specific_non_local_locked_platform)
           @platforms.delete(@most_specific_non_local_locked_platform)
@@ -1124,27 +1131,27 @@ def lockfiles_equal?(current, proposed, preserve_unknown_sections)
       current == proposed
     end
 
-    def additional_base_requirements_to_prevent_downgrades(resolution_packages)
-      return resolution_packages unless @locked_gems && !sources.expired_sources?(@locked_gems.sources)
+    def additional_base_requirements_to_prevent_downgrades(resolution_base)
+      return resolution_base unless @locked_gems && !sources.expired_sources?(@locked_gems.sources)
       @originally_locked_specs.each do |locked_spec|
         next if locked_spec.source.is_a?(Source::Path)
 
         name = locked_spec.name
         next if @changed_dependencies.include?(name)
 
-        resolution_packages.base_requirements[name] = Gem::Requirement.new(">= #{locked_spec.version}")
+        resolution_base.base_requirements[name] = Gem::Requirement.new(">= #{locked_spec.version}")
       end
-      resolution_packages
+      resolution_base
     end
 
-    def additional_base_requirements_to_force_updates(resolution_packages)
-      return resolution_packages if @explicit_unlocks.empty?
+    def additional_base_requirements_to_force_updates(resolution_base)
+      return resolution_base if @explicit_unlocks.empty?
       full_update = dup_for_full_unlock.resolve
       @explicit_unlocks.each do |name|
         version = full_update.version_for(name)
-        resolution_packages.base_requirements[name] = Gem::Requirement.new("= #{version}") if version
+        resolution_base.base_requirements[name] = Gem::Requirement.new("= #{version}") if version
       end
-      resolution_packages
+      resolution_base
     end
 
     def dup_for_full_unlock

bundler/lib/bundler/resolver.rb

@@ -312,6 +312,16 @@ def name_for_explicit_dependency_source
       "Gemfile"
     end
 
+    def raise_incomplete!(incomplete_specs)
+      raise_not_found!(@base.get_package(incomplete_specs.first.name))
+    end
+
+    def sort_versions_by_preferred(package, versions)
+      @gem_version_promoter.sort_versions(package, versions)
+    end
+
+    private
+
     def raise_not_found!(package)
       name = package.name
       source = source_for(name)
@@ -348,12 +358,6 @@ def raise_not_found!(package)
       raise GemNotFound, message
     end
 
-    def sort_versions_by_preferred(package, versions)
-      @gem_version_promoter.sort_versions(package, versions)
-    end
-
-    private
-
     def filtered_versions_for(package)
       @gem_version_promoter.filter_versions(package, @all_versions[package])
     end

bundler/lib/bundler/spec_set.rb

@@ -130,12 +130,15 @@ def materialized_for_all_platforms
     end
 
     def incomplete_for_platform?(deps, platform)
-      return false if @specs.empty?
+      incomplete_specs_for_platform(deps, platform).any?
+    end
+
+    def incomplete_specs_for_platform(deps, platform)
+      return [] if @specs.empty?
 
       validation_set = self.class.new(@specs)
       validation_set.for(deps, [platform])
-
-      validation_set.incomplete_specs.any?
+      validation_set.incomplete_specs
     end
 
     def missing_specs_for(deps)

bundler/spec/commands/lock_spec.rb

@@ -1346,6 +1346,48 @@
     L
   end
 
+  it "refuses to add platforms incompatible with the lockfile" do
+    build_repo4 do
+      build_gem "sorbet-static", "0.5.11989" do |s|
+        s.platform = "x86_64-linux"
+      end
+    end
+
+    gemfile <<~G
+      source "https://gem.repo4"
+
+      gem "sorbet-static"
+    G
+
+    lockfile <<~L
+      GEM
+        remote: https://gem.repo4/
+        specs:
+          sorbet-static (0.5.11989-x86_64-linux)
+
+      PLATFORMS
+        x86_64-linux
+
+      DEPENDENCIES
+        sorbet-static
+
+      BUNDLED WITH
+         #{Bundler::VERSION}
+    L
+
+    simulate_platform "x86_64-linux" do
+      bundle "lock --add-platform ruby", raise_on_error: false
+    end
+
+    nice_error = <<~E.strip
+      Could not find gems matching 'sorbet-static' valid for all resolution platforms (x86_64-linux, ruby) in rubygems repository https://gem.repo4/ or installed locally.
+
+      The source contains the following gems matching 'sorbet-static':
+        * sorbet-static-0.5.11989-x86_64-linux
+    E
+    expect(err).to include(nice_error)
+  end
+
   it "does not crash on conflicting ruby requirements between platform versions in two different gems" do
     build_repo4 do
       build_gem "unf_ext", "0.0.8.2"