Merge pull request #9556 from Shopify/rwstauner/ruby-platform-lock

Fix two issues by including the ruby platform variants in the lock file

Author: hsbt

bundler/lib/bundler/cli/outdated.rb

@@ -77,7 +77,7 @@ def run
         gemfile_specs + dependency_specs
       end
 
-      specs.sort_by(&:name).uniq(&:name).each do |current_spec|
+      specs_for_outdated_check(specs).each do |current_spec|
         next unless gems.empty? || gems.include?(current_spec.name)
 
         active_spec = retrieve_active_spec(definition, current_spec)
@@ -152,6 +152,12 @@ def nothing_outdated_message
       end
     end
 
+    def specs_for_outdated_check(specs)
+      specs.group_by(&:name).values.filter_map do |matching_specs|
+        MatchPlatform.select_best_platform_match(matching_specs, Bundler.local_platform).first || matching_specs.first
+      end.sort_by(&:name)
+    end
+
     def retrieve_active_spec(definition, current_spec)
       active_spec = definition.resolve.find_by_name_and_platform(current_spec.name, current_spec.platform)
       return unless active_spec

bundler/lib/bundler/lazy_specification.rb

@@ -9,7 +9,8 @@ class LazySpecification
     include ForcePlatform
 
     attr_reader :name, :version, :platform, :materialization
-    attr_accessor :source, :remote, :force_ruby_platform, :dependencies, :required_ruby_version, :required_rubygems_version, :overrides
+    attr_accessor :source, :remote, :force_ruby_platform, :dependencies, :required_ruby_version, :required_rubygems_version
+    attr_accessor :overrides, :locked_platforms
 
     #
     # For backwards compatibility with existing lockfiles, if the most specific
@@ -49,6 +50,7 @@ def initialize(name, version, platform, source = nil, **materialization_options)
       @force_ruby_platform = default_force_ruby_platform
       @most_specific_locked_platform = nil
       @materialization = nil
+      @locked_platforms = nil
     end
 
     def missing?
@@ -145,7 +147,7 @@ def materialize_for_installation
         # Exact spec is incompatible; in frozen mode, try to find a compatible platform variant
         # In non-frozen mode, return nil to trigger re-resolution and lockfile update
         if Bundler.frozen_bundle?
-          materialize([name, version]) {|specs| resolve_best_platform(specs) }
+          materialize([name, version]) {|specs| resolve_best_platform(specs, locked_platforms_only: true) }
         end
       else
         materialize([name, version]) {|specs| resolve_best_platform(specs) }
@@ -186,12 +188,12 @@ def use_exact_resolved_specifications?
     # Try platforms in order of preference until finding a compatible spec.
     # Used for legacy lockfiles and as a fallback when the exact locked spec
     # is incompatible. Falls back to frozen bundle behavior if none match.
-    def resolve_best_platform(specs)
-      find_compatible_platform_spec(specs) || frozen_bundle_fallback(specs)
+    def resolve_best_platform(specs, locked_platforms_only: false)
+      find_compatible_platform_spec(specs, locked_platforms_only: locked_platforms_only) || frozen_bundle_fallback(specs)
     end
 
-    def find_compatible_platform_spec(specs)
-      candidate_platforms.each do |plat|
+    def find_compatible_platform_spec(specs, locked_platforms_only: false)
+      candidate_platforms(locked_platforms_only: locked_platforms_only).each do |plat|
         candidates = MatchPlatform.select_best_platform_match(specs, plat)
         spec = choose_compatible(candidates, fallback_to_non_installable: false)
         return spec if spec
@@ -201,9 +203,12 @@ def find_compatible_platform_spec(specs)
 
     # Platforms to try in order of preference. Ruby platform is last since it
     # requires compilation, but works when precompiled gems are incompatible.
-    def candidate_platforms
+    def candidate_platforms(locked_platforms_only: false)
       target = source.is_a?(Source::Path) ? platform : Bundler.local_platform
-      [target, platform, Gem::Platform::RUBY].uniq
+      platforms = [target, platform, Gem::Platform::RUBY].uniq
+      return platforms unless locked_platforms_only && locked_platforms
+
+      platforms & locked_platforms
     end
 
     # In frozen mode, accept any candidate. Will error at install time.

bundler/lib/bundler/materialization.rb

@@ -12,6 +12,7 @@ def initialize(dep, platform, candidates:)
       @dep = dep
       @platform = platform
       @candidates = candidates
+      set_locked_platforms
     end
 
     def complete?
@@ -55,5 +56,14 @@ def incomplete_specs
     private
 
     attr_reader :dep, :platform
+
+    def set_locked_platforms
+      return unless @candidates
+
+      platforms = @candidates.map(&:platform)
+      @candidates.each do |candidate|
+        candidate.locked_platforms = platforms if candidate.respond_to?(:locked_platforms=)
+      end
+    end
   end
 end

bundler/lib/bundler/resolver.rb

@@ -305,11 +305,15 @@ def all_versions_for(package)
           next groups if package.force_ruby_platform?
         end
 
-        platform_group = Resolver::SpecGroup.new(platform_specs.flatten.uniq)
+        platform_specs = platform_specs.flatten.uniq
+        platform_group = Resolver::SpecGroup.new((platform_specs + ruby_specs).uniq)
         next groups if platform_group == ruby_group
 
         groups << Resolver::Candidate.new(version, group: platform_group, priority: 1)
 
+        platform_only_group = Resolver::SpecGroup.new(platform_specs)
+        groups << Resolver::Candidate.new(version, group: platform_only_group, priority: 0) unless platform_only_group == platform_group
+
         groups
       end
     end

spec/commands/lock_spec.rb

@@ -1083,8 +1083,10 @@
     simulate_platform("x86-mingw32") { bundle :lock }
 
     checksums = checksums_section_when_enabled do |c|
+      c.checksum gem_repo4, "ffi", "1.9.14"
       c.checksum gem_repo4, "ffi", "1.9.14", "x86-mingw32"
       c.checksum gem_repo4, "gssapi", "1.2.0"
+      c.checksum gem_repo4, "mixlib-shellout", "2.2.6"
       c.checksum gem_repo4, "mixlib-shellout", "2.2.6", "universal-mingw32"
       c.checksum gem_repo4, "win32-process", "0.8.3"
     end
@@ -1093,9 +1095,11 @@
       GEM
         remote: https://gem.repo4/
         specs:
+          ffi (1.9.14)
           ffi (1.9.14-x86-mingw32)
           gssapi (1.2.0)
             ffi (>= 1.0.1)
+          mixlib-shellout (2.2.6)
           mixlib-shellout (2.2.6-universal-mingw32)
             win32-process (~> 0.8.2)
           win32-process (0.8.3)

spec/install/gemfile/platform_spec.rb

@@ -208,6 +208,7 @@
         c.checksum gem_repo4, "empyrean", "0.1.0"
         c.checksum gem_repo4, "ffi", "1.9.23", "java"
         c.checksum gem_repo4, "method_source", "0.9.0"
+        c.checksum gem_repo4, "pry", "0.11.3"
         c.checksum gem_repo4, "pry", "0.11.3", "java"
         c.checksum gem_repo4, "spoon", "0.0.6"
       end
@@ -220,6 +221,9 @@
             empyrean (0.1.0)
             ffi (1.9.23-java)
             method_source (0.9.0)
+            pry (0.11.3)
+              coderay (~> 1.1.0)
+              method_source (~> 0.9.0)
             pry (0.11.3-java)
               coderay (~> 1.1.0)
               method_source (~> 0.9.0)

spec/install/gemfile/specific_platform_spec.rb

@@ -261,6 +261,99 @@
         expect(the_bundle).not_to include_gem("nokogiri 1.18.10 x86_64-linux")
       end
     end
+
+    it "adds and installs the ruby variant when only an incompatible platform-specific variant was locked" do
+      build_repo4 do
+        build_gem "nokogiri", "1.18.10"
+        build_gem "nokogiri", "1.18.10" do |s|
+          s.platform = "x86_64-linux"
+          s.required_ruby_version = "< #{Gem.ruby_version}"
+        end
+      end
+
+      gemfile <<~G
+        source "https://gem.repo4"
+
+        gem "nokogiri"
+      G
+
+      lockfile <<-L
+        GEM
+          remote: https://gem.repo4/
+          specs:
+            nokogiri (1.18.10-x86_64-linux)
+
+        PLATFORMS
+          x86_64-linux
+
+        DEPENDENCIES
+          nokogiri
+
+        BUNDLED WITH
+          #{Bundler::VERSION}
+      L
+
+      simulate_platform "x86_64-linux" do
+        bundle "install --verbose"
+        expect(out).to include("Installing nokogiri 1.18.10")
+        expect(the_bundle).to include_gem("nokogiri 1.18.10")
+      end
+
+      expect(lockfile).to eq <<~L
+        GEM
+          remote: https://gem.repo4/
+          specs:
+            nokogiri (1.18.10)
+            nokogiri (1.18.10-x86_64-linux)
+
+        PLATFORMS
+          x86_64-linux
+
+        DEPENDENCIES
+          nokogiri
+
+        BUNDLED WITH
+          #{Bundler::VERSION}
+      L
+    end
+
+    it "fails at install time when only an incompatible platform-specific variant is locked" do
+      build_repo4 do
+        build_gem "nokogiri", "1.18.10"
+        build_gem "nokogiri", "1.18.10" do |s|
+          s.platform = "x86_64-linux"
+          s.required_ruby_version = "< #{Gem.ruby_version}"
+        end
+      end
+
+      gemfile <<~G
+        source "https://gem.repo4"
+
+        gem "nokogiri"
+      G
+
+      lockfile <<-L
+        GEM
+          remote: https://gem.repo4/
+          specs:
+            nokogiri (1.18.10-x86_64-linux)
+
+        PLATFORMS
+          x86_64-linux
+
+        DEPENDENCIES
+          nokogiri
+
+        BUNDLED WITH
+          #{Bundler::VERSION}
+      L
+
+      simulate_platform "x86_64-linux" do
+        bundle "install --verbose", env: { "BUNDLE_FROZEN" => "true" }, raise_on_error: false
+        expect(exitstatus).not_to eq(0)
+        expect(err).to include("nokogiri-1.18.10-x86_64-linux requires ruby version < #{Gem.ruby_version}")
+      end
+    end
   end
 
   it "doesn't discard previously installed platform specific gem and fall back to ruby on subsequent bundles" do
@@ -766,10 +859,13 @@
       bundle "update --conservative nokogiri"
     end
 
+    checksums.checksum gem_repo4, "nokogiri", "1.13.0"
+
     expect(lockfile).to eq <<~L
       GEM
         remote: https://gem.repo4/
         specs:
+          nokogiri (1.13.0)
           nokogiri (1.13.0-x86_64-darwin)
           sorbet-static (0.5.10601-x86_64-darwin)
 
@@ -785,6 +881,61 @@
     L
   end
 
+  it "locks ruby fallback variant dependencies without adding the ruby platform" do
+    build_repo4 do
+      build_gem "native_tool", "1.0" do |s|
+        s.add_dependency "rake"
+      end
+
+      build_gem "native_tool", "1.0" do |s|
+        s.platform = "x86_64-linux"
+      end
+
+      build_gem "rake"
+
+      build_gem "sorbet-static", "0.5.10601" do |s|
+        s.platform = "x86_64-linux"
+      end
+    end
+
+    simulate_platform "x86_64-linux" do
+      install_gemfile <<~G
+        source "https://gem.repo4"
+
+        gem "native_tool"
+        gem "sorbet-static"
+      G
+    end
+
+    checksums = checksums_section_when_enabled do |c|
+      c.checksum gem_repo4, "native_tool", "1.0"
+      c.checksum gem_repo4, "native_tool", "1.0", "x86_64-linux"
+      c.checksum gem_repo4, "rake", "1.0"
+      c.checksum gem_repo4, "sorbet-static", "0.5.10601", "x86_64-linux"
+    end
+
+    expect(lockfile).to eq <<~L
+      GEM
+        remote: https://gem.repo4/
+        specs:
+          native_tool (1.0)
+            rake
+          native_tool (1.0-x86_64-linux)
+          rake (1.0)
+          sorbet-static (0.5.10601-x86_64-linux)
+
+      PLATFORMS
+        x86_64-linux
+
+      DEPENDENCIES
+        native_tool
+        sorbet-static
+      #{checksums}
+      BUNDLED WITH
+        #{Bundler::VERSION}
+    L
+  end
+
   it "automatically fixes the lockfile if only ruby platform is locked and some gem has no ruby variant available" do
     build_repo4 do
       build_gem("sorbet-static-and-runtime", "0.5.10160") do |s|

spec/lock/lockfile_spec.rb

@@ -1324,13 +1324,15 @@
       G
 
       checksums = checksums_section_when_enabled do |c|
+        c.checksum gem_repo2, "platform_specific", "1.0"
         c.checksum gem_repo2, "platform_specific", "1.0", "universal-java-16"
       end
 
       expect(lockfile).to eq <<~G
         GEM
           remote: https://gem.repo2/
           specs:
+            platform_specific (1.0)
             platform_specific (1.0-universal-java-16)
 
         PLATFORMS