Fix installing gems with native extensions + transitive dependencies

I am seeing the following error during bundle install:

```
Gem::MissingSpecError: Could not find 'ffi' (>= 1.15.5) among 48 total gem(s) (Gem::MissingSpecError)
```

This is reproducible with:

```ruby
source 'https://rubygems.org'

gem 'llhttp-ffi'
```

A binary search of this repo indicates the issue may have been
introduced in #9381.

It seems only direct dependencies are checked when determining whether
a Gem with native extensions is ready to install. I believe this can
lead to a failure if a transitive dependency is not yet installed.

In the example above, llhttp-ffi depends on ffi-compiler, which depends
on ffi.  Since ffi-compiler has no extensions, it is installed
immediately without waiting for ffi. When llhttp-ffi then checks its
direct dependencies, ffi-compiler is already installed, so llhttp-ffi
starts building its native extension. The build requires ffi, which may
not have been installed yet.

Author: nicholasdower

bundler/lib/bundler/installer/parallel_installer.rb

@@ -6,7 +6,7 @@
 module Bundler
   class ParallelInstaller
     class SpecInstallation
-      attr_accessor :spec, :name, :full_name, :post_install_message, :state, :error
+      attr_accessor :spec, :name, :full_name, :post_install_message, :state, :error, :dependencies
       def initialize(spec)
         @spec = spec
         @name = spec.name
@@ -46,25 +46,11 @@ def has_post_install_message?
         !post_install_message.empty?
       end
 
-      def ignorable_dependency?(dep)
-        dep.type == :development || dep.name == @name
-      end
-
-      # Checks installed dependencies against spec's dependencies to make
-      # sure needed dependencies have been installed.
+      # Recursively checks that all dependencies (direct and transitive) have been installed.
       def dependencies_installed?(installed_specs)
-        dependencies.all? {|d| installed_specs.include? d.name }
-      end
-
-      # Represents only the non-development dependencies, the ones that are
-      # itself and are in the total list.
-      def dependencies
-        @dependencies ||= all_dependencies.reject {|dep| ignorable_dependency? dep }
-      end
-
-      # Represents all dependencies
-      def all_dependencies
-        @spec.dependencies
+        dependencies.all? do |dep|
+          installed_specs.include?(dep.name) && dep.dependencies_installed?(installed_specs)
+        end
       end
 
       def to_s
@@ -85,6 +71,12 @@ def initialize(installer, all_specs, size, standalone, force, local: false, skip
       @force = force
       @local = local
       @specs = all_specs.map {|s| SpecInstallation.new(s) }
+      specs_by_name = @specs.to_h {|s| [s.name, s] }
+      @specs.each do |spec_install|
+        spec_install.dependencies = spec_install.spec.dependencies.filter_map do |dep|
+          specs_by_name[dep.name] unless dep.type == :development || dep.name == spec_install.name
+        end
+      end
       @specs.each do |spec_install|
         spec_install.state = :installed if skip.include?(spec_install.name)
       end if skip

spec/bundler/installer/parallel_installer_spec.rb

@@ -6,6 +6,49 @@
 require "bundler"
 
 RSpec.describe Bundler::ParallelInstaller do
+  def build_spec(name, extensions: [], dependencies: [])
+    spec = Object.new
+    spec.define_singleton_method(:name) { name }
+    spec.define_singleton_method(:full_name) { "#{name}-1.0" }
+    spec.define_singleton_method(:extensions) { extensions }
+    spec.define_singleton_method(:dependencies) { dependencies }
+    spec
+  end
+
+  def build_dep(name, type: :runtime)
+    dep = Object.new
+    dep.define_singleton_method(:name) { name }
+    dep.define_singleton_method(:type) { type }
+    dep
+  end
+
+  describe "install ordering" do
+    it "blocks gems with extensions until all dependencies are installed" do
+      alpha = build_spec("alpha")
+      beta = build_spec("beta", dependencies: [build_dep("alpha")])
+      gamma = build_spec("gamma", extensions: ["ext/Rakefile"], dependencies: [build_dep("beta")])
+
+      parallel_installer = described_class.new(double("installer"), [alpha, beta, gamma], 1, false, false)
+      gamma_install = parallel_installer.instance_variable_get(:@specs).find {|s| s.name == "gamma" }
+      gamma_install.state = :downloaded
+
+      expect(gamma_install.ready_to_install?({})).to be_falsey
+      expect(gamma_install.ready_to_install?({ "beta" => true })).to be_falsey
+      expect(gamma_install.ready_to_install?({ "alpha" => true, "beta" => true })).to be_truthy
+    end
+
+    it "does not block gems without extensions regardless of dependencies" do
+      alpha = build_spec("alpha")
+      beta = build_spec("beta", dependencies: [build_dep("alpha")])
+
+      parallel_installer = described_class.new(double("installer"), [alpha, beta], 1, false, false)
+      beta_install = parallel_installer.instance_variable_get(:@specs).find {|s| s.name == "beta" }
+      beta_install.state = :downloaded
+
+      expect(beta_install.ready_to_install?({})).to be_truthy
+    end
+  end
+
   describe "priority queue" do
     before do
       require "support/artifice/compact_index"

spec/bundler/installer/spec_installation_spec.rb

@@ -3,18 +3,17 @@
 require "bundler/installer/parallel_installer"
 
 RSpec.describe Bundler::ParallelInstaller::SpecInstallation do
-  let!(:dep) do
-    a_spec = Object.new
-    def a_spec.name
-      "I like tests"
-    end
-
-    def a_spec.full_name
-      "I really like tests"
-    end
-    a_spec
+  def build_spec(name, extensions: [])
+    spec = Object.new
+    spec.define_singleton_method(:name) { name }
+    spec.define_singleton_method(:full_name) { "#{name}-1.0" }
+    spec.define_singleton_method(:extensions) { extensions }
+    spec.define_singleton_method(:dependencies) { [] }
+    spec
   end
 
+  let!(:dep) { build_spec("I like tests") }
+
   describe "#ready_to_enqueue?" do
     context "when in enqueued state" do
       it "is falsey" do
@@ -39,29 +38,51 @@ def a_spec.full_name
   end
 
   describe "#dependencies_installed?" do
-    context "when all dependencies are installed" do
-      it "returns true" do
-        dependencies = []
-        dependencies << instance_double("SpecInstallation", spec: "alpha", name: "alpha", installed?: true, all_dependencies: [], type: :production)
-        dependencies << instance_double("SpecInstallation", spec: "beta", name: "beta", installed?: true, all_dependencies: [], type: :production)
-        all_specs = dependencies + [instance_double("SpecInstallation", spec: "gamma", name: "gamma", installed?: false, all_dependencies: [], type: :production)]
+    it "returns true when all dependencies are installed" do
+      alpha = described_class.new(build_spec("alpha"))
+      alpha.dependencies = []
+
+      beta = described_class.new(build_spec("beta"))
+      beta.dependencies = [alpha]
+
+      gamma = described_class.new(build_spec("gamma"))
+      gamma.dependencies = [beta]
+
+      expect(gamma.dependencies_installed?({})).to be_falsey
+      expect(gamma.dependencies_installed?({ "beta" => true })).to be_falsey
+      expect(gamma.dependencies_installed?({ "alpha" => true, "beta" => true })).to be_truthy
+    end
+  end
+
+  describe "#ready_to_install?" do
+    context "when spec has no extensions" do
+      it "returns true regardless of dependencies" do
+        beta = described_class.new(build_spec("beta"))
+        beta.dependencies = []
+
         spec = described_class.new(dep)
-        allow(spec).to receive(:all_dependencies).and_return(dependencies)
-        installed_specs = all_specs.select(&:installed?).map {|s| [s.name, true] }.to_h
-        expect(spec.dependencies_installed?(installed_specs)).to be_truthy
+        spec.state = :downloaded
+        spec.dependencies = [beta]
+
+        expect(spec.ready_to_install?({})).to be_truthy
       end
     end
 
-    context "when all dependencies are not installed" do
-      it "returns false" do
-        dependencies = []
-        dependencies << instance_double("SpecInstallation", spec: "alpha", name: "alpha", installed?: false, all_dependencies: [], type: :production)
-        dependencies << instance_double("SpecInstallation", spec: "beta", name: "beta", installed?: true, all_dependencies: [], type: :production)
-        all_specs = dependencies + [instance_double("SpecInstallation", spec: "gamma", name: "gamma", installed?: false, all_dependencies: [], type: :production)]
-        spec = described_class.new(dep)
-        allow(spec).to receive(:all_dependencies).and_return(dependencies)
-        installed_specs = all_specs.select(&:installed?).map {|s| [s.name, true] }.to_h
-        expect(spec.dependencies_installed?(installed_specs)).to be_falsey
+    context "when spec has extensions" do
+      it "returns true when all dependencies are installed" do
+        alpha = described_class.new(build_spec("alpha"))
+        alpha.dependencies = []
+
+        beta = described_class.new(build_spec("beta"))
+        beta.dependencies = [alpha]
+
+        gamma = described_class.new(build_spec("gamma", extensions: ["ext/Rakefile"]))
+        gamma.state = :downloaded
+        gamma.dependencies = [beta]
+
+        expect(gamma.ready_to_install?({})).to be_falsey
+        expect(gamma.ready_to_install?({ "beta" => true })).to be_falsey
+        expect(gamma.ready_to_install?({ "alpha" => true, "beta" => true })).to be_truthy
       end
     end
   end