Merge pull request #6733 from rubygems/improve-error-message-in-froze…

…n-mode

Improve edge case error message

(cherry picked from commit c13621a)

bundler/lib/bundler/definition.rb

@@ -146,7 +146,7 @@ def initialize(lockfile, dependencies, sources, unlock, ruby_version = nil, opti
       @dependency_changes = converge_dependencies
       @local_changes = converge_locals
 
-      @incomplete_lockfile = check_missing_lockfile_specs
+      @missing_lockfile_dep = check_missing_lockfile_dep
     end
 
     def gem_version_promoter
@@ -234,6 +234,14 @@ def requested_dependencies
     end
 
     def current_dependencies
+      filter_relevant(dependencies)
+    end
+
+    def current_locked_dependencies
+      filter_relevant(locked_dependencies)
+    end
+
+    def filter_relevant(dependencies)
       dependencies.select do |d|
         d.should_include? && !d.gem_platforms([generic_local_platform]).empty?
       end
@@ -273,7 +281,7 @@ def resolve
       @resolve ||= if Bundler.frozen_bundle?
         Bundler.ui.debug "Frozen, using resolution from the lockfile"
         @locked_specs
-      elsif !unlocking? && nothing_changed?
+      elsif no_resolve_needed?
         if deleted_deps.any?
           Bundler.ui.debug "Some dependencies were deleted, using a subset of the resolution from the lockfile"
           SpecSet.new(filter_specs(@locked_specs, @dependencies - deleted_deps))
@@ -356,19 +364,6 @@ def to_lock
     end
 
     def ensure_equivalent_gemfile_and_lockfile(explicit_flag = false)
-      msg = String.new
-      msg << "You are trying to install in deployment mode after changing\n" \
-             "your Gemfile. Run `bundle install` elsewhere and add the\n" \
-             "updated #{Bundler.default_lockfile.relative_path_from(SharedHelpers.pwd)} to version control."
-
-      unless explicit_flag
-        suggested_command = unless Bundler.settings.locations("frozen").keys.include?(:env)
-          "bundle config set frozen false"
-        end
-        msg << "\n\nIf this is a development machine, remove the #{Bundler.default_gemfile} " \
-               "freeze \nby running `#{suggested_command}`." if suggested_command
-      end
-
       added =   []
       deleted = []
       changed = []
@@ -382,13 +377,8 @@ def ensure_equivalent_gemfile_and_lockfile(explicit_flag = false)
       deleted.concat deleted_deps.map {|d| "* #{pretty_dep(d)}" } if deleted_deps.any?
 
       both_sources = Hash.new {|h, k| h[k] = [] }
-      @dependencies.each {|d| both_sources[d.name][0] = d }
-
-      locked_dependencies.each do |d|
-        next if !Bundler.feature_flag.bundler_3_mode? && @locked_specs[d.name].empty?
-
-        both_sources[d.name][1] = d
-      end
+      current_dependencies.each {|d| both_sources[d.name][0] = d }
+      current_locked_dependencies.each {|d| both_sources[d.name][1] = d }
 
       both_sources.each do |name, (dep, lock_dep)|
         next if dep.nil? || lock_dep.nil?
@@ -403,11 +393,20 @@ def ensure_equivalent_gemfile_and_lockfile(explicit_flag = false)
       end
 
       reason = change_reason
-      msg << "\n\n#{reason.split(", ").map(&:capitalize).join("\n")}" unless reason.strip.empty?
+      msg = String.new
+      msg << "#{reason.capitalize.strip}, but the lockfile can't be updated because frozen mode is set"
       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"
+      msg << "\n\nRun `bundle install` elsewhere and add the updated #{Bundler.default_lockfile.relative_path_from(SharedHelpers.pwd)} to version control.\n"
+
+      unless explicit_flag
+        suggested_command = unless Bundler.settings.locations("frozen").keys.include?(:env)
+          "bundle config set frozen false"
+        end
+        msg << "If this is a development machine, remove the #{Bundler.default_gemfile.relative_path_from(SharedHelpers.pwd)} " \
+               "freeze by running `#{suggested_command}`." if suggested_command
+      end
 
       raise ProductionError, msg if added.any? || deleted.any? || changed.any? || !nothing_changed?
     end
@@ -472,7 +471,11 @@ def most_specific_locked_platform
     private :sources
 
     def nothing_changed?
-      !@source_changes && !@dependency_changes && !@new_platform && !@path_changes && !@local_changes && !@incomplete_lockfile
+      !@source_changes && !@dependency_changes && !@new_platform && !@path_changes && !@local_changes && !@missing_lockfile_dep
+    end
+
+    def no_resolve_needed?
+      !unlocking? && nothing_changed?
     end
 
     def unlocking?
@@ -609,7 +612,7 @@ def change_reason
         [@new_platform, "you added a new platform to your gemfile"],
         [@path_changes, "the gemspecs for path gems changed"],
         [@local_changes, "the gemspecs for git local gems changed"],
-        [@incomplete_lockfile, "your lock file is missing some gems"],
+        [@missing_lockfile_dep, "your lock file is missing \"#{@missing_lockfile_dep}\""],
       ].select(&:first).map(&:last).join(", ")
     end
 
@@ -664,7 +667,7 @@ def converge_locals
       !sources_with_changes.each {|source| @unlock[:sources] << source.name }.empty?
     end
 
-    def check_missing_lockfile_specs
+    def check_missing_lockfile_dep
       all_locked_specs = @locked_specs.map(&:name) << "bundler"
 
       missing = @locked_specs.select do |s|
@@ -674,10 +677,14 @@ def check_missing_lockfile_specs
       if missing.any?
         @locked_specs.delete(missing)
 
-        true
-      else
-        false
+        return missing.first.name
       end
+
+      return if @dependency_changes
+
+      current_dependencies.find do |d|
+        @locked_specs[d.name].empty?
+      end&.name
     end
 
     def converge_paths

bundler/lib/bundler/runtime.rb

@@ -94,7 +94,7 @@ def self.definition_method(meth)
     definition_method :requires
 
     def lock(opts = {})
-      return if @definition.nothing_changed? && !@definition.unlocking?
+      return if @definition.no_resolve_needed?
       @definition.lock(Bundler.default_lockfile, opts[:preserve_unknown_sections])
     end
 

bundler/spec/commands/cache_spec.rb

@@ -291,7 +291,7 @@
       G
       subject
       expect(exitstatus).to eq(16)
-      expect(err).to include("deployment mode")
+      expect(err).to include("frozen mode")
       expect(err).to include("You have added to the Gemfile")
       expect(err).to include("* rack-obama")
       bundle "env"

bundler/spec/commands/inject_spec.rb

@@ -109,7 +109,7 @@
         gem "rack-obama"
       G
       bundle "inject 'rack' '> 0'", :raise_on_error => false
-      expect(err).to match(/trying to install in deployment mode after changing/)
+      expect(err).to match(/the lockfile can't be updated because frozen mode is set/)
 
       expect(bundled_app_lock.read).not_to match(/rack-obama/)
     end

bundler/spec/commands/update_spec.rb

@@ -658,27 +658,27 @@
       bundle "update", :all => true, :raise_on_error => false
 
       expect(last_command).to be_failure
-      expect(err).to match(/You are trying to install in deployment mode after changing.your Gemfile/m)
-      expect(err).to match(/freeze \nby running `bundle config set frozen false`./m)
+      expect(err).to match(/Bundler is unlocking, but the lockfile can't be updated because frozen mode is set/)
+      expect(err).to match(/freeze by running `bundle config set frozen false`./)
     end
 
     it "should fail loudly when frozen is set globally" do
       bundle "config set --global frozen 1"
       bundle "update", :all => true, :raise_on_error => false
-      expect(err).to match(/You are trying to install in deployment mode after changing.your Gemfile/m).
-        and match(/freeze \nby running `bundle config set frozen false`./m)
+      expect(err).to match(/Bundler is unlocking, but the lockfile can't be updated because frozen mode is set/).
+        and match(/freeze by running `bundle config set frozen false`./)
     end
 
     it "should fail loudly when deployment is set globally" do
       bundle "config set --global deployment true"
       bundle "update", :all => true, :raise_on_error => false
-      expect(err).to match(/You are trying to install in deployment mode after changing.your Gemfile/m).
-        and match(/freeze \nby running `bundle config set frozen false`./m)
+      expect(err).to match(/Bundler is unlocking, but the lockfile can't be updated because frozen mode is set/).
+        and match(/freeze by running `bundle config set frozen false`./)
     end
 
     it "should not suggest any command to unfreeze bundler if frozen is set through ENV" do
       bundle "update", :all => true, :raise_on_error => false, :env => { "BUNDLE_FROZEN" => "true" }
-      expect(err).to match(/You are trying to install in deployment mode after changing.your Gemfile/m)
+      expect(err).to match(/Bundler is unlocking, but the lockfile can't be updated because frozen mode is set/)
       expect(err).not_to match(/by running/)
     end
   end

bundler/spec/install/deploy_spec.rb

@@ -66,7 +66,7 @@
       G
 
       bundle "install --deployment", :raise_on_error => false
-      expect(err).to include("deployment mode")
+      expect(err).to include("frozen mode")
       expect(err).to include("You have added to the Gemfile")
       expect(err).to include("* rack-obama")
       expect(err).not_to include("You have deleted from the Gemfile")
@@ -272,7 +272,7 @@
 
       bundle "config set --local deployment true"
       bundle :install, :raise_on_error => false
-      expect(err).to include("deployment mode")
+      expect(err).to include("frozen mode")
       expect(err).to include("You have added to the Gemfile")
       expect(err).to include("* rack-obama")
       expect(err).not_to include("You have deleted from the Gemfile")
@@ -297,8 +297,12 @@
       expect(out).to eq("WIN")
     end
 
-    it "works if a gem is missing, but it's on a different platform, and the Gemfile has no global source", :bundler => "< 3" do
+    it "works if a gem is missing, but it's on a different platform" do
+      build_repo2
+
       install_gemfile <<-G
+        source "#{file_uri_for(gem_repo2)}"
+
         source "#{file_uri_for(gem_repo1)}" do
           gem "rake", platform: :#{not_local_tag}
         end
@@ -308,6 +312,40 @@
       expect(last_command).to be_success
     end
 
+    it "shows a good error if a gem is missing from the lockfile" do
+      build_repo4 do
+        build_gem "foo"
+        build_gem "bar"
+      end
+
+      gemfile <<-G
+        source "https://gem.repo4"
+
+        gem "foo"
+        gem "bar"
+      G
+
+      lockfile <<~L
+        GEM
+          remote: https://gem.repo4/
+          specs:
+            foo (1.0)
+
+        PLATFORMS
+          #{lockfile_platforms}
+
+        DEPENDENCIES
+          foo
+          bar
+
+        BUNDLED WITH
+           #{Bundler::VERSION}
+      L
+
+      bundle :install, :env => { "BUNDLE_FROZEN" => "true" }, :raise_on_error => false, :artifice => "compact_index"
+      expect(err).to include("Your lock file is missing \"bar\", but the lockfile can't be updated because frozen mode is set")
+    end
+
     it "explodes if a path gem is missing" do
       build_lib "path_gem"
       install_gemfile <<-G
@@ -333,7 +371,7 @@
 
       ENV["BUNDLE_FROZEN"] = "1"
       bundle "install", :raise_on_error => false
-      expect(err).to include("deployment mode")
+      expect(err).to include("frozen mode")
       expect(err).to include("You have added to the Gemfile")
       expect(err).to include("* rack-obama")
       expect(err).not_to include("You have deleted from the Gemfile")
@@ -349,7 +387,7 @@
 
       ENV["BUNDLE_DEPLOYMENT"] = "true"
       bundle "install", :raise_on_error => false
-      expect(err).to include("deployment mode")
+      expect(err).to include("frozen mode")
       expect(err).to include("You have added to the Gemfile")
       expect(err).to include("* rack-obama")
       expect(err).not_to include("You have deleted from the Gemfile")
@@ -379,7 +417,7 @@
       ENV["BUNDLE_FROZEN"] = "false"
       ENV["BUNDLE_DEPLOYMENT"] = "false"
       bundle "install"
-      expect(out).not_to include("deployment mode")
+      expect(out).not_to include("frozen mode")
       expect(out).not_to include("You have added to the Gemfile")
       expect(out).not_to include("* rack-obama")
     end
@@ -392,7 +430,7 @@
 
       bundle "config set --local deployment true"
       bundle :install, :raise_on_error => false
-      expect(err).to include("deployment mode")
+      expect(err).to include("frozen mode")
       expect(err).to include("You have added to the Gemfile:\n* activesupport\n\n")
       expect(err).to include("You have deleted from the Gemfile:\n* rack")
       expect(err).not_to include("You have changed in the Gemfile")
@@ -406,7 +444,7 @@
 
       bundle "config set --local deployment true"
       bundle :install, :raise_on_error => false
-      expect(err).to include("deployment mode")
+      expect(err).to include("frozen mode")
       expect(err).not_to include("You have added to the Gemfile")
       expect(err).to include("You have changed in the Gemfile:\n* rack from `no specified source` to `git://hubz.com`")
     end
@@ -426,7 +464,7 @@
 
       bundle "config set --local deployment true"
       bundle :install, :raise_on_error => false
-      expect(err).to include("deployment mode")
+      expect(err).to include("frozen mode")
       expect(err).not_to include("You have deleted from the Gemfile")
       expect(err).not_to include("You have added to the Gemfile")
       expect(err).to include("You have changed in the Gemfile:\n* rack from `#{lib_path("rack-1.0")}` to `no specified source`")
@@ -450,7 +488,7 @@
 
       bundle "config set --local deployment true"
       bundle :install, :raise_on_error => false
-      expect(err).to include("deployment mode")
+      expect(err).to include("frozen mode")
       expect(err).to include("You have changed in the Gemfile:\n* rack from `#{lib_path("rack")}` to `no specified source`")
       expect(err).not_to include("You have added to the Gemfile")
       expect(err).not_to include("You have deleted from the Gemfile")
@@ -469,7 +507,7 @@
 
       run "require 'rack'", :raise_on_error => false
       expect(err).to include strip_whitespace(<<-E).strip
-The dependencies in your gemfile changed
+The dependencies in your gemfile changed, but the lockfile can't be updated because frozen mode is set (Bundler::ProductionError)
 
 You have added to the Gemfile:
 * rack (= 1.0.0)
@@ -502,7 +540,7 @@
       simulate_new_machine
       bundle "config set --local deployment true"
       bundle "install --verbose"
-      expect(out).not_to include("You are trying to install in deployment mode after changing your Gemfile")
+      expect(out).not_to include("but the lockfile can't be updated because frozen mode is set")
       expect(out).not_to include("You have added to the Gemfile")
       expect(out).not_to include("You have deleted from the Gemfile")
       expect(out).to include("vendor/cache/foo")

bundler/spec/install/gemfile/gemspec_spec.rb

@@ -150,7 +150,7 @@
     output = bundle("install", :dir => tmp.join("foo"))
     expect(output).not_to match(/You have added to the Gemfile/)
     expect(output).not_to match(/You have deleted from the Gemfile/)
-    expect(output).not_to match(/install in deployment mode after changing/)
+    expect(output).not_to match(/the lockfile can't be updated because frozen mode is set/)
   end
 
   it "should match a lockfile without needing to re-resolve" do

bundler/spec/lock/lockfile_spec.rb

@@ -1371,7 +1371,7 @@
     L
 
     bundle "install --verbose"
-    expect(out).to include("re-resolving dependencies because your lock file is missing some gems")
+    expect(out).to include("re-resolving dependencies because your lock file is missing \"minitest-bisect\"")
 
     expect(lockfile).to eq <<~L
       GEM