[rubygems/rubygems] Refactor lockfile generation

https://github.com/rubygems/rubygems/commit/6a0c03c77f

7 files changed, 70 insertions, 44 deletions

diff --git a/lib/bundler.rb b/lib/bundler.rb
index 7bb6690e52..59a1107bb7 100644
--- a/lib/bundler.rb
+++ b/lib/bundler.rb
@@ -200,12 +200,13 @@ module Bundler
     #
     # @param unlock [Hash, Boolean, nil] Gems that have been requested
     #   to be updated or true if all gems should be updated
+    # @param lockfile [Pathname] Path to Gemfile.lock
     # @return [Bundler::Definition]
-    def definition(unlock = nil)
+    def definition(unlock = nil, lockfile = default_lockfile)
       @definition = nil if unlock
       @definition ||= begin
         configure
-        Definition.build(default_gemfile, default_lockfile, unlock)
+        Definition.build(default_gemfile, lockfile, unlock)
       end
     end
 
diff --git a/lib/bundler/cli/lock.rb b/lib/bundler/cli/lock.rb
index 7247121df5..dac3d2a09a 100644
--- a/lib/bundler/cli/lock.rb
+++ b/lib/bundler/cli/lock.rb
@@ -33,8 +33,11 @@ module Bundler
         update = { bundler: bundler }
       end
 
+      file = options[:lockfile]
+      file = file ? Pathname.new(file).expand_path : Bundler.default_lockfile
+
       Bundler.settings.temporary(frozen: false) do
-        definition = Bundler.definition(update)
+        definition = Bundler.definition(update, file)
 
         Bundler::CLI::Common.configure_gem_version_promoter(definition, options) if options[:update]
 
@@ -60,10 +63,8 @@ module Bundler
         if print
           puts definition.to_lock
         else
-          file = options[:lockfile]
-          file = file ? File.expand_path(file) : Bundler.default_lockfile
           puts "Writing lockfile to #{file}"
-          definition.lock(file)
+          definition.lock
         end
       end
 
diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb
index 0b0e63f77e..32a7089925 100644
--- a/lib/bundler/definition.rb
+++ b/lib/bundler/definition.rb
@@ -320,38 +320,24 @@ module Bundler
       dependencies.map(&:groups).flatten.uniq
     end
 
-    def lock(file, preserve_unknown_sections = false)
-      return if Definition.no_lock
-
-      contents = to_lock
-
-      # Convert to \r\n if the existing lock has them
-      # i.e., Windows with `git config core.autocrlf=true`
-      contents.gsub!(/\n/, "\r\n") if @lockfile_contents.match?("\r\n")
-
-      if @locked_bundler_version
-        locked_major = @locked_bundler_version.segments.first
-        current_major = bundler_version_to_lock.segments.first
-
-        updating_major = locked_major < current_major
-      end
-
-      preserve_unknown_sections ||= !updating_major && (Bundler.frozen_bundle? || !(unlocking? || @unlocking_bundler))
+    def lock(file_or_preserve_unknown_sections = false, preserve_unknown_sections_or_unused = false)
+      if [true, false, nil].include?(file_or_preserve_unknown_sections)
+        target_lockfile = lockfile || Bundler.default_lockfile
+        preserve_unknown_sections = file_or_preserve_unknown_sections
+      else
+        target_lockfile = file_or_preserve_unknown_sections
+        preserve_unknown_sections = preserve_unknown_sections_or_unused
 
-      if file && File.exist?(file) && lockfiles_equal?(@lockfile_contents, contents, preserve_unknown_sections)
-        return if Bundler.frozen_bundle?
-        SharedHelpers.filesystem_access(file) { FileUtils.touch(file) }
-        return
-      end
+        suggestion = if target_lockfile == lockfile
+          "To fix this warning, remove it from the `Definition#lock` call."
+        else
+          "Instead, instantiate a new definition passing `#{target_lockfile}`, and call `lock` without a file argument on that definition"
+        end
 
-      if Bundler.frozen_bundle?
-        Bundler.ui.error "Cannot write a changed lockfile while frozen."
-        return
+        warn "Passing a file to `Definition#lock` is deprecated. #{suggestion}"
       end
 
-      SharedHelpers.filesystem_access(file) do |p|
-        File.open(p, "wb") {|f| f.puts(contents) }
-      end
+      write_lock(target_lockfile, preserve_unknown_sections)
     end
 
     def locked_ruby_version
@@ -518,7 +504,45 @@ module Bundler
     end
 
     def lockfile_exists?
-      lockfile && File.exist?(lockfile)
+      file_exists?(lockfile)
+    end
+
+    def file_exists?(file)
+      file && File.exist?(file)
+    end
+
+    def write_lock(file, preserve_unknown_sections)
+      return if Definition.no_lock
+
+      contents = to_lock
+
+      # Convert to \r\n if the existing lock has them
+      # i.e., Windows with `git config core.autocrlf=true`
+      contents.gsub!(/\n/, "\r\n") if @lockfile_contents.match?("\r\n")
+
+      if @locked_bundler_version
+        locked_major = @locked_bundler_version.segments.first
+        current_major = bundler_version_to_lock.segments.first
+
+        updating_major = locked_major < current_major
+      end
+
+      preserve_unknown_sections ||= !updating_major && (Bundler.frozen_bundle? || !(unlocking? || @unlocking_bundler))
+
+      if file_exists?(file) && lockfiles_equal?(@lockfile_contents, contents, preserve_unknown_sections)
+        return if Bundler.frozen_bundle?
+        SharedHelpers.filesystem_access(file) { FileUtils.touch(file) }
+        return
+      end
+
+      if Bundler.frozen_bundle?
+        Bundler.ui.error "Cannot write a changed lockfile while frozen."
+        return
+      end
+
+      SharedHelpers.filesystem_access(file) do |p|
+        File.open(p, "wb") {|f| f.puts(contents) }
+      end
     end
 
     def resolver
diff --git a/lib/bundler/injector.rb b/lib/bundler/injector.rb
index 2cf7754ecb..cf561c2ee4 100644
--- a/lib/bundler/injector.rb
+++ b/lib/bundler/injector.rb
@@ -50,7 +50,7 @@ module Bundler
         append_to(gemfile_path, build_gem_lines(@options[:conservative_versioning])) if @deps.any?
 
         # since we resolved successfully, write out the lockfile
-        @definition.lock(Bundler.default_lockfile)
+        @definition.lock
 
         # invalidate the cached Bundler.definition
         Bundler.reset_paths!
diff --git a/lib/bundler/installer.rb b/lib/bundler/installer.rb
index 5245da2e0e..018324f840 100644
--- a/lib/bundler/installer.rb
+++ b/lib/bundler/installer.rb
@@ -261,7 +261,7 @@ module Bundler
     end
 
     def lock
-      @definition.lock(Bundler.default_lockfile)
+      @definition.lock
     end
   end
 end
diff --git a/lib/bundler/runtime.rb b/lib/bundler/runtime.rb
index 993f1082c3..ec772cfe7b 100644
--- a/lib/bundler/runtime.rb
+++ b/lib/bundler/runtime.rb
@@ -95,7 +95,7 @@ module Bundler
 
     def lock(opts = {})
       return if @definition.no_resolve_needed?
-      @definition.lock(Bundler.default_lockfile, opts[:preserve_unknown_sections])
+      @definition.lock(opts[:preserve_unknown_sections])
     end
 
     alias_method :gems, :specs
diff --git a/spec/bundler/bundler/definition_spec.rb b/spec/bundler/bundler/definition_spec.rb
index a19cf789ee..85f13d287c 100644
--- a/spec/bundler/bundler/definition_spec.rb
+++ b/spec/bundler/bundler/definition_spec.rb
@@ -10,34 +10,34 @@ RSpec.describe Bundler::Definition do
       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, []) }
+      subject { Bundler::Definition.new("Gemfile.lock", [], Bundler::SourceList.new, []) }
 
       it "raises an PermissionError with explanation" do
         allow(File).to receive(:open).and_call_original
         expect(File).to receive(:open).with("Gemfile.lock", "wb").
           and_raise(Errno::EACCES)
-        expect { subject.lock("Gemfile.lock") }.
+        expect { subject.lock }.
           to raise_error(Bundler::PermissionError, /Gemfile\.lock/)
       end
     end
     context "when a temporary resource access issue occurs" do
-      subject { Bundler::Definition.new(nil, [], Bundler::SourceList.new, []) }
+      subject { Bundler::Definition.new("Gemfile.lock", [], Bundler::SourceList.new, []) }
 
       it "raises a TemporaryResourceError with explanation" do
         allow(File).to receive(:open).and_call_original
         expect(File).to receive(:open).with("Gemfile.lock", "wb").
           and_raise(Errno::EAGAIN)
-        expect { subject.lock("Gemfile.lock") }.
+        expect { subject.lock }.
           to raise_error(Bundler::TemporaryResourceError, /temporarily unavailable/)
       end
     end
     context "when Bundler::Definition.no_lock is set to true" do
-      subject { Bundler::Definition.new(nil, [], Bundler::SourceList.new, []) }
+      subject { Bundler::Definition.new("Gemfile.lock", [], Bundler::SourceList.new, []) }
       before { Bundler::Definition.no_lock = true }
       after { Bundler::Definition.no_lock = false }
 
       it "does not create a lock file" do
-        subject.lock("Gemfile.lock")
+        subject.lock
         expect(File.file?("Gemfile.lock")).to eq false
       end
     end