Auto merge of #5266 - bundler:seg-install-frozen-gemspec-deps-changed, r=indirect

[Definition] Fail when installing in frozen mode & gemspec deps have changed

Closes #5264

- [x] Specs

8 files changed, 81 insertions, 20 deletions

diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb
index 5a90b84125..bf3c34d9ff 100644
--- a/lib/bundler/definition.rb
+++ b/lib/bundler/definition.rb
@@ -104,7 +104,7 @@ module Bundler
 
       add_current_platform unless Bundler.settings[:frozen]
 
-      converge_gemspec_sources
+      converge_path_sources_to_gemspec_sources
       @path_changes = converge_paths
       @source_changes = converge_sources
 
@@ -462,12 +462,13 @@ module Bundler
         changed << "* #{name} from `#{gemfile_source_name}` to `#{lockfile_source_name}`"
       end
 
+      msg << "\n\n#{change_reason.split(", ").join("\n")}\n"
       msg << "\n\nYou have added to the Gemfile:\n" << added.join("\n") if added.any?
       msg << "\n\nYou have deleted from the Gemfile:\n" << deleted.join("\n") if deleted.any?
       msg << "\n\nYou have changed in the Gemfile:\n" << changed.join("\n") if changed.any?
       msg << "\n"
 
-      raise ProductionError, msg if added.any? || deleted.any? || changed.any?
+      raise ProductionError, msg if added.any? || deleted.any? || changed.any? || !nothing_changed?
     end
 
     def validate_runtime!
@@ -592,7 +593,8 @@ module Bundler
       locked_index = Index.new
       locked_index.use(@locked_specs.select {|s| source.can_lock?(s) })
 
-      source.specs != locked_index
+      # order here matters, since Index#== is checking source.specs.include?(locked_index)
+      locked_index != source.specs
     end
 
     # Get all locals and override their matching sources.
@@ -628,7 +630,7 @@ module Bundler
       gemspec_source || source
     end
 
-    def converge_gemspec_sources
+    def converge_path_sources_to_gemspec_sources
       @locked_sources.map! do |source|
         converge_path_source_to_gemspec_source(source)
       end
@@ -750,8 +752,9 @@ module Bundler
           next unless other
 
           deps2 = other.dependencies.select {|d| d.type != :development }
+          runtime_dependencies = s.dependencies.select {|d| d.type != :development }
           # If the dependencies of the path source have changed, unlock it
-          next unless s.dependencies.sort == deps2.sort
+          next unless runtime_dependencies.sort == deps2.sort
         end
 
         converged << s
diff --git a/lib/bundler/index.rb b/lib/bundler/index.rb
index 4fe1418773..373f6132af 100644
--- a/lib/bundler/index.rb
+++ b/lib/bundler/index.rb
@@ -144,6 +144,8 @@ module Bundler
       end
     end
 
+    # Whether all the specs in self are in other
+    # TODO: rename to #include?
     def ==(other)
       all? do |spec|
         other_spec = other[spec].first
diff --git a/lib/bundler/source/path.rb b/lib/bundler/source/path.rb
index 4661a6d068..2900b3f939 100644
--- a/lib/bundler/source/path.rb
+++ b/lib/bundler/source/path.rb
@@ -4,10 +4,12 @@ module Bundler
     class Path < Source
       autoload :Installer, "bundler/source/path/installer"
 
-      attr_reader :path, :options, :root_path
+      attr_reader :path, :options, :root_path, :original_path
       attr_writer :name
       attr_accessor :version
 
+      protected :original_path
+
       DEFAULT_GLOB = "{,*,*/*}.gemspec".freeze
 
       def initialize(options)
@@ -61,7 +63,7 @@ module Bundler
 
       def eql?(other)
         return unless other.class == self.class
-        expanded_path == expand(other.path) &&
+        expand(@original_path) == expand(other.original_path) &&
           version == other.version
       end
 
@@ -130,8 +132,8 @@ module Bundler
       end
 
       def lockfile_path
-        return relative_path if path.absolute?
-        expand(path).relative_path_from(Bundler.root)
+        return relative_path(original_path) if original_path.absolute?
+        expand(original_path).relative_path_from(Bundler.root)
       end
 
       def app_cache_path(custom_path = nil)
@@ -186,7 +188,7 @@ module Bundler
         index
       end
 
-      def relative_path
+      def relative_path(path = self.path)
         if path.to_s.start_with?(root_path.to_s)
           return path.relative_path_from(root_path)
         end
diff --git a/lib/bundler/spec_set.rb b/lib/bundler/spec_set.rb
index d3ffa0d5ca..19b9cddc3e 100644
--- a/lib/bundler/spec_set.rb
+++ b/lib/bundler/spec_set.rb
@@ -20,8 +20,8 @@ module Bundler
       specs = []
       skip += ["bundler"]
 
-      until deps.empty?
-        dep = deps.shift
+      loop do
+        break unless dep = deps.shift
         next if handled[dep] || skip.include?(dep.name)
 
         handled[dep] = true
@@ -155,14 +155,15 @@ module Bundler
     end
 
     def spec_for_dependency(dep, match_current_platform)
+      specs_for_platforms = lookup[dep.name]
       if match_current_platform
         Bundler.rubygems.platforms.reverse_each do |pl|
-          match = GemHelpers.select_best_platform_match(lookup[dep.name], pl)
+          match = GemHelpers.select_best_platform_match(specs_for_platforms, pl)
           return match if match
         end
         nil
       else
-        GemHelpers.select_best_platform_match(lookup[dep.name], dep.__platform)
+        GemHelpers.select_best_platform_match(specs_for_platforms, dep.__platform)
       end
     end
 
diff --git a/spec/install/deploy_spec.rb b/spec/install/deploy_spec.rb
index 1e54204e70..4a4f6e912f 100644
--- a/spec/install/deploy_spec.rb
+++ b/spec/install/deploy_spec.rb
@@ -77,10 +77,8 @@ RSpec.describe "install with --deployment or --frozen" do
       gem "bar", :path => "#{lib_path("nested")}"
     G
 
-    bundle :install
-    bundle "install --deployment"
-
-    expect(exitstatus).to eq(0) if exitstatus
+    bundle! :install
+    bundle! "install --deployment"
   end
 
   it "works when there are credentials in the source URL" do
diff --git a/spec/install/gemfile/gemspec_spec.rb b/spec/install/gemfile/gemspec_spec.rb
index 2c3e8f265c..541a7e4008 100644
--- a/spec/install/gemfile/gemspec_spec.rb
+++ b/spec/install/gemfile/gemspec_spec.rb
@@ -211,6 +211,31 @@ RSpec.describe "bundle install from an existing gemspec" do
     expect(the_bundle).to include_gems "foo 1.0.0"
   end
 
+  context "in deployment mode" do
+    context "when the lockfile was not updated after a change to the gemspec's dependencies" do
+      it "reports that installation failed" do
+        build_lib "cocoapods", :path => bundled_app do |s|
+          s.add_dependency "activesupport", ">= 1"
+        end
+
+        install_gemfile! <<-G
+          source "file://#{gem_repo1}"
+          gemspec
+        G
+
+        expect(the_bundle).to include_gems("cocoapods 1.0", "activesupport 2.3.5")
+
+        build_lib "cocoapods", :path => bundled_app do |s|
+          s.add_dependency "activesupport", ">= 1.0.1"
+        end
+
+        bundle "install --deployment"
+
+        expect(out).to include("changed")
+      end
+    end
+  end
+
   context "when child gemspecs conflict with a released gemspec" do
     before do
       # build the "parent" gem that depends on another gem in the same repo
diff --git a/spec/lock/lockfile_spec.rb b/spec/lock/lockfile_spec.rb
index 8ff6b1a76d..7d768e337c 100644
--- a/spec/lock/lockfile_spec.rb
+++ b/spec/lock/lockfile_spec.rb
@@ -600,6 +600,36 @@ RSpec.describe "the lockfile format" do
     G
   end
 
+  it "serializes pinned path sources to the lockfile even when packaging" do
+    build_lib "foo"
+
+    install_gemfile! <<-G
+      gem "foo", :path => "#{lib_path("foo-1.0")}"
+    G
+
+    bundle! "package --all"
+    bundle! "install --local"
+
+    lockfile_should_be <<-G
+      PATH
+        remote: #{lib_path("foo-1.0")}
+        specs:
+          foo (1.0)
+
+      GEM
+        specs:
+
+      PLATFORMS
+        #{generic_local_platform}
+
+      DEPENDENCIES
+        foo!
+
+      BUNDLED WITH
+         #{Bundler::VERSION}
+    G
+  end
+
   it "sorts serialized sources by type" do
     build_lib "foo"
     bar = build_git "bar"
diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb
index 738b1f7b3b..815671bba5 100644
--- a/spec/support/helpers.rb
+++ b/spec/support/helpers.rb
@@ -3,9 +3,9 @@ module Spec
   module Helpers
     def reset!
       Dir["#{tmp}/{gems/*,*}"].each do |dir|
-        next if %(base remote1 gems rubygems).include?(File.basename(dir))
+        next if %w(base remote1 gems rubygems).include?(File.basename(dir))
         if ENV["BUNDLER_SUDO_TESTS"]
-          `sudo rm -rf #{dir}`
+          `sudo rm -rf "#{dir}"`
         else
           FileUtils.rm_rf(dir)
         end