diff options
author | Samuel Giddins <segiddins@segiddins.me> | 2016-06-19 01:15:51 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-06-19 01:15:51 -0400 |
commit | 6922e787a05f1a185763fa973d1a208040326224 (patch) | |
tree | c08fff9a7552d789ac8ae368484e55797d4a124f | |
parent | e45c11f34b350ac74d661b03927e8345c2f7da4c (diff) | |
parent | 1b66b1d4dbe779b658c67e1fd7094808aa6f34ee (diff) | |
download | bundler-6922e787a05f1a185763fa973d1a208040326224.tar.gz |
Merge pull request #4618 from jrafanie/use_set_for_orderless_array_comparison
[WIP] Properly detect path spec changes to avoid needless re-resolve
-rw-r--r-- | lib/bundler/definition.rb | 13 | ||||
-rw-r--r-- | lib/bundler/index.rb | 8 | ||||
-rw-r--r-- | spec/bundler/definition_spec.rb | 114 | ||||
-rw-r--r-- | spec/install/gemfile/path_spec.rb | 34 |
4 files changed, 160 insertions, 9 deletions
diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb index d4af6b70..efc249be 100644 --- a/lib/bundler/definition.rb +++ b/lib/bundler/definition.rb @@ -216,9 +216,11 @@ module Bundler @resolve ||= begin last_resolve = converge_locked_specs if Bundler.settings[:frozen] || (!@unlocking && nothing_changed?) + Bundler.ui.debug("Found no changes, using resolution from the lockfile") last_resolve else # Run a resolve against the locally available gems + Bundler.ui.debug("Found changes from the lockfile, re-resolving dependencies") last_resolve.merge Resolver.resolve(expanded_dependencies, index, source_requirements, last_resolve, ruby_version) end end @@ -480,14 +482,21 @@ module Bundler end end - !locked || unlocking || dependencies_for_source_changed?(locked) || source.specs != locked.specs + !locked || unlocking || dependencies_for_source_changed?(source) || specs_for_source_changed?(source) end def dependencies_for_source_changed?(source) deps_for_source = @dependencies.select {|s| s.source == source } locked_deps_for_source = @locked_deps.select {|s| s.source == source } - deps_for_source != locked_deps_for_source + Set.new(deps_for_source) != Set.new(locked_deps_for_source) + end + + def specs_for_source_changed?(source) + locked_index = Index.new + locked_index.use(@locked_specs.select {|s| source.can_lock?(s) }) + + source.specs != locked_index end # Get all locals and override their matching sources. diff --git a/lib/bundler/index.rb b/lib/bundler/index.rb index c28a1209..f0ee411d 100644 --- a/lib/bundler/index.rb +++ b/lib/bundler/index.rb @@ -134,10 +134,16 @@ module Bundler def ==(other) all? do |spec| other_spec = other[spec].first - other_spec && (spec.dependencies & other_spec.dependencies).empty? && spec.source == other_spec.source + other_spec && dependencies_eql?(spec, other_spec) && spec.source == other_spec.source end end + def dependencies_eql?(spec, other_spec) + deps = spec.dependencies.select {|d| d.type != :development } + other_deps = other_spec.dependencies.select {|d| d.type != :development } + Set.new(deps) == Set.new(other_deps) + end + def add_source(index) raise ArgumentError, "Source must be an index, not #{index.class}" unless index.is_a?(Index) @sources << index diff --git a/spec/bundler/definition_spec.rb b/spec/bundler/definition_spec.rb index 443a2c97..a8ee3080 100644 --- a/spec/bundler/definition_spec.rb +++ b/spec/bundler/definition_spec.rb @@ -3,13 +3,12 @@ require "spec_helper" require "bundler/definition" describe Bundler::Definition do - before do - allow(Bundler).to receive(:settings) { Bundler::Settings.new(".") } - allow(Bundler).to receive(:default_gemfile) { Pathname.new("Gemfile") } - allow(Bundler).to receive(:ui) { double("UI", :info => "") } - end - describe "#lock" do + before do + allow(Bundler).to receive(:settings) { Bundler::Settings.new(".") } + allow(Bundler).to receive(:default_gemfile) { Pathname.new("Gemfile") } + allow(Bundler).to receive(:ui) { double("UI", :info => "", :debug => "") } + end context "when it's not possible to write to the file" do subject { Bundler::Definition.new(nil, [], Bundler::SourceList.new, []) } @@ -31,4 +30,107 @@ describe Bundler::Definition do end end end + + describe "detects changes" do + it "for a path gem with changes" do + build_lib "foo", "1.0", :path => lib_path("foo") + + install_gemfile <<-G + source "file://#{gem_repo1}" + gem "foo", :path => "#{lib_path("foo")}" + G + + build_lib "foo", "1.0", :path => lib_path("foo") do |s| + s.add_dependency "rack", "1.0" + end + + bundle :install, :env => { "DEBUG" => 1 } + + expect(out).to match(/re-resolving dependencies/) + lockfile_should_be <<-G + PATH + remote: #{lib_path("foo")} + specs: + foo (1.0) + rack (= 1.0) + + GEM + remote: file:#{gem_repo1}/ + specs: + rack (1.0.0) + + PLATFORMS + ruby + + DEPENDENCIES + foo! + + BUNDLED WITH + #{Bundler::VERSION} + G + end + + it "for a path gem with deps and no changes" do + build_lib "foo", "1.0", :path => lib_path("foo") do |s| + s.add_dependency "rack", "1.0" + s.add_development_dependency "net-ssh", "1.0" + end + + install_gemfile <<-G + source "file://#{gem_repo1}" + gem "foo", :path => "#{lib_path("foo")}" + G + + bundle :check, :env => { "DEBUG" => 1 } + + expect(out).to match(/using resolution from the lockfile/) + lockfile_should_be <<-G + PATH + remote: #{lib_path("foo")} + specs: + foo (1.0) + rack (= 1.0) + + GEM + remote: file:#{gem_repo1}/ + specs: + rack (1.0.0) + + PLATFORMS + ruby + + DEPENDENCIES + foo! + + BUNDLED WITH + #{Bundler::VERSION} + G + end + + it "for a rubygems gem" do + install_gemfile <<-G + source "file://#{gem_repo1}" + gem "foo" + G + + bundle :check, :env => { "DEBUG" => 1 } + + expect(out).to match(/using resolution from the lockfile/) + lockfile_should_be <<-G + GEM + remote: file:#{gem_repo1}/ + specs: + foo (1.0) + + PLATFORMS + ruby + + DEPENDENCIES + foo + + BUNDLED WITH + #{Bundler::VERSION} + G + end + end end diff --git a/spec/install/gemfile/path_spec.rb b/spec/install/gemfile/path_spec.rb index 64919370..39b0aed7 100644 --- a/spec/install/gemfile/path_spec.rb +++ b/spec/install/gemfile/path_spec.rb @@ -363,6 +363,40 @@ describe "bundle install with explicit source paths" do expect(exitstatus).to eq(0) if exitstatus end + context "existing lockfile" do + it "rubygems gems don't re-resolve without changes" do + install_gemfile <<-G + source "file://#{gem_repo1}" + gem 'rack-obama', '1.0' + gem 'net-ssh', '1.0' + G + + bundle :check, :env => { "DEBUG" => 1 } + expect(out).to match(/using resolution from the lockfile/) + should_be_installed "rack-obama 1.0", "net-ssh 1.0" + end + + it "source path gems w/deps don't re-resolve without changes" do + build_lib "rack-obama", "1.0", :path => lib_path("omg") do |s| + s.add_dependency "yard" + end + + build_lib "net-ssh", "1.0", :path => lib_path("omg") do |s| + s.add_dependency "yard" + end + + install_gemfile <<-G + source "file://#{gem_repo1}" + gem 'rack-obama', :path => "#{lib_path("omg")}" + gem 'net-ssh', :path => "#{lib_path("omg")}" + G + + bundle :check, :env => { "DEBUG" => 1 } + expect(out).to match(/using resolution from the lockfile/) + should_be_installed "rack-obama 1.0", "net-ssh 1.0" + end + end + it "installs executable stubs" do build_lib "foo" do |s| s.executables = ["foo"] |