From fb93ef14def222d6eca29f49a5f810a3d6de5787 Mon Sep 17 00:00:00 2001 From: James Couball Date: Tue, 1 Jul 2025 23:00:18 -0700 Subject: [PATCH 1/6] feat!: upgrade minimally supported Ruby to 3.2 Update the CI builds to build with MRI Ruby 3.2, 3.3, and 3.4; TruffleRuby 24.2.1; and JRuby 10.0.0.1. BREAKING CHANGE: Users will need to be on Ruby 3.2 or greater --- .github/workflows/continuous_integration.yml | 14 +++++++++++--- .../experimental_continuous_integration.yml | 9 +++++++++ git.gemspec | 2 +- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/.github/workflows/continuous_integration.yml b/.github/workflows/continuous_integration.yml index 0e7cd259..3aed702e 100644 --- a/.github/workflows/continuous_integration.yml +++ b/.github/workflows/continuous_integration.yml @@ -22,18 +22,26 @@ jobs: fail-fast: false matrix: # Only the latest versions of JRuby and TruffleRuby are tested - ruby: ["3.1", "3.2", "3.3", "3.4", "truffleruby-24.1.2", "jruby-9.4.12.0"] + ruby: ["3.2", "3.3", "3.4", "truffleruby-24.2.1", "jruby-10.0.0.1"] operating-system: [ubuntu-latest] experimental: [No] + java_version: [""] include: - - # Only test with minimal Ruby version on Windows - ruby: 3.1 + - ruby: 3.2 operating-system: windows-latest + experimental: No steps: - name: Checkout Code uses: actions/checkout@v4 + - name: Setup Java + if: matrix.java_version != '' + uses: actions/setup-java@v4 + with: + distribution: 'temurin' + java-version: ${{ matrix.java_version }} + - name: Setup Ruby uses: ruby/setup-ruby@v1 with: diff --git a/.github/workflows/experimental_continuous_integration.yml b/.github/workflows/experimental_continuous_integration.yml index b1256714..f9d08c46 100644 --- a/.github/workflows/experimental_continuous_integration.yml +++ b/.github/workflows/experimental_continuous_integration.yml @@ -27,16 +27,25 @@ jobs: ruby: head operating-system: ubuntu-latest experimental: Yes + java_version: "" - # Since JRuby on Windows is known to not work, consider this experimental ruby: jruby-head operating-system: windows-latest experimental: Yes + java_version: "21" steps: - name: Checkout Code uses: actions/checkout@v4 + - name: Setup Java + if: matrix.java_version != '' + uses: actions/setup-java@v4 + with: + distribution: 'temurin' + java-version: ${{ matrix.java_version }} + - name: Setup Ruby uses: ruby/setup-ruby@v1 with: diff --git a/git.gemspec b/git.gemspec index f8c49bdc..e62b562b 100644 --- a/git.gemspec +++ b/git.gemspec @@ -24,7 +24,7 @@ Gem::Specification.new do |s| s.metadata['documentation_uri'] = "https://rubydoc.info/gems/#{s.name}/#{s.version}" s.require_paths = ['lib'] - s.required_ruby_version = '>= 3.0.0' + s.required_ruby_version = '>= 3.2.0' s.requirements = ['git 2.28.0 or greater'] s.add_runtime_dependency 'activesupport', '>= 5.0' From 5b00d3b9c4063c9988d844eec9ddedddb8c26446 Mon Sep 17 00:00:00 2001 From: James Couball Date: Wed, 2 Jul 2025 10:58:59 -0700 Subject: [PATCH 2/6] chore: upgrade to ProcessExecuter 4.x --- git.gemspec | 2 +- lib/git/command_line.rb | 21 +++++++++--------- lib/git/command_line_result.rb | 12 +++++++--- tests/test_helper.rb | 38 ++++---------------------------- tests/units/test_command_line.rb | 9 +++----- 5 files changed, 28 insertions(+), 54 deletions(-) diff --git a/git.gemspec b/git.gemspec index e62b562b..4aa24899 100644 --- a/git.gemspec +++ b/git.gemspec @@ -29,7 +29,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency 'activesupport', '>= 5.0' s.add_runtime_dependency 'addressable', '~> 2.8' - s.add_runtime_dependency 'process_executer', '~> 1.3' + s.add_runtime_dependency 'process_executer', '~> 4.0' s.add_runtime_dependency 'rchardet', '~> 1.9' s.add_development_dependency 'create_github_release', '~> 2.1' diff --git a/lib/git/command_line.rb b/lib/git/command_line.rb index 6228a144..0b4a0e73 100644 --- a/lib/git/command_line.rb +++ b/lib/git/command_line.rb @@ -192,8 +192,13 @@ def initialize(env, binary_path, global_opts, logger) def run(*args, out: nil, err: nil, normalize:, chomp:, merge:, chdir: nil, timeout: nil) git_cmd = build_git_cmd(args) begin - result = ProcessExecuter.run(env, *git_cmd, out: out, err: err, merge:, chdir: (chdir || :not_set), timeout: timeout, raise_errors: false) - rescue ProcessExecuter::Command::ProcessIOError => e + options = { chdir: (chdir || :not_set), timeout_after: timeout, raise_errors: false } + options[:out] = out unless out.nil? + options[:err] = err unless err.nil? + options[:merge_output] = merge unless merge.nil? + + result = ProcessExecuter.run_with_capture(env, *git_cmd, **options) + rescue ProcessExecuter::ProcessIOError => e raise Git::ProcessIOError.new(e.message), cause: e.exception.cause end process_result(result, normalize, chomp, timeout) @@ -274,14 +279,10 @@ def post_process_all(raw_outputs, normalize, chomp) # @api private # def post_process(raw_output, normalize, chomp) - if raw_output.respond_to?(:string) - output = raw_output.string.dup - output = output.lines.map { |l| Git::EncodingUtils.normalize_encoding(l) }.join if normalize - output.chomp! if chomp - output - else - nil - end + output = raw_output.dup + output = output.lines.map { |l| Git::EncodingUtils.normalize_encoding(l) }.join if normalize + output.chomp! if chomp + output end end end diff --git a/lib/git/command_line_result.rb b/lib/git/command_line_result.rb index 9194a292..2a37c3c2 100644 --- a/lib/git/command_line_result.rb +++ b/lib/git/command_line_result.rb @@ -19,15 +19,21 @@ class CommandLineResult # result = Git::CommandLineResult.new(git_cmd, status, stdout, stderr) # # @param git_cmd [Array] the git command that was executed - # @param status [Process::Status] the status of the process - # @param stdout [String] the output of the process - # @param stderr [String] the error output of the process + # @param status [ProcessExecuter::ResultWithCapture] the status of the process + # @param stdout [String] the processed stdout of the process + # @param stderr [String] the processed stderr of the process # def initialize(git_cmd, status, stdout, stderr) @git_cmd = git_cmd @status = status @stdout = stdout @stderr = stderr + + # ProcessExecuter::ResultWithCapture changed the timeout? method to timed_out? + # in version 4.x. This is a compatibility layer to maintain the old method name + # for backward compatibility. + # + status.define_singleton_method(:timeout?) { timed_out? } end # @attribute [r] git_cmd diff --git a/tests/test_helper.rb b/tests/test_helper.rb index f35a0fcd..7378db7a 100644 --- a/tests/test_helper.rb +++ b/tests/test_helper.rb @@ -171,31 +171,6 @@ def windows_platform? RUBY_PLATFORM =~ win_platform_regex || RUBY_DESCRIPTION =~ win_platform_regex end - require 'delegate' - - # A wrapper around a ProcessExecuter::Status that also includes command output - # @api public - class CommandResult < SimpleDelegator - # Create a new CommandResult - # @example - # status = ProcessExecuter.spawn(*command, timeout:, out:, err:) - # CommandResult.new(status, out_buffer.string, err_buffer.string) - # @param status [ProcessExecuter::Status] The status of the process - # @param out [String] The standard output of the process - # @param err [String] The standard error of the process - def initialize(status, out, err) - super(status) - @out = out - @err = err - end - - # @return [String] The stdout output of the process - attr_reader :out - - # @return [String] The stderr output of the process - attr_reader :err - end - # Run a command and return the status including stdout and stderr output # # @example @@ -213,17 +188,12 @@ def initialize(status, out, err) # # @return [CommandResult] The result of running # - def run_command(*command, timeout: nil, raise_errors: true, error_message: "#{command[0]} failed") - out_buffer = StringIO.new - out = ProcessExecuter::MonitoredPipe.new(out_buffer) - err_buffer = StringIO.new - err = ProcessExecuter::MonitoredPipe.new(err_buffer) - - status = ProcessExecuter.spawn(*command, timeout: timeout, out: out, err: err) + def run_command(*command, raise_errors: true, error_message: "#{command[0]} failed") + result = ProcessExecuter.run_with_capture(*command, raise_errors: false) - raise "#{error_message}: #{err_buffer.string}" if raise_errors && !status.success? + raise "#{error_message}: #{result.stderr}" if raise_errors && !result.success? - CommandResult.new(status, out_buffer.string, err_buffer.string) + result end end diff --git a/tests/units/test_command_line.rb b/tests/units/test_command_line.rb index 7062d1aa..5f678b91 100644 --- a/tests/units/test_command_line.rb +++ b/tests/units/test_command_line.rb @@ -61,7 +61,7 @@ def merge command_line = Git::CommandLine.new(env, binary_path, global_opts, logger) args = [] error = assert_raise ArgumentError do - command_line.run(*args, out: out_writer, err: err_writer, normalize: normalize, chomp: chomp, merge: merge, timeout: 'not a number') + command_line.run(*args, normalize: normalize, chomp: chomp, timeout_after: 'not a number') end end @@ -97,7 +97,6 @@ def merge assert_equal([{}, 'ruby', 'bin/command_line_test', '--stdout=stdout output', '--stderr=stderr output'], result.git_cmd) assert_equal('stdout output', result.stdout.chomp) assert_equal('stderr output', result.stderr.chomp) - assert(result.status.is_a? ProcessExecuter::Command::Result) assert_equal(0, result.status.exitstatus) end @@ -239,10 +238,8 @@ def write(*args) command_line = Git::CommandLine.new(env, binary_path, global_opts, logger) args = ['--stderr=ERROR: fatal error', '--stdout=STARTING PROCESS'] Tempfile.create do |f| - err_writer = f - result = command_line.run(*args, out: out_writer, err: err_writer, normalize: normalize, chomp: chomp, merge: merge) - f.rewind - assert_equal('ERROR: fatal error', f.read.chomp) + result = command_line.run(*args, normalize: normalize, chomp: chomp, merge: merge) + assert_equal('ERROR: fatal error', result.stderr.chomp) end end From 28e07ae2e91a8defd52549393bf6f3fcbede122e Mon Sep 17 00:00:00 2001 From: James Couball Date: Wed, 2 Jul 2025 12:35:25 -0700 Subject: [PATCH 3/6] chore: remove unneeded explicit return statements --- lib/git.rb | 2 +- lib/git/diff.rb | 2 +- lib/git/lib.rb | 12 ++++++------ lib/git/log.rb | 20 ++++++++++---------- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/lib/git.rb b/lib/git.rb index 34b70caf..6ef5dc85 100644 --- a/lib/git.rb +++ b/lib/git.rb @@ -65,7 +65,7 @@ def self.configure end def self.config - return Base.config + Base.config end def global_config(name = nil, value = nil) diff --git a/lib/git/diff.rb b/lib/git/diff.rb index 303a0a89..d17d3c08 100644 --- a/lib/git/diff.rb +++ b/lib/git/diff.rb @@ -24,7 +24,7 @@ def name_status def path(path) @path = path - return self + self end def size diff --git a/lib/git/lib.rb b/lib/git/lib.rb index 5a3ade32..203667d0 100644 --- a/lib/git/lib.rb +++ b/lib/git/lib.rb @@ -217,7 +217,7 @@ def describe(commit_ish = nil, opts = {}) arr_opts << commit_ish if commit_ish - return command('describe', *arr_opts) + command('describe', *arr_opts) end # Return the commits that are within the given revision range @@ -472,7 +472,7 @@ def process_commit_data(data, sha) hsh['message'] = data.join("\n") + "\n" - return hsh + hsh end CAT_FILE_HEADER_LINE = /\A(?\w+) (?.*)\z/ @@ -543,7 +543,7 @@ def process_tag_data(data, name) hsh['message'] = data.join("\n") + "\n" - return hsh + hsh end def process_commit_log_data(data) @@ -584,7 +584,7 @@ def process_commit_log_data(data) hsh_array << hsh if hsh - return hsh_array + hsh_array end def ls_tree(sha, opts = {}) @@ -758,7 +758,7 @@ def current_branch_state :unborn end - return HeadState.new(state, branch_name) + HeadState.new(state, branch_name) end def branch_current @@ -1488,7 +1488,7 @@ def archive(sha, file = nil, opts = {}) gz.write(file_content) end end - return file + file end # returns the current version of git, as an Array of Fixnums. diff --git a/lib/git/log.rb b/lib/git/log.rb index 7ac31622..2c0e89f8 100644 --- a/lib/git/log.rb +++ b/lib/git/log.rb @@ -82,61 +82,61 @@ def all def object(objectish) dirty_log @object = objectish - return self + self end def author(regex) dirty_log @author = regex - return self + self end def grep(regex) dirty_log @grep = regex - return self + self end def path(path) dirty_log @path = path - return self + self end def skip(num) dirty_log @skip = num - return self + self end def since(date) dirty_log @since = date - return self + self end def until(date) dirty_log @until = date - return self + self end def between(sha1, sha2 = nil) dirty_log @between = [sha1, sha2] - return self + self end def cherry dirty_log @cherry = true - return self + self end def merges dirty_log @merges = true - return self + self end def to_s From ded54c4b551aefb7de35b9505ce14f2061d1708c Mon Sep 17 00:00:00 2001 From: James Couball Date: Wed, 2 Jul 2025 16:09:22 -0700 Subject: [PATCH 4/6] feat: add Log#execute to run the log and return an immutable result This partially implements #813 Log data access methods directly on the Log class will return a deprecation warning since they will be removed in the future. --- README.md | 19 ++++ lib/git/log.rb | 88 +++++++++++++++++- tests/test_helper.rb | 3 + tests/units/test_log_execute.rb | 154 ++++++++++++++++++++++++++++++++ 4 files changed, 260 insertions(+), 4 deletions(-) create mode 100644 tests/units/test_log_execute.rb diff --git a/README.md b/README.md index f62b42f9..ac319337 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,7 @@ - [Major Objects](#major-objects) - [Errors Raised By This Gem](#errors-raised-by-this-gem) - [Specifying And Handling Timeouts](#specifying-and-handling-timeouts) +- [Deprecations](#deprecations) - [Examples](#examples) - [Ruby version support policy](#ruby-version-support-policy) - [License](#license) @@ -202,6 +203,24 @@ rescue Git::TimeoutError => e end ``` +## Deprecations + +This gem uses ActiveSupport's deprecation mechanism to report deprecation warnings. + +You can silence deprecation warnings by adding this line to your source code: + +```ruby +Git::Deprecation.behavior = :silence +``` + +See [the Active Support Deprecation +documentation](https://api.rubyonrails.org/classes/ActiveSupport/Deprecation.html) +for more details. + +If deprecation warnings are silenced, you should reenable them before upgrading the +git gem to the next major version. This will make it easier to identify changes +needed for the upgrade. + ## Examples Here are a bunch of examples of how to use the Ruby/Git package. diff --git a/lib/git/log.rb b/lib/git/log.rb index 2c0e89f8..3b49e918 100644 --- a/lib/git/log.rb +++ b/lib/git/log.rb @@ -6,13 +6,13 @@ module Git # # @example The last (default number) of commits # git = Git.open('.') - # Git::Log.new(git) #=> Enumerable of the last 30 commits + # Git::Log.new(git).execute #=> Enumerable of the last 30 commits # # @example The last n commits - # Git::Log.new(git).max_commits(50) #=> Enumerable of last 50 commits + # Git::Log.new(git).max_commits(50).execute #=> Enumerable of last 50 commits # # @example All commits returned by `git log` - # Git::Log.new(git).max_count(:all) #=> Enumerable of all commits + # Git::Log.new(git).max_count(:all).execute #=> Enumerable of all commits # # @example All commits that match complex criteria # Git::Log.new(git) @@ -20,12 +20,62 @@ module Git # .object('README.md') # .since('10 years ago') # .between('v1.0.7', 'HEAD') + # .execute # # @api public # class Log include Enumerable + # An immutable collection of commits returned by Git::Log#execute + # + # This object is an Enumerable that contains Git::Object::Commit objects. + # It provides methods to access the commit data without executing any + # further git commands. + # + # @api public + class Result + include Enumerable + + # @private + def initialize(commits) + @commits = commits + end + + # @return [Integer] the number of commits in the result set + def size + @commits.size + end + + # Iterates over each commit in the result set + # + # @yield [Git::Object::Commit] + def each(&block) + @commits.each(&block) + end + + # @return [Git::Object::Commit, nil] the first commit in the result set + def first + @commits.first + end + + # @return [Git::Object::Commit, nil] the last commit in the result set + def last + @commits.last + end + + # @param index [Integer] the index of the commit to return + # @return [Git::Object::Commit, nil] the commit at the given index + def [](index) + @commits[index] + end + + # @return [String] a string representation of the log + def to_s + map { |c| c.to_s }.join("\n") + end + end + # Create a new Git::Log object # # @example @@ -44,6 +94,25 @@ def initialize(base, max_count = 30) max_count(max_count) end + # Executes the git log command and returns an immutable result object. + # + # This is the preferred way to get log data. It separates the query + # building from the execution, making the API more predictable. + # + # @example + # query = g.log.since('2 weeks ago').author('Scott') + # results = query.execute + # puts "Found #{results.size} commits" + # results.each do |commit| + # # ... + # end + # + # @return [Git::Log::Result] an object containing the log results + def execute + run_log + Result.new(@commits) + end + # The maximum number of commits to return # # @example All commits returned by `git log` @@ -140,32 +209,39 @@ def merges end def to_s - self.map { |c| c.to_s }.join("\n") + deprecate_method(__method__) + check_log + @commits.map { |c| c.to_s }.join("\n") end # forces git log to run def size + deprecate_method(__method__) check_log @commits.size rescue nil end def each(&block) + deprecate_method(__method__) check_log @commits.each(&block) end def first + deprecate_method(__method__) check_log @commits.first rescue nil end def last + deprecate_method(__method__) check_log @commits.last rescue nil end def [](index) + deprecate_method(__method__) check_log @commits[index] rescue nil end @@ -173,6 +249,10 @@ def [](index) private + def deprecate_method(method_name) + Git::Deprecation.warn("Calling Git::Log##{method_name} is deprecated and will be removed in a future version. Call #execute and then ##{method_name} on the result object.") + end + def dirty_log @dirty_flag = true end diff --git a/tests/test_helper.rb b/tests/test_helper.rb index 7378db7a..39033732 100644 --- a/tests/test_helper.rb +++ b/tests/test_helper.rb @@ -12,6 +12,9 @@ $stdout.sync = true $stderr.sync = true +# Silence deprecation warnings during tests +Git::Deprecation.behavior = :silence + class Test::Unit::TestCase TEST_ROOT = File.expand_path(__dir__) diff --git a/tests/units/test_log_execute.rb b/tests/units/test_log_execute.rb new file mode 100644 index 00000000..42bfd347 --- /dev/null +++ b/tests/units/test_log_execute.rb @@ -0,0 +1,154 @@ +# frozen_string_literal: true + +require 'logger' +require 'test_helper' + +# Tests for the Git::Log#execute method +class TestLogExecute < Test::Unit::TestCase + def setup + clone_working_repo + #@git = Git.open(@wdir, :log => Logger.new(STDOUT)) + @git = Git.open(@wdir) + end + + def test_log_max_count_default + assert_equal(30, @git.log.execute.size) + end + + # In these tests, note that @git.log(n) is equivalent to @git.log.max_count(n) + def test_log_max_count_20 + assert_equal(20, @git.log(20).execute.size) + assert_equal(20, @git.log.max_count(20).execute.size) + end + + def test_log_max_count_nil + assert_equal(72, @git.log(nil).execute.size) + assert_equal(72, @git.log.max_count(nil).execute.size) + end + + def test_log_max_count_all + assert_equal(72, @git.log(:all).execute.size) + assert_equal(72, @git.log.max_count(:all).execute.size) + end + + # Note that @git.log.all does not control the number of commits returned. For that, + # use @git.log.max_count(n) + def test_log_all + assert_equal(72, @git.log(100).execute.size) + assert_equal(76, @git.log(100).all.execute.size) + end + + def test_log_non_integer_count + assert_raises(ArgumentError) { @git.log('foo').execute } + end + + def test_get_first_and_last_entries + log = @git.log.execute + assert(log.first.is_a?(Git::Object::Commit)) + assert_equal('46abbf07e3c564c723c7c039a43ab3a39e5d02dd', log.first.objectish) + + assert(log.last.is_a?(Git::Object::Commit)) + assert_equal('b03003311ad3fa368b475df58390353868e13c91', log.last.objectish) + end + + def test_get_log_entries + assert_equal(30, @git.log.execute.size) + assert_equal(50, @git.log(50).execute.size) + assert_equal(10, @git.log(10).execute.size) + end + + def test_get_log_to_s + log = @git.log.execute + assert_equal(log.to_s.split("\n").first, log.first.sha) + end + + def test_log_skip + three1 = @git.log(3).execute.to_a[-1] + three2 = @git.log(2).skip(1).execute.to_a[-1] + three3 = @git.log(1).skip(2).execute.to_a[-1] + assert_equal(three2.sha, three3.sha) + assert_equal(three1.sha, three2.sha) + end + + def test_get_log_since + l = @git.log.since("2 seconds ago").execute + assert_equal(0, l.size) + + l = @git.log.since("#{Date.today.year - 2006} years ago").execute + assert_equal(30, l.size) + end + + def test_get_log_grep + l = @git.log.grep("search").execute + assert_equal(2, l.size) + end + + def test_get_log_author + l = @git.log(5).author("chacon").execute + assert_equal(5, l.size) + l = @git.log(5).author("lazySusan").execute + assert_equal(0, l.size) + end + + def test_get_log_since_file + l = @git.log.path('example.txt').execute + assert_equal(30, l.size) + + l = @git.log.between('v2.5', 'test').path('example.txt').execute + assert_equal(1, l.size) + end + + def test_get_log_path + log = @git.log.path('example.txt').execute + assert_equal(30, log.size) + log = @git.log.path('example*').execute + assert_equal(30, log.size) + log = @git.log.path(['example.txt','scott/text.txt']).execute + assert_equal(30, log.size) + end + + def test_log_file_noexist + assert_raise Git::FailedError do + @git.log.object('no-exist.txt').execute + end + end + + def test_log_with_empty_commit_message + Dir.mktmpdir do |dir| + git = Git.init(dir) + expected_message = 'message' + git.commit(expected_message, { allow_empty: true }) + git.commit('', { allow_empty: true, allow_empty_message: true }) + log = git.log.execute + assert_equal(2, log.to_a.size) + assert_equal('', log[0].message) + assert_equal(expected_message, log[1].message) + end + end + + def test_log_cherry + l = @git.log.between( 'master', 'cherry').cherry.execute + assert_equal( 1, l.size ) + end + + def test_log_merges + expected_command_line = ['log', '--max-count=30', '--no-color', '--pretty=raw', '--merges', {chdir: nil}] + assert_command_line_eq(expected_command_line) { |git| git.log.merges.execute } + end + + def test_execute_returns_immutable_results + log_query = @git.log(10) + initial_results = log_query.execute + assert_equal(10, initial_results.size) + + # Modify the original query object + log_query.max_count(5) + new_results = log_query.execute + + # The initial result set should not have changed + assert_equal(10, initial_results.size) + + # The new result set should reflect the change + assert_equal(5, new_results.size) + end +end From e22eb10bf2e4049f1a0fb325341ef7489f25e66e Mon Sep 17 00:00:00 2001 From: James Couball Date: Wed, 2 Jul 2025 16:20:06 -0700 Subject: [PATCH 5/6] feat(diff): refactor Git::Diff to separate concerns and improve AP This pull request refactors the Git::Diff decomposing it into new, more focused classes, while backward compatibility is maintained via a deprecated facade. --- lib/git/base.rb | 21 ++++ lib/git/diff.rb | 159 +++++++++++++-------------- lib/git/diff_path_status.rb | 45 ++++++++ lib/git/diff_stats.rb | 59 ++++++++++ lib/git/lib.rb | 2 +- tests/units/test_diff.rb | 2 +- tests/units/test_diff_path_status.rb | 42 +++++++ tests/units/test_diff_stats.rb | 52 +++++++++ 8 files changed, 300 insertions(+), 82 deletions(-) create mode 100644 lib/git/diff_path_status.rb create mode 100644 lib/git/diff_stats.rb create mode 100644 tests/units/test_diff_path_status.rb create mode 100644 tests/units/test_diff_stats.rb diff --git a/lib/git/base.rb b/lib/git/base.rb index 3f01530e..d14a557e 100644 --- a/lib/git/base.rb +++ b/lib/git/base.rb @@ -782,6 +782,27 @@ def merge_base(*args) shas.map { |sha| gcommit(sha) } end +# Returns a Git::Diff::Stats object for accessing diff statistics. + # + # @param objectish [String] The first commit or object to compare. Defaults to 'HEAD'. + # @param obj2 [String, nil] The second commit or object to compare. + # @return [Git::Diff::Stats] + def diff_stats(objectish = 'HEAD', obj2 = nil) + Git::DiffStats.new(self, objectish, obj2) + end + + # Returns a Git::Diff::PathStatus object for accessing the name-status report. + # + # @param objectish [String] The first commit or object to compare. Defaults to 'HEAD'. + # @param obj2 [String, nil] The second commit or object to compare. + # @return [Git::Diff::PathStatus] + def diff_path_status(objectish = 'HEAD', obj2 = nil) + Git::DiffPathStatus.new(self, objectish, obj2) + end + + # Provided for backwards compatibility + alias diff_name_status diff_path_status + private # Normalize options before they are sent to Git::Base.new diff --git a/lib/git/diff.rb b/lib/git/diff.rb index d17d3c08..1aaeb1e3 100644 --- a/lib/git/diff.rb +++ b/lib/git/diff.rb @@ -1,8 +1,10 @@ # frozen_string_literal: true -module Git +require_relative 'diff_path_status' +require_relative 'diff_stats' - # object that holds the last X commits on given branch +module Git + # object that holds the diff between two commits class Diff include Enumerable @@ -12,63 +14,68 @@ def initialize(base, from = nil, to = nil) @to = to && to.to_s @path = nil - @full_diff = nil @full_diff_files = nil - @stats = nil end attr_reader :from, :to - def name_status - cache_name_status - end - def path(path) @path = path self end - def size - cache_stats - @stats[:total][:files] + def patch + @base.lib.diff_full(@from, @to, { path_limiter: @path }) end + alias_method :to_s, :patch - def lines - cache_stats - @stats[:total][:lines] + def [](key) + process_full + @full_diff_files.assoc(key)[1] end - def deletions - cache_stats - @stats[:total][:deletions] + def each(&block) + process_full + @full_diff_files.map { |file| file[1] }.each(&block) end - def insertions - cache_stats - @stats[:total][:insertions] + # + # DEPRECATED METHODS + # + + def name_status + Git::Deprecation.warn("Git::Diff#name_status is deprecated. Use Git::Base#diff_path_status instead.") + path_status_provider.to_h end - def stats - cache_stats - @stats + def size + Git::Deprecation.warn("Git::Diff#size is deprecated. Use Git::Base#diff_stats(...).total[:files] instead.") + stats_provider.total[:files] end - # if file is provided and is writable, it will write the patch into the file - def patch(file = nil) - cache_full - @full_diff + + + def lines + Git::Deprecation.warn("Git::Diff#lines is deprecated. Use Git::Base#diff_stats(...).lines instead.") + stats_provider.lines end - alias_method :to_s, :patch - # enumerable methods + def deletions + Git::Deprecation.warn("Git::Diff#deletions is deprecated. Use Git::Base#diff_stats(...).deletions instead.") + stats_provider.deletions + end - def [](key) - process_full - @full_diff_files.assoc(key)[1] + def insertions + Git::Deprecation.warn("Git::Diff#insertions is deprecated. Use Git::Base#diff_stats(...).insertions instead.") + stats_provider.insertions end - def each(&block) # :yields: each Git::DiffFile in turn - process_full - @full_diff_files.map { |file| file[1] }.each(&block) + def stats + Git::Deprecation.warn("Git::Diff#stats is deprecated. Use Git::Base#diff_stats instead.") + # CORRECTED: Re-create the original hash structure for backward compatibility + { + files: stats_provider.files, + total: stats_provider.total + } end class DiffFile @@ -102,56 +109,48 @@ def blob(type = :dst) private - def cache_full - @full_diff ||= @base.lib.diff_full(@from, @to, {:path_limiter => @path}) - end - - def process_full - return if @full_diff_files - cache_full - @full_diff_files = process_full_diff - end + def process_full + return if @full_diff_files + @full_diff_files = process_full_diff + end - def cache_stats - @stats ||= @base.lib.diff_stats(@from, @to, {:path_limiter => @path}) - end + # CORRECTED: Pass the @path variable to the new objects + def path_status_provider + @path_status_provider ||= Git::DiffPathStatus.new(@base, @from, @to, @path) + end - def cache_name_status - @name_status ||= @base.lib.diff_name_status(@from, @to, {:path => @path}) - end + # CORRECTED: Pass the @path variable to the new objects + def stats_provider + @stats_provider ||= Git::DiffStats.new(@base, @from, @to, @path) + end - # break up @diff_full - def process_full_diff - defaults = { - :mode => '', - :src => '', - :dst => '', - :type => 'modified' - } - final = {} - current_file = nil - @full_diff.split("\n").each do |line| - if m = %r{\Adiff --git ("?)a/(.+?)\1 ("?)b/(.+?)\3\z}.match(line) - current_file = Git::EscapedPath.new(m[2]).unescape - final[current_file] = defaults.merge({:patch => line, :path => current_file}) - else - if m = /^index ([0-9a-f]{4,40})\.\.([0-9a-f]{4,40})( ......)*/.match(line) - final[current_file][:src] = m[1] - final[current_file][:dst] = m[2] - final[current_file][:mode] = m[3].strip if m[3] - end - if m = /^([[:alpha:]]*?) file mode (......)/.match(line) - final[current_file][:type] = m[1] - final[current_file][:mode] = m[2] - end - if m = /^Binary files /.match(line) - final[current_file][:binary] = true - end - final[current_file][:patch] << "\n" + line + def process_full_diff + defaults = { + mode: '', src: '', dst: '', type: 'modified' + } + final = {} + current_file = nil + patch.split("\n").each do |line| + if m = %r{\Adiff --git ("?)a/(.+?)\1 ("?)b/(.+?)\3\z}.match(line) + current_file = Git::EscapedPath.new(m[2]).unescape + final[current_file] = defaults.merge({ patch: line, path: current_file }) + else + if m = /^index ([0-9a-f]{4,40})\.\.([0-9a-f]{4,40})( ......)*/.match(line) + final[current_file][:src] = m[1] + final[current_file][:dst] = m[2] + final[current_file][:mode] = m[3].strip if m[3] + end + if m = /^([[:alpha:]]*?) file mode (......)/.match(line) + final[current_file][:type] = m[1] + final[current_file][:mode] = m[2] + end + if m = /^Binary files /.match(line) + final[current_file][:binary] = true end + final[current_file][:patch] << "\n" + line end - final.map { |e| [e[0], DiffFile.new(@base, e[1])] } end - + final.map { |e| [e[0], DiffFile.new(@base, e[1])] } + end end end diff --git a/lib/git/diff_path_status.rb b/lib/git/diff_path_status.rb new file mode 100644 index 00000000..8ee4c8a2 --- /dev/null +++ b/lib/git/diff_path_status.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module Git + class DiffPathStatus + include Enumerable + + # @private + def initialize(base, from, to, path_limiter = nil) + # Eagerly check for invalid arguments + [from, to].compact.each do |arg| + raise ArgumentError, "Invalid argument: '#{arg}'" if arg.start_with?('-') + end + + @base = base + @from = from + @to = to + @path_limiter = path_limiter + @path_status = nil + end + + # Iterates over each file's status. + # + # @yield [path, status] + def each(&block) + fetch_path_status.each(&block) + end + + # Returns the name-status report as a Hash. + # + # @return [Hash] A hash where keys are file paths + # and values are their status codes. + def to_h + fetch_path_status + end + + private + + # Lazily fetches and caches the path status from the git lib. + def fetch_path_status + @path_status ||= @base.lib.diff_path_status( + @from, @to, { path: @path_limiter } + ) + end + end +end diff --git a/lib/git/diff_stats.rb b/lib/git/diff_stats.rb new file mode 100644 index 00000000..0a3826be --- /dev/null +++ b/lib/git/diff_stats.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +module Git + # Provides access to the statistics of a diff between two commits, + # including insertions, deletions, and file-level details. + class DiffStats + # @private + def initialize(base, from, to, path_limiter = nil) + # Eagerly check for invalid arguments + [from, to].compact.each do |arg| + raise ArgumentError, "Invalid argument: '#{arg}'" if arg.start_with?('-') + end + + @base = base + @from = from + @to = to + @path_limiter = path_limiter + @stats = nil + end + + # Returns the total number of lines deleted. + def deletions + fetch_stats[:total][:deletions] + end + + # Returns the total number of lines inserted. + def insertions + fetch_stats[:total][:insertions] + end + + # Returns the total number of lines changed (insertions + deletions). + def lines + fetch_stats[:total][:lines] + end + + # Returns a hash of statistics for each file in the diff. + # + # @return [Hash] + def files + fetch_stats[:files] + end + + # Returns a hash of the total statistics for the diff. + # + # @return [{insertions: Integer, deletions: Integer, lines: Integer, files: Integer}] + def total + fetch_stats[:total] + end + + private + + # Lazily fetches and caches the stats from the git lib. + def fetch_stats + @stats ||= @base.lib.diff_stats( + @from, @to, { path_limiter: @path_limiter } + ) + end + end +end diff --git a/lib/git/lib.rb b/lib/git/lib.rb index 203667d0..6695af3e 100644 --- a/lib/git/lib.rb +++ b/lib/git/lib.rb @@ -848,7 +848,7 @@ def diff_stats(obj1 = 'HEAD', obj2 = nil, opts = {}) hsh end - def diff_name_status(reference1 = nil, reference2 = nil, opts = {}) + def diff_path_status(reference1 = nil, reference2 = nil, opts = {}) assert_args_are_not_options('commit or commit range', reference1, reference2) opts_arr = ['--name-status'] diff --git a/tests/units/test_diff.rb b/tests/units/test_diff.rb index 3e859da5..95a7fa70 100644 --- a/tests/units/test_diff.rb +++ b/tests/units/test_diff.rb @@ -128,7 +128,7 @@ def test_diff_patch_with_bad_commit end end - def test_diff_name_status_with_bad_commit + def test_diff_path_status_with_bad_commit assert_raise(ArgumentError) do @git.diff('-s').name_status end diff --git a/tests/units/test_diff_path_status.rb b/tests/units/test_diff_path_status.rb new file mode 100644 index 00000000..b145acc4 --- /dev/null +++ b/tests/units/test_diff_path_status.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'test_helper' + +class TestDiffPathStatus < Test::Unit::TestCase + def setup + clone_working_repo + @git = Git.open(@wdir) + end + + def test_path_status + path_status = @git.diff_name_status('gitsearch1', 'v2.5') + status_hash = path_status.to_h + + assert_equal(3, status_hash.size) + assert_equal('M', status_hash['example.txt']) + assert_equal('D', status_hash['scott/newfile']) + # CORRECTED: The test repository state shows this file is Deleted, not Added. + assert_equal('D', status_hash['scott/text.txt']) + end + + def test_path_status_with_path_limiter + # Test the class in isolation by instantiating it directly with a path_limiter + path_status = Git::DiffPathStatus.new(@git, 'gitsearch1', 'v2.5', 'scott/') + status_hash = path_status.to_h + + assert_equal(2, status_hash.size) + assert_equal('D', status_hash['scott/newfile']) + assert_equal('D', status_hash['scott/text.txt']) + assert(!status_hash.key?('example.txt')) + end + + def test_path_status_with_bad_commit + assert_raise(ArgumentError) do + @git.diff_name_status('-s') + end + + assert_raise(ArgumentError) do + @git.diff_name_status('gitsearch1', '-s') + end + end +end diff --git a/tests/units/test_diff_stats.rb b/tests/units/test_diff_stats.rb new file mode 100644 index 00000000..608de015 --- /dev/null +++ b/tests/units/test_diff_stats.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'test_helper' + +class TestDiffStats < Test::Unit::TestCase + def setup + clone_working_repo + @git = Git.open(@wdir) + end + + def test_total_stats + stats = @git.diff_stats('gitsearch1', 'v2.5') + + assert_equal(3, stats.total[:files]) + assert_equal(74, stats.total[:lines]) + assert_equal(10, stats.total[:deletions]) + assert_equal(64, stats.total[:insertions]) + end + + def test_file_stats + stats = @git.diff_stats('gitsearch1', 'v2.5') + assert_equal(1, stats.files["scott/newfile"][:deletions]) + # CORRECTED: A deleted file should have 0 insertions. + assert_equal(0, stats.files["scott/newfile"][:insertions]) + end + + def test_diff_stats_with_path + stats = Git::DiffStats.new(@git, 'gitsearch1', 'v2.5', 'scott/') + + assert_equal(2, stats.total[:files]) + assert_equal(9, stats.total[:lines]) + assert_equal(9, stats.total[:deletions]) + assert_equal(0, stats.total[:insertions]) + end + + def test_diff_stats_on_object + stats = @git.diff_stats('v2.5', 'gitsearch1') + assert_equal(10, stats.insertions) + assert_equal(64, stats.deletions) + end + + def test_diff_stats_with_bad_commit + # CORRECTED: No longer need to call a method, error is raised on initialize. + assert_raise(ArgumentError) do + @git.diff_stats('-s') + end + + assert_raise(ArgumentError) do + @git.diff_stats('gitsearch1', '-s') + end + end +end From ee789fb474bbe8c844c02b93b746b5b8e8c70feb Mon Sep 17 00:00:00 2001 From: James Couball Date: Wed, 2 Jul 2025 16:27:44 -0700 Subject: [PATCH 6/6] chore: release v4.0.0 --- .release-please-manifest.json | 2 +- CHANGELOG.md | 19 +++++++++++++++++++ lib/git/version.rb | 2 +- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/.release-please-manifest.json b/.release-please-manifest.json index b4b8d0ff..e6f87756 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,3 +1,3 @@ { - ".": "3.1.1" + ".": "4.0.0" } diff --git a/CHANGELOG.md b/CHANGELOG.md index feedb6bd..0449fc36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,25 @@ # Change Log +## [4.0.0](https://github.com/ruby-git/ruby-git/compare/v3.1.1...v4.0.0) (2025-07-02) + + +### ⚠ BREAKING CHANGES + +* Users will need to be on Ruby 3.2 or greater + +### Features + +* Add Log#execute to run the log and return an immutable result ([ded54c4](https://github.com/ruby-git/ruby-git/commit/ded54c4b551aefb7de35b9505ce14f2061d1708c)) +* **diff:** Refactor Git::Diff to separate concerns and improve AP ([e22eb10](https://github.com/ruby-git/ruby-git/commit/e22eb10bf2e4049f1a0fb325341ef7489f25e66e)) +* Upgrade minimally supported Ruby to 3.2 ([fb93ef1](https://github.com/ruby-git/ruby-git/commit/fb93ef14def222d6eca29f49a5f810a3d6de5787)) + + +### Other Changes + +* Remove unneeded explicit return statements ([28e07ae](https://github.com/ruby-git/ruby-git/commit/28e07ae2e91a8defd52549393bf6f3fcbede122e)) +* Upgrade to ProcessExecuter 4.x ([5b00d3b](https://github.com/ruby-git/ruby-git/commit/5b00d3b9c4063c9988d844eec9ddedddb8c26446)) + ## [3.1.1](https://github.com/ruby-git/ruby-git/compare/v3.1.0...v3.1.1) (2025-07-02) diff --git a/lib/git/version.rb b/lib/git/version.rb index a6a12505..29e6a753 100644 --- a/lib/git/version.rb +++ b/lib/git/version.rb @@ -3,5 +3,5 @@ module Git # The current gem version # @return [String] the current gem version. - VERSION='3.1.1' + VERSION='4.0.0' end