Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Now won't wait for dependencies that will never install. Partial fix for #3692 #4012

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions lib/bundler/installer/parallel_installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,14 @@ def ignorable_dependency?(dep)
# sure needed dependencies have been installed.
def dependencies_installed?(all_specs)
installed_specs = all_specs.select(&:installed?).map(&:name)
dependencies.all? {|d| installed_specs.include? d.name }
dependencies(all_specs.map(&:name)).all? {|d| installed_specs.include? d.name }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty sure this change is breaking things?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@segiddins I don't see any failing tests for this line in Travis. Or am I missing something?

end

# Represents only the non-development dependencies and the ones that
# are itself.
def dependencies
@dependencies ||= all_dependencies.reject {|dep| ignorable_dependency? dep }
# Represents only the non-development dependencies, the ones that are
# itself and are in the total list.
def dependencies(all_spec_names)
@dependencies ||= all_dependencies.reject {|dep| ignorable_dependency? dep }.
select {|dep| all_spec_names.include? dep.name }
end

# Represents all dependencies
Expand Down
13 changes: 13 additions & 0 deletions spec/install/parallel/spec_installation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,18 @@ def a_spec.name
expect(spec.dependencies_installed?(all_specs)).to be_falsey
end
end

context "when dependencies that are not on the overall installation list are the only ones not installed" do
it "returns true" do
dependencies = []
dependencies << instance_double("SpecInstallation", :spec => "alpha", :name => "alpha", :installed? => true, :all_dependencies => [], :type => :production)
all_specs = dependencies + [instance_double("SpecInstallation", :spec => "gamma", :name => "gamma", :installed? => false, :all_dependencies => [], :type => :production)]
# Add dependency which is not in all_specs
dependencies << instance_double("SpecInstallation", :spec => "beta", :name => "beta", :installed? => false, :all_dependencies => [], :type => :production)
spec = ParallelInstaller::SpecInstallation.new(dep)
allow(spec).to receive(:all_dependencies).and_return(dependencies)
expect(spec.dependencies_installed?(all_specs)).to be_truthy
end
end
end
end