From d85ca3f76927a5b4901c7c25c521aed0f16e2db7 Mon Sep 17 00:00:00 2001 From: James Couball Date: Mon, 31 Mar 2025 13:45:21 -0700 Subject: [PATCH 1/3] chore: move option validators to their own module --- lib/ruby_git.rb | 1 + lib/ruby_git/option_validators.rb | 37 +++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 lib/ruby_git/option_validators.rb diff --git a/lib/ruby_git.rb b/lib/ruby_git.rb index a3ac9b6..e94403b 100644 --- a/lib/ruby_git.rb +++ b/lib/ruby_git.rb @@ -3,6 +3,7 @@ require_relative 'ruby_git/command_line' require_relative 'ruby_git/encoding_normalizer' require_relative 'ruby_git/errors' +require_relative 'ruby_git/option_validators' require_relative 'ruby_git/repository' require_relative 'ruby_git/status' require_relative 'ruby_git/version' diff --git a/lib/ruby_git/option_validators.rb b/lib/ruby_git/option_validators.rb new file mode 100644 index 0000000..4645783 --- /dev/null +++ b/lib/ruby_git/option_validators.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module RubyGit + # Module containing option validators for RubyGit + # @api public + module OptionValidators + # Raise an error if an option is not a Boolean (or optionally nil) value + # @param name [String] the name of the option + # @param value [Object] the value of the option + # @param nullable [Boolean] whether the option can be nil (default is false) + # @return [void] + # @raise [ArgumentError] if the option is not a Boolean (or optionally nil) value + # @api private + def validate_boolean_option(name:, value:, nullable: false) + return if nullable && value.nil? + + return if [true, false].include?(value) + + raise ArgumentError, "The '#{name}:' option must be a Boolean value but was #{value.inspect}" + end + + # Raise an error if an option is not a String (or optionally nil) value + # @param name [String] the name of the option + # @param value [Object] the value of the option + # @param nullable [Boolean] whether the option can be nil (default is false) + # @return [void] + # @raise [ArgumentError] if the option is not a String (or optionally nil) value + # @api private + def validate_string_option(name:, value:, nullable: false) + return if nullable && value.nil? + + return if value.is_a?(String) + + raise ArgumentError, "The '#{name}:' option must be a String or nil but was #{value.inspect}" + end + end +end From b8a9093ea2decab694b833aeeb3a964a566c6d2e Mon Sep 17 00:00:00 2001 From: James Couball Date: Mon, 31 Mar 2025 13:46:24 -0700 Subject: [PATCH 2/3] feat: add initial_branch option to RubyGit::Worktree.init --- lib/ruby_git/worktree.rb | 39 ++++---- spec/lib/ruby_git/worktree_init_spec.rb | 128 ++++++++++++++++++------ 2 files changed, 117 insertions(+), 50 deletions(-) diff --git a/lib/ruby_git/worktree.rb b/lib/ruby_git/worktree.rb index 46ffff4..ebe0017 100644 --- a/lib/ruby_git/worktree.rb +++ b/lib/ruby_git/worktree.rb @@ -7,6 +7,9 @@ module RubyGit # Create a new Worktree using {.init}, {.clone}, or {.open}. # class Worktree + extend RubyGit::OptionValidators + include RubyGit::OptionValidators + # The root path of the working tree # # @example @@ -28,18 +31,29 @@ class Worktree # @example # worktree = Worktree.init(worktree_path) # - # @param [String] worktree_path the root path of a Git working tree + # @param worktree_path [String] the root path of a Git working tree + # @param initial_branch [String] the initial branch in the newly created repository # - # @raise [RubyGit::Error] if worktree_path is not a directory + # @raise [ArgumentError] if worktree_path does not exist or is not a directory + # @raise [RubyGit::Error] if there is an error initializing the repository # # @return [RubyGit::Worktree] the working tree whose root is at `path` # - def self.init(worktree_path) - raise RubyGit::Error, "Path '#{worktree_path}' not valid." unless File.directory?(worktree_path) + def self.init(worktree_path, initial_branch: nil) # rubocop:disable Metrics/MethodLength + validate_string_option(name: :initial_branch, value: initial_branch, nullable: true) command = ['init'] + command << '--initial-branch' << initial_branch unless initial_branch.nil? + options = { chdir: worktree_path, out: StringIO.new, err: StringIO.new } - RubyGit::CommandLine.run(*command, **options) + + begin + RubyGit::CommandLine.run(*command, **options) + rescue Errno::ENOENT + raise ArgumentError, "Directory '#{worktree_path}' does not exist." + rescue Errno::ENOTDIR + raise ArgumentError, "Path '#{worktree_path}' is not a directory." + end new(worktree_path) end @@ -300,20 +314,5 @@ def root_path(worktree_path) def run_with_context(*command, **options) RubyGit::CommandLine.run(*command, repository_path: repository.path, worktree_path: path, **options) end - - # Raise an error if an option is not a Boolean (or optionally nil) value - # @param name [String] the name of the option - # @param value [Object] the value of the option - # @param nullable [Boolean] whether the option can be nil (default is false) - # @return [void] - # @raise [ArgumentError] if the option is not a Boolean (or optionally nil) value - # @api private - def validate_boolean_option(name:, value:, nullable: false) - return if nullable && value.nil? - - return if [true, false].include?(value) - - raise ArgumentError, "The '#{name}:' option must be a Boolean value but was #{value.inspect}" - end end end diff --git a/spec/lib/ruby_git/worktree_init_spec.rb b/spec/lib/ruby_git/worktree_init_spec.rb index 58fcdfd..7138fca 100644 --- a/spec/lib/ruby_git/worktree_init_spec.rb +++ b/spec/lib/ruby_git/worktree_init_spec.rb @@ -3,46 +3,114 @@ require 'tmpdir' RSpec.describe RubyGit::Worktree do - describe '.init(worktree_path)' do - subject { described_class.init(worktree_path) } - let(:tmpdir) { Dir.mktmpdir } - after { FileUtils.rm_rf(tmpdir) } - - context 'when worktree_path does not exist' do - let(:worktree_path) { tmpdir } - before { FileUtils.rmdir(tmpdir) } - it 'should raise a RubyGit::Error' do - expect { subject }.to raise_error(RubyGit::Error) - end - end + describe '.init' do + subject { described_class.init(worktree_path, initial_branch: initial_branch) } + let(:initial_branch) { nil } - context 'when worktree_path exists' do - let(:worktree_path) { tmpdir } - context 'and is not a directory' do - before do - FileUtils.rmdir(worktree_path) - FileUtils.touch(worktree_path) + describe 'initializing a worktree' do + let(:tmpdir) { @tmpdir } + + around do |example| + in_temp_dir do |tmpdir| + @tmpdir = tmpdir + example.run end - it 'should raise RubyGit::Error' do - expect { subject }.to raise_error(RubyGit::Error) + end + + context 'when worktree_path does not exist' do + let(:worktree_path) { File.join(tmpdir, 'subdir') } + + it 'should raise an ArgumentError' do + expect { subject }.to raise_error(ArgumentError) end end - context 'and is a directory' do - context 'and is in a working tree' do + context 'when worktree_path exists' do + let(:worktree_path) { tmpdir } + context 'and is not a directory' do before do - raise RuntimeError unless system('git init', chdir: worktree_path, %i[out err] => IO::NULL) + FileUtils.rmdir(worktree_path) + FileUtils.touch(worktree_path) + end + it 'should raise an ArgumentError' do + expect { subject }.to raise_error(ArgumentError) + end + end + + context 'and is a directory' do + context 'and is in a working tree' do + before do + raise RuntimeError unless system('git init', chdir: worktree_path, %i[out err] => IO::NULL) + end + it 'should return a Worktree object to the existing working tree' do + expect(subject).to be_kind_of(RubyGit::Worktree) + expect(subject).to have_attributes(path: File.realpath(worktree_path)) + end + end + + context 'and is not in the working tree' do + it 'should initialize the working tree and return a Worktree object' do + expect(subject).to be_kind_of(RubyGit::Worktree) + expect(subject).to have_attributes(path: File.realpath(worktree_path)) + end + end + end + end + end + + describe 'constructing the git command line' do + let(:worktree_path) { '/my/worktree/path' } + let(:result) { instance_double(RubyGit::CommandLine::Result, stdout: '') } + + before do + allow(described_class).to( + receive(:new).with(worktree_path).and_return(instance_double(RubyGit::Worktree)) + ) + end + + context 'called with no arguments' do + let(:expected_command) { %w[init] } + + it 'should run the expected git command' do + expect(RubyGit::CommandLine).to( + receive(:run).with(*expected_command, Hash).and_return(result) + ) + + subject + end + end + + describe 'initial_branch option' do + context 'when nil' do + let(:expected_command) { %w[init] } + + it 'should run the expected git command' do + expect(RubyGit::CommandLine).to( + receive(:run).with(*expected_command, Hash).and_return(result) + ) + subject end - it 'should return a Worktree object to the existing working tree' do - expect(subject).to be_kind_of(RubyGit::Worktree) - expect(subject).to have_attributes(path: File.realpath(worktree_path)) + end + + context 'when a string' do + let(:expected_command) { ['init', '--initial-branch', initial_branch] } + let(:initial_branch) { 'my-branch' } + + it 'should run the expected git command' do + expect(RubyGit::CommandLine).to( + receive(:run).with(*expected_command, Hash).and_return(result) + ) + subject end end - context 'and is not in the working tree' do - it 'should initialize the working tree and return a Worktree object' do - expect(subject).to be_kind_of(RubyGit::Worktree) - expect(subject).to have_attributes(path: File.realpath(worktree_path)) + context 'when not a string' do + let(:initial_branch) { 123 } + + it 'should raise an ArgumentError' do + expect { subject }.to( + raise_error(ArgumentError, %(The 'initial_branch:' option must be a String or nil but was 123)) + ) end end end From 1b3c3481880fffacd40b4721c574e367912c8bc2 Mon Sep 17 00:00:00 2001 From: James Couball Date: Tue, 1 Apr 2025 11:12:27 -0700 Subject: [PATCH 3/3] fix: rewrap any errors raised by Process.spawn in RubyGit::SpawnError Different Ruby engines / platforms (MRI, MRI on Windows, JRuby, TruffleRuby might raise different errors for a similar reasons due to differences in how they implement Process.spawn. This change raises a consistent error when Process.spawn raises an error. Process.spawn does this when it can't even run the given command. If there is a problem once the command is running (uncaught signal, timeout, non-zero exitstatus), the the returned result will contain that information. --- lib/ruby_git/command_line/runner.rb | 33 +++++++++++++++++++------ lib/ruby_git/errors.rb | 8 ++++++ lib/ruby_git/worktree.rb | 10 ++------ spec/lib/ruby_git/worktree_init_spec.rb | 13 +++++++--- 4 files changed, 45 insertions(+), 19 deletions(-) diff --git a/lib/ruby_git/command_line/runner.rb b/lib/ruby_git/command_line/runner.rb index fd3b9cb..03fa66d 100644 --- a/lib/ruby_git/command_line/runner.rb +++ b/lib/ruby_git/command_line/runner.rb @@ -187,20 +187,39 @@ def call(*args, **options_hash) # # @api private # - def run_with_chdir(args, options) - return ProcessExecuter.run_with_options(args, options) unless jruby? && options.chdir != :not_set + def run_with_chdir(args, options) # rubocop:disable Metrics/MethodLength + return run_and_handle_spawn_error(args, options) unless jruby? && options.chdir != :not_set # :nocov: Not executed in MRI Ruby - Dir.chdir(options.chdir) do - saved_chdir = options.chdir - options.merge!(chdir: :not_set) - ProcessExecuter.run_with_options(args, options).tap do - options.merge!(chdir: saved_chdir) + begin + Dir.chdir(options.chdir) do + saved_chdir = options.chdir + options.merge!(chdir: :not_set) + run_and_handle_spawn_error(args, options).tap do + options.merge!(chdir: saved_chdir) + end end + rescue Errno::ENOENT, Errno::ENOTDIR => e + raise RubyGit::SpawnError, "chdir(#{options.chdir}) failed: #{e.message}" end # :nocov: end + # Catch ProcessExecuter::SpawnError and raise a RubyGit::SpawnError in its place + # + # @param args [Array] the command to run + # @param options [RubyGit::CommandLine::Options] the options to pass to `Process.spawn` + # + # @return [ProcessExecuter::Result] the result of the command + # + # @api private + # + def run_and_handle_spawn_error(args, options) + ProcessExecuter.run_with_options(args, options) + rescue ProcessExecuter::SpawnError => e + raise RubyGit::SpawnError, e.message + end + # Returns true if running on JRuby # # @return [Boolean] diff --git a/lib/ruby_git/errors.rb b/lib/ruby_git/errors.rb index 4384342..574b95c 100644 --- a/lib/ruby_git/errors.rb +++ b/lib/ruby_git/errors.rb @@ -21,6 +21,7 @@ module RubyGit # │ └─> RubyGit::SignaledError # │ └─> RubyGit::TimeoutError # ├─> RubyGit::ProcessIOError + # ├─> RubyGit::SpawnError # └─> RubyGit::UnexpectedResultError # ``` # @@ -32,6 +33,7 @@ module RubyGit # | `SignaledError` | This error is raised when the git command line is terminated as a result of receiving a signal. This could happen if the process is forcibly terminated or if there is a serious system error. | # | `TimeoutError` | This is a specific type of `SignaledError` that is raised when the git command line operation times out and is killed via the SIGKILL signal. This happens if the operation takes longer than the timeout duration configured in `Git.config.timeout` or via the `:timeout` parameter given in git methods that support timeouts. | # | `ProcessIOError` | An error was encountered reading or writing to a subprocess. | + # | `SpawnError` | An error was encountered when spawning a subprocess and it never started. | # | `UnexpectedResultError` | The command line ran without error but did not return the expected results. | # # @example Rescuing a generic error @@ -166,4 +168,10 @@ class ProcessIOError < RubyGit::Error; end # @api public # class UnexpectedResultError < RubyGit::Error; end + + # Raised when the git command could not be spawned + # + # @api public + # + class SpawnError < RubyGit::Error; end end diff --git a/lib/ruby_git/worktree.rb b/lib/ruby_git/worktree.rb index ebe0017..567c720 100644 --- a/lib/ruby_git/worktree.rb +++ b/lib/ruby_git/worktree.rb @@ -39,7 +39,7 @@ class Worktree # # @return [RubyGit::Worktree] the working tree whose root is at `path` # - def self.init(worktree_path, initial_branch: nil) # rubocop:disable Metrics/MethodLength + def self.init(worktree_path, initial_branch: nil) validate_string_option(name: :initial_branch, value: initial_branch, nullable: true) command = ['init'] @@ -47,13 +47,7 @@ def self.init(worktree_path, initial_branch: nil) # rubocop:disable Metrics/Meth options = { chdir: worktree_path, out: StringIO.new, err: StringIO.new } - begin - RubyGit::CommandLine.run(*command, **options) - rescue Errno::ENOENT - raise ArgumentError, "Directory '#{worktree_path}' does not exist." - rescue Errno::ENOTDIR - raise ArgumentError, "Path '#{worktree_path}' is not a directory." - end + RubyGit::CommandLine.run(*command, **options) new(worktree_path) end diff --git a/spec/lib/ruby_git/worktree_init_spec.rb b/spec/lib/ruby_git/worktree_init_spec.rb index 7138fca..3d8b238 100644 --- a/spec/lib/ruby_git/worktree_init_spec.rb +++ b/spec/lib/ruby_git/worktree_init_spec.rb @@ -20,8 +20,10 @@ context 'when worktree_path does not exist' do let(:worktree_path) { File.join(tmpdir, 'subdir') } - it 'should raise an ArgumentError' do - expect { subject }.to raise_error(ArgumentError) + expected_error = truffleruby? ? RubyGit::FailedError : RubyGit::SpawnError + + it "should raise an error #{expected_error}" do + expect { subject }.to raise_error(expected_error) end end @@ -32,8 +34,11 @@ FileUtils.rmdir(worktree_path) FileUtils.touch(worktree_path) end - it 'should raise an ArgumentError' do - expect { subject }.to raise_error(ArgumentError) + + expected_error = truffleruby? ? RubyGit::FailedError : RubyGit::SpawnError + + it "should raise a #{expected_error} " do + expect { subject }.to raise_error(expected_error) end end