Merge pull request #7035 from rubygems/multiplatform-resolve-fix

Raise an error when adding a gem incompatible with some locked platform

Author: deivid-rodriguez

bundler/lib/bundler/definition.rb

@@ -452,8 +452,8 @@ def validate_platforms!
       return if current_platform_locked?
 
       raise ProductionError, "Your bundle only supports platforms #{@platforms.map(&:to_s)} " \
-        "but your local platform is #{Bundler.local_platform}. " \
-        "Add the current platform to the lockfile with\n`bundle lock --add-platform #{Bundler.local_platform}` and try again."
+        "but your local platform is #{local_platform}. " \
+        "Add the current platform to the lockfile with\n`bundle lock --add-platform #{local_platform}` and try again."
     end
 
     def add_platform(platform)
@@ -509,7 +509,7 @@ def dependencies_with_bundler
     def resolution_packages
       @resolution_packages ||= begin
         last_resolve = converge_locked_specs
-        remove_ruby_from_platforms_if_necessary!(current_dependencies)
+        remove_invalid_platforms!(current_dependencies)
         packages = Resolver::Base.new(source_requirements, expanded_dependencies, last_resolve, @platforms, :locked_specs => @originally_locked_specs, :unlock => @unlock[:gems], :prerelease => gem_version_promoter.pre?)
         additional_base_requirements_for_resolve(packages, last_resolve)
       end
@@ -600,7 +600,7 @@ def current_ruby_platform_locked?
 
     def current_platform_locked?
       @platforms.any? do |bundle_platform|
-        MatchPlatform.platforms_match?(bundle_platform, Bundler.local_platform)
+        MatchPlatform.platforms_match?(bundle_platform, local_platform)
       end
     end
 
@@ -956,17 +956,19 @@ def additional_base_requirements_for_resolve(resolution_packages, last_resolve)
       resolution_packages
     end
 
-    def remove_ruby_from_platforms_if_necessary!(dependencies)
-      return if Bundler.frozen_bundle? ||
-                Bundler.local_platform == Gem::Platform::RUBY ||
-                !platforms.include?(Gem::Platform::RUBY) ||
-                (@new_platform && platforms.last == Gem::Platform::RUBY) ||
+    def remove_invalid_platforms!(dependencies)
+      return if Bundler.frozen_bundle?
+
+      platforms.each do |platform|
+        next if local_platform == platform ||
+                (@new_platform && platforms.last == platform) ||
                 @path_changes ||
                 @dependency_changes ||
-                !@originally_locked_specs.incomplete_ruby_specs?(dependencies)
+                !@originally_locked_specs.incomplete_for_platform?(dependencies, platform)
 
-      remove_platform(Gem::Platform::RUBY)
-      add_current_platform
+        remove_platform(platform)
+        add_current_platform if platform == Gem::Platform::RUBY
+      end
     end
 
     def source_map

bundler/lib/bundler/resolver.rb

@@ -248,8 +248,22 @@ def all_versions_for(package)
       results = filter_matching_specs(results, locked_requirement) if locked_requirement
 
       versions = results.group_by(&:version).reduce([]) do |groups, (version, specs)|
-        platform_specs = package.platforms.flat_map {|platform| select_best_platform_match(specs, platform) }
-        next groups if platform_specs.empty?
+        platform_specs = package.platforms.map {|platform| select_best_platform_match(specs, platform) }
+
+        # If package is a top-level dependency,
+        #   candidate is only valid if there are matching versions for all resolution platforms.
+        #
+        # If package is not a top-level deependency,
+        #   then it's not necessary that it has matching versions for all platforms, since it may have been introduced only as
+        #   a dependency for a platform specific variant, so it will only need to have a valid version for that platform.
+        #
+        if package.top_level?
+          next groups if platform_specs.any?(&:empty?)
+        else
+          next groups if platform_specs.all?(&:empty?)
+        end
+
+        platform_specs.flatten!
 
         ruby_specs = select_best_platform_match(specs, Gem::Platform::RUBY)
         groups << Resolver::Candidate.new(version, :specs => ruby_specs) if ruby_specs.any?
@@ -295,15 +309,21 @@ def raise_not_found!(package)
                         end
       specs_matching_requirement = filter_matching_specs(specs, package.dependency.requirement)
 
-      if specs_matching_requirement.any?
+      not_found_message = if specs_matching_requirement.any?
         specs = specs_matching_requirement
         matching_part = requirement_label
         platforms = package.platforms
-        platform_label = platforms.size == 1 ? "platform '#{platforms.first}" : "platforms '#{platforms.join("', '")}"
-        requirement_label = "#{requirement_label}' with #{platform_label}"
+
+        if platforms.size == 1
+          "Could not find gem '#{requirement_label}' with platform '#{platforms.first}'"
+        else
+          "Could not find gems matching '#{requirement_label}' valid for all resolution platforms (#{platforms.join(", ")})"
+        end
+      else
+        "Could not find gem '#{requirement_label}'"
       end
 
-      message = String.new("Could not find gem '#{requirement_label}' in #{source}#{cache_message}.\n")
+      message = String.new("#{not_found_message} in #{source}#{cache_message}.\n")
 
       if specs.any?
         message << "\n#{other_specs_matching_message(specs, matching_part)}"

bundler/lib/bundler/resolver/package.rb

@@ -21,6 +21,7 @@ def initialize(name, platforms, locked_specs:, unlock:, prerelease: false, depen
         @locked_version = locked_specs[name].first&.version
         @unlock = unlock
         @dependency = dependency || Dependency.new(name, @locked_version)
+        @top_level = !dependency.nil?
         @prerelease = @dependency.prerelease? || @locked_version&.prerelease? || prerelease ? :consider_first : :ignore
       end
 
@@ -32,6 +33,10 @@ def root?
         false
       end
 
+      def top_level?
+        @top_level
+      end
+
       def meta?
         @name.end_with?("\0")
       end

bundler/lib/bundler/spec_set.rb

@@ -100,12 +100,12 @@ def materialized_for_all_platforms
       end
     end
 
-    def incomplete_ruby_specs?(deps)
+    def incomplete_for_platform?(deps, platform)
       return false if @specs.empty?
 
       @incomplete_specs = []
 
-      self.for(deps, true, [Gem::Platform::RUBY])
+      self.for(deps, true, [platform])
 
       @incomplete_specs.any?
     end

bundler/spec/install/gemfile/specific_platform_spec.rb

@@ -685,6 +685,70 @@
     L
   end
 
+  it "automatically fixes the lockfile if multiple platforms locked, but no valid versions of direct dependencies for all of them" do
+    simulate_platform "x86_64-linux" do
+      build_repo4 do
+        build_gem "nokogiri", "1.14.0" do |s|
+          s.platform = "x86_64-linux"
+        end
+        build_gem "nokogiri", "1.14.0" do |s|
+          s.platform = "arm-linux"
+        end
+
+        build_gem "sorbet-static", "0.5.10696" do |s|
+          s.platform = "x86_64-linux"
+        end
+      end
+
+      gemfile <<~G
+        source "#{file_uri_for(gem_repo4)}"
+
+        gem "nokogiri"
+        gem "sorbet-static"
+      G
+
+      lockfile <<~L
+        GEM
+          remote: #{file_uri_for(gem_repo4)}/
+          specs:
+            nokogiri (1.14.0-arm-linux)
+            nokogiri (1.14.0-x86_64-linux)
+            sorbet-static (0.5.10696-x86_64-linux)
+
+        PLATFORMS
+          arm-linux
+          x86_64-linux
+
+        DEPENDENCIES
+          nokogiri
+          sorbet-static
+
+        BUNDLED WITH
+           #{Bundler::VERSION}
+      L
+
+      bundle "update"
+
+      expect(lockfile).to eq <<~L
+        GEM
+          remote: #{file_uri_for(gem_repo4)}/
+          specs:
+            nokogiri (1.14.0-x86_64-linux)
+            sorbet-static (0.5.10696-x86_64-linux)
+
+        PLATFORMS
+          x86_64-linux
+
+        DEPENDENCIES
+          nokogiri
+          sorbet-static
+
+        BUNDLED WITH
+           #{Bundler::VERSION}
+      L
+    end
+  end
+
   it "automatically fixes the lockfile without removing other variants if it's missing platform gems, but they are installed locally" do
     simulate_platform "x86_64-darwin-21" do
       build_repo4 do

bundler/spec/install/gems/resolving_spec.rb

@@ -416,7 +416,7 @@
           end
 
           nice_error = <<~E.strip
-            Could not find gem 'sorbet-static (= 0.5.10554)' with platforms 'arm64-darwin-21', 'aarch64-linux' in rubygems repository #{file_uri_for(gem_repo4)}/ or installed locally.
+            Could not find gems matching 'sorbet-static (= 0.5.10554)' valid for all resolution platforms (arm64-darwin-21, aarch64-linux) in rubygems repository #{file_uri_for(gem_repo4)}/ or installed locally.
 
             The source contains the following gems matching 'sorbet-static (= 0.5.10554)':
               * sorbet-static-0.5.10554-universal-darwin-21
@@ -425,6 +425,62 @@
         end
       end
 
+      context "when adding a new gem that does not resolve under all locked platforms" do
+        before do
+          simulate_platform "x86_64-linux" do
+            build_repo4 do
+              build_gem "nokogiri", "1.14.0" do |s|
+                s.platform = "x86_64-linux"
+              end
+              build_gem "nokogiri", "1.14.0" do |s|
+                s.platform = "arm-linux"
+              end
+
+              build_gem "sorbet-static", "0.5.10696" do |s|
+                s.platform = "x86_64-linux"
+              end
+            end
+
+            lockfile <<~L
+              GEM
+                remote: #{file_uri_for(gem_repo4)}/
+                specs:
+                  nokogiri (1.14.0-arm-linux)
+                  nokogiri (1.14.0-x86_64-linux)
+
+              PLATFORMS
+                arm-linux
+                x86_64-linux
+
+              DEPENDENCIES
+                nokogiri
+
+              BUNDLED WITH
+                 #{Bundler::VERSION}
+            L
+
+            gemfile <<~G
+              source "#{file_uri_for(gem_repo4)}"
+
+              gem "nokogiri"
+              gem "sorbet-static"
+            G
+
+            bundle "lock", :raise_on_error => false
+          end
+        end
+
+        it "raises a proper error" do
+          nice_error = <<~E.strip
+            Could not find gems matching 'sorbet-static' valid for all resolution platforms (arm-linux, x86_64-linux) in rubygems repository #{file_uri_for(gem_repo4)}/ or installed locally.
+
+            The source contains the following gems matching 'sorbet-static':
+              * sorbet-static-0.5.10696-x86_64-linux
+          E
+          expect(err).to end_with(nice_error)
+        end
+      end
+
       it "gives a meaningful error on ruby version mismatches between dependencies" do
         build_repo4 do
           build_gem "requires-old-ruby" do |s|