Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor lockfile generation and deprecate Definition#lock with explicit lockfile #7047

Merged
merged 7 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions bundler/lib/bundler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -200,12 +200,13 @@ def environment
#
# @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

Expand Down
9 changes: 5 additions & 4 deletions bundler/lib/bundler/cli/lock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,11 @@ def run
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]

Expand All @@ -60,10 +63,8 @@ def run
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

Expand Down
82 changes: 54 additions & 28 deletions bundler/lib/bundler/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -320,38 +320,26 @@ def groups
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
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

preserve_unknown_sections ||= !updating_major && (Bundler.frozen_bundle? || !(unlocking? || @unlocking_bundler))
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"
deivid-rodriguez marked this conversation as resolved.
Show resolved Hide resolved
end

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
msg = "`Definition#lock` was passed a target file argument. #{suggestion}"

if Bundler.frozen_bundle?
Bundler.ui.error "Cannot write a changed lockfile while frozen."
return
Bundler::SharedHelpers.major_deprecation 2, msg
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
Expand Down Expand Up @@ -518,7 +506,45 @@ def should_add_extra_platforms?
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
Expand Down
2 changes: 1 addition & 1 deletion bundler/lib/bundler/injector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def inject(gemfile_path, lockfile_path)
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!
Expand Down
4 changes: 2 additions & 2 deletions bundler/lib/bundler/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,8 @@ def resolve_if_needed(options)
true
end

def lock(opts = {})
@definition.lock(Bundler.default_lockfile, opts[:preserve_unknown_sections])
def lock
@definition.lock
martinemde marked this conversation as resolved.
Show resolved Hide resolved
end
end
end
2 changes: 1 addition & 1 deletion bundler/lib/bundler/runtime.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def self.definition_method(meth)

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
Expand Down
23 changes: 10 additions & 13 deletions bundler/spec/bundler/definition_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,40 +5,37 @@
RSpec.describe Bundler::Definition do
describe "#lock" do
before do
allow(Bundler).to receive(:settings) { Bundler::Settings.new(".") }
allow(Bundler::SharedHelpers).to receive(:find_gemfile) { Pathname.new("Gemfile") }
allow(Bundler::SharedHelpers).to receive(:find_gemfile) { bundled_app_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, []) }

subject { Bundler::Definition.new(bundled_app_lock, [], Bundler::SourceList.new, {}) }

context "when it's not possible to write to the file" do
it "raises an PermissionError with explanation" do
allow(File).to receive(:open).and_call_original
expect(File).to receive(:open).with("Gemfile.lock", "wb").
expect(File).to receive(:open).with(bundled_app_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, []) }

it "raises a TemporaryResourceError with explanation" do
allow(File).to receive(:open).and_call_original
expect(File).to receive(:open).with("Gemfile.lock", "wb").
expect(File).to receive(:open).with(bundled_app_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, []) }
before { Bundler::Definition.no_lock = true }
after { Bundler::Definition.no_lock = false }

it "does not create a lock file" do
subject.lock("Gemfile.lock")
expect(File.file?("Gemfile.lock")).to eq false
subject.lock
expect(bundled_app_lock).not_to be_file
end
end
end
Expand Down