diff --git a/bundler/lib/bundler/installer/parallel_installer.rb b/bundler/lib/bundler/installer/parallel_installer.rb index 020db30b8443..fef326ed0a9c 100644 --- a/bundler/lib/bundler/installer/parallel_installer.rb +++ b/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 diff --git a/spec/bundler/installer/spec_installation_spec.rb b/spec/bundler/installer/spec_installation_spec.rb index cbe2589b99b7..57868766d9b3 100644 --- a/spec/bundler/installer/spec_installation_spec.rb +++ b/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 diff --git a/spec/commands/install_spec.rb b/spec/commands/install_spec.rb index bb9b2b1f98a5..8dedeb7d0776 100644 --- a/spec/commands/install_spec.rb +++ b/spec/commands/install_spec.rb @@ -1379,6 +1379,48 @@ def run end end + describe "when a native extension requires a transitive dependency at build time" do + before do + build_repo4 do + build_gem "alpha", "1.0.0" do |s| + extension = "ext/alpha/extconf.rb" + s.extensions = extension + s.write(extension, <<~CODE) + require "mkmf" + sleep 1 + create_makefile("alpha") + CODE + s.write "lib/alpha.rb", "ALPHA = '1.0.0'" + end + + build_gem "beta", "1.0.0" do |s| + s.add_dependency "alpha" + s.write "lib/beta.rb", "require 'alpha'\nBETA = '1.0.0'" + end + + build_gem "gamma", "1.0.0" do |s| + s.add_dependency "beta" + extension = "ext/gamma/extconf.rb" + s.extensions = extension + s.write(extension, <<~EXTCONF) + require "beta" + require "mkmf" + create_makefile("gamma") + EXTCONF + end + end + end + + it "installs successfully" do + install_gemfile <<~G + source "https://gem.repo4" + gem "gamma" + G + + expect(the_bundle).to include_gems "alpha 1.0.0", "beta 1.0.0", "gamma 1.0.0" + end + end + describe "when configured path is UTF-8 and a file inside a gem package too" do let(:app_path) do path = tmp("♥")