From c11e5e48e840c1b1f7d1c1f6c9aa5a348f0dab47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Mon, 13 Jul 2026 21:13:12 +0200 Subject: [PATCH 1/4] feat!: start blame from Start instead of ObjectId --- gix-blame/src/file/function.rs | 202 ++++++++++++++++---- gix-blame/src/lib.rs | 2 +- gix-blame/src/types.rs | 25 +++ gix-blame/tests/blame.rs | 139 +++++++++++++- gix-blame/tests/fixtures/make_blame_repo.sh | 12 ++ 5 files changed, 336 insertions(+), 44 deletions(-) diff --git a/gix-blame/src/file/function.rs b/gix-blame/src/file/function.rs index 849919ae611..1b6cc198ccc 100644 --- a/gix-blame/src/file/function.rs +++ b/gix-blame/src/file/function.rs @@ -10,7 +10,10 @@ use gix_traverse::commit::find as find_commit; use smallvec::SmallVec; use super::{Change, UnblamedHunk, process_changes}; -use crate::{BlameEntry, Error, Options, Outcome, Statistics, types::BlamePathEntry}; +use crate::{ + BlameEntry, Error, Options, Outcome, Statistics, + types::{BlamePathEntry, Start}, +}; /// Produce a list of consecutive [`BlameEntry`] instances to indicate in which commits the ranges of the file /// at `suspect:` originated in. @@ -20,8 +23,9 @@ use crate::{BlameEntry, Error, Options, Outcome, Statistics, types::BlamePathEnt /// * `odb` /// - Access to database objects, also for used for diffing. /// - Should have an object cache for good diff performance. -/// * `suspect` -/// - The first commit to be responsible for parts of `file_path`. +/// * `start` +/// - Where to start the blame. Can be either a commit or a file in a worktree that contains +/// untracked changes. /// * `cache` /// - Optionally, the commitgraph cache. /// * `resource_cache` @@ -62,50 +66,51 @@ use crate::{BlameEntry, Error, Options, Outcome, Statistics, types::BlamePathEnt /// <---><---><-----><-------><-----><-><-><-> pub fn file( odb: impl gix_object::Find + gix_object::FindHeader, - suspect: ObjectId, + start: Start, cache: Option, resource_cache: &mut gix_diff::blob::Platform, file_path: &BStr, options: Options, ) -> Result { - let _span = gix_trace::coarse!("gix_blame::file()", ?file_path, ?suspect); + let _span = gix_trace::coarse!("gix_blame::file()", ?file_path, ?start); let mut stats = Statistics::default(); let (mut buf, mut buf2, mut buf3) = (Vec::new(), Vec::new(), Vec::new()); - let blamed_file_entry_id = find_path_entry_in_commit( + + let InitialState { + blamed_file_blob, + mut hunks_to_blame, + mut out, + first_suspect, + } = initial_state( &odb, - &suspect, - file_path, + start, cache.as_ref(), + file_path, + &options, &mut buf, &mut buf2, &mut stats, - )? - .ok_or_else(|| Error::FileMissing { - file_path: file_path.to_owned(), - commit_id: suspect, - })?; - let blamed_file_blob = odb.find_blob(&blamed_file_entry_id, &mut buf)?.data.to_vec(); - let num_lines_in_blamed = tokens_for_diffing(&blamed_file_blob).tokenize().count() as u32; - - // Binary or otherwise empty? - if num_lines_in_blamed == 0 { - return Ok(Outcome::default()); - } + )?; - let ranges_to_blame = options.ranges.to_zero_based_exclusive_ranges(num_lines_in_blamed); - let mut hunks_to_blame = ranges_to_blame - .into_iter() - .map(|range| UnblamedHunk::new(range, suspect)) - .collect::>(); + if hunks_to_blame.is_empty() { + out.sort_by_key(|entry| entry.start_in_blamed_file); + return Ok(Outcome { + entries: coalesce_blame_entries(out), + blob: blamed_file_blob, + statistics: stats, + blame_path: None, + }); + } - let (mut buf, mut buf2) = (Vec::new(), Vec::new()); - let commit = find_commit(cache.as_ref(), &odb, &suspect, &mut buf)?; let mut queue: gix_revwalk::PriorityQueue = gix_revwalk::PriorityQueue::new(); - queue.insert(commit.commit_time()?, suspect); - let mut out = Vec::new(); + if let Some(first_suspect) = first_suspect { + let commit = find_commit(cache.as_ref(), &odb, &first_suspect, &mut buf)?; + queue.insert(commit.commit_time()?, first_suspect); + } + let mut diff_state = gix_diff::tree::State::default(); let mut previous_entry: Option<(ObjectId, ObjectId)> = None; let mut blame_path = if options.debug_track_path { @@ -757,8 +762,6 @@ fn blob_changes( diff_algorithm: gix_diff::blob::Algorithm, stats: &mut Statistics, ) -> Result, Error> { - use gix_diff::blob::Hunk; - resource_cache.set_resource( previous_oid, gix_object::tree::EntryKind::Blob, @@ -775,10 +778,24 @@ fn blob_changes( )?; let outcome = resource_cache.prepare_diff()?; - let input = gix_diff::blob::InternedInput::new( + + Ok(blob_changes_from_data( outcome.old.data.as_slice().unwrap_or_default(), outcome.new.data.as_slice().unwrap_or_default(), - ); + diff_algorithm, + stats, + )) +} + +fn blob_changes_from_data( + old_data: &[u8], + new_data: &[u8], + diff_algorithm: gix_diff::blob::Algorithm, + stats: &mut Statistics, +) -> Vec { + use gix_diff::blob::Hunk; + + let input = gix_diff::blob::InternedInput::new(old_data, new_data); let mut diff = gix_diff::blob::Diff::compute(diff_algorithm, &input); diff.postprocess_lines(&input); @@ -816,7 +833,7 @@ fn blob_changes( } stats.blobs_diffed += 1; - Ok(changes) + changes } fn find_path_entry_in_commit( @@ -878,3 +895,120 @@ fn collect_parents( pub(crate) fn tokens_for_diffing(data: &[u8]) -> impl TokenSource { gix_diff::blob::sources::byte_lines(data) } + +struct InitialState { + blamed_file_blob: Vec, + hunks_to_blame: Vec, + out: Vec, + first_suspect: Option, +} + +#[allow(clippy::too_many_arguments)] +fn initial_state( + odb: &impl gix_object::Find, + start: Start<'_>, + cache: Option<&gix_commitgraph::Graph>, + file_path: &BStr, + options: &Options, + buf: &mut Vec, + buf2: &mut Vec, + stats: &mut Statistics, +) -> Result { + match start { + Start::Commit(suspect) => { + let blamed_file_entry_id = find_path_entry_in_commit(&odb, &suspect, file_path, cache, buf, buf2, stats)? + .ok_or_else(|| Error::FileMissing { + file_path: file_path.to_owned(), + commit_id: suspect, + })?; + let blamed_file_blob = odb.find_blob(&blamed_file_entry_id, buf)?.data.to_vec(); + let num_lines_in_blamed = tokens_for_diffing(&blamed_file_blob).tokenize().count() as u32; + + // Binary or otherwise empty? + if num_lines_in_blamed == 0 { + return Ok(InitialState { + blamed_file_blob, + hunks_to_blame: Vec::new(), + out: Vec::new(), + first_suspect: None, + }); + } + + let ranges_to_blame = options.ranges.to_zero_based_exclusive_ranges(num_lines_in_blamed); + let hunks_to_blame = ranges_to_blame + .into_iter() + .map(|range| UnblamedHunk::new(range, suspect)) + .collect::>(); + + Ok(InitialState { + blamed_file_blob, + hunks_to_blame, + out: Vec::new(), + first_suspect: Some(suspect), + }) + } + Start::Contents { + first_suspect, + contents, + } => { + let null_id = first_suspect.kind().null(); + let blamed_file_blob = contents.into_owned(); + let num_lines_in_blamed = tokens_for_diffing(&blamed_file_blob).tokenize().count() as u32; + + if num_lines_in_blamed == 0 { + return Ok(InitialState { + blamed_file_blob, + hunks_to_blame: Vec::new(), + out: Vec::new(), + first_suspect: None, + }); + } + + let ranges_to_blame = options.ranges.to_zero_based_exclusive_ranges(num_lines_in_blamed); + let mut hunks_to_blame = ranges_to_blame + .into_iter() + .map(|range| UnblamedHunk::new(range, null_id)) + .collect::>(); + + let mut out = Vec::new(); + + let Some(first_suspect_entry_id) = + find_path_entry_in_commit(odb, &first_suspect, file_path, cache, buf, buf2, stats)? + else { + // File does not exist in the starting commit. Treat the requested + // ranges as entirely "not committed yet". + out.extend( + hunks_to_blame + .iter() + .filter_map(|hunk| BlameEntry::from_unblamed_hunk(hunk, null_id)), + ); + + return Ok(InitialState { + blamed_file_blob, + hunks_to_blame: Vec::new(), + out, + first_suspect: None, + }); + }; + + let first_suspect_blob = odb.find_blob(&first_suspect_entry_id, buf)?.data.to_vec(); + + let changes = blob_changes_from_data(&first_suspect_blob, &blamed_file_blob, options.diff_algorithm, stats); + hunks_to_blame = process_changes(hunks_to_blame, changes, null_id, first_suspect); + + unblamed_to_out_is_done(&mut hunks_to_blame, &mut out, null_id); + + let first_suspect = hunks_to_blame + .iter() + .any(|hunk| hunk.has_suspect(&first_suspect)) + .then_some(first_suspect); + + Ok(InitialState { + blamed_file_blob, + hunks_to_blame, + out, + first_suspect, + }) + } + } +} diff --git a/gix-blame/src/lib.rs b/gix-blame/src/lib.rs index 0f7b35d2559..7495189e6fc 100644 --- a/gix-blame/src/lib.rs +++ b/gix-blame/src/lib.rs @@ -17,7 +17,7 @@ mod error; pub use error::Error; mod types; -pub use types::{BlameEntry, BlamePathEntry, BlameRanges, Options, Outcome, Statistics}; +pub use types::{BlameEntry, BlamePathEntry, BlameRanges, Options, Outcome, Start, Statistics}; mod file; pub use file::function::file; diff --git a/gix-blame/src/types.rs b/gix-blame/src/types.rs index 31d07e74f2d..09efb912191 100644 --- a/gix-blame/src/types.rs +++ b/gix-blame/src/types.rs @@ -190,6 +190,31 @@ pub struct BlamePathEntry { pub parent_index: usize, } +/// The starting point for [`file()`](crate::file()). +#[derive(Debug)] +pub enum Start<'a> { + /// Start from a specific commit. + Commit(ObjectId), + + /// Start from `contents`, then continue from `first_suspect`. + /// + /// Lines that only exist in `contents` are attributed to the null id, + /// i.e. "not committed yet". + /// + /// It is assumed that the data in `contents` is ready to be used for diffing, in particular + /// that it has been run through the configured worktree filters. + /// + /// See [Pipeline::convert_to_diffable()](gix_diff::blob::Pipeline::convert_to_diffable) for + /// how to obtain the contents of a worktree file by running them through the configured + /// worktree filters. + Contents { + /// The commit to start from after it has been compared to `contents`. + first_suspect: ObjectId, + /// The contents to start the blame from. + contents: std::borrow::Cow<'a, [u8]>, + }, +} + /// The outcome of [`file()`](crate::file()). #[derive(Debug, Default, Clone)] pub struct Outcome { diff --git a/gix-blame/tests/blame.rs b/gix-blame/tests/blame.rs index 68f4a6eff05..d19c209c03a 100644 --- a/gix-blame/tests/blame.rs +++ b/gix-blame/tests/blame.rs @@ -228,7 +228,26 @@ impl Fixture { ) -> Result { gix_blame::file( &self.odb, - self.suspect, + gix_blame::Start::Commit(self.suspect), + None, + &mut self.resource_cache, + source_file_name, + options, + ) + } + + fn blame_untracked_changes( + &mut self, + source_file_name: &bstr::BStr, + contents: Vec, + options: gix_blame::Options, + ) -> Result { + gix_blame::file( + &self.odb, + gix_blame::Start::Contents { + first_suspect: self.suspect, + contents: contents.into(), + }, None, &mut self.resource_cache, source_file_name, @@ -251,7 +270,7 @@ macro_rules! mktest { let lines_blamed = gix_blame::file( &odb, - suspect, + gix_blame::Start::Commit(suspect), None, &mut resource_cache, source_file_name.as_ref(), @@ -340,7 +359,7 @@ fn diff_algorithm_parity() { let lines_blamed = gix_blame::file( &odb, - suspect, + gix_blame::Start::Commit(suspect), None, &mut resource_cache, source_file_name.as_ref(), @@ -377,7 +396,7 @@ fn file_that_was_added_in_two_branches() -> gix_testtools::Result { let source_file_name = "file-with-two-roots.txt"; let lines_blamed = gix_blame::file( &odb, - suspect, + gix_blame::Start::Commit(suspect), None, &mut resource_cache, source_file_name.into(), @@ -407,7 +426,7 @@ fn since() -> gix_testtools::Result { let lines_blamed = gix_blame::file( &odb, - suspect, + gix_blame::Start::Commit(suspect), None, &mut resource_cache, source_file_name.as_ref(), @@ -449,7 +468,7 @@ mod blame_ranges { let lines_blamed = gix_blame::file( &odb, - suspect, + gix_blame::Start::Commit(suspect), None, &mut resource_cache, source_file_name.as_ref(), @@ -492,7 +511,7 @@ mod blame_ranges { let lines_blamed = gix_blame::file( &odb, - suspect, + gix_blame::Start::Commit(suspect), None, &mut resource_cache, source_file_name.as_ref(), @@ -533,7 +552,7 @@ mod blame_ranges { let lines_blamed = gix_blame::file( &odb, - suspect, + gix_blame::Start::Commit(suspect), None, &mut resource_cache, source_file_name.as_ref(), @@ -579,7 +598,7 @@ mod rename_tracking { let source_file_name = "after-rename.txt"; let lines_blamed = gix_blame::file( &odb, - suspect, + gix_blame::Start::Commit(suspect), None, &mut resource_cache, source_file_name.into(), @@ -634,6 +653,108 @@ mod rename_tracking { } } +mod untracked_changes { + use gix_blame::BlameRanges; + + use crate::{Baseline, Fixture}; + + #[test] + fn untracked_lines() -> gix_testtools::Result { + let worktree_path = gix_testtools::scripted_fixture_read_only("make_blame_repo.sh")?; + + let mut fixture = Fixture::for_worktree_path(worktree_path.to_path_buf())?; + let source_file_name = "untracked-lines.txt"; + let contents = std::fs::read(worktree_path.join(source_file_name)).expect("file to be present and readable"); + + let lines_blamed = fixture + .blame_untracked_changes( + source_file_name.into(), + contents, + gix_blame::Options { + diff_algorithm: gix_diff::blob::Algorithm::Histogram, + ranges: BlameRanges::default(), + since: None, + rewrites: Some(gix_diff::Rewrites::default()), + debug_track_path: false, + }, + )? + .entries; + + assert_eq!(lines_blamed.len(), 2); + + let git_dir = worktree_path.join(".git"); + let baseline = Baseline::collect(git_dir.join("untracked-lines.baseline"), source_file_name.into())?; + + pretty_assertions::assert_eq!(lines_blamed, baseline); + + Ok(()) + } + + #[test] + fn untracked_file() -> gix_testtools::Result { + let worktree_path = gix_testtools::scripted_fixture_read_only("make_blame_repo.sh")?; + + let mut fixture = Fixture::for_worktree_path(worktree_path.to_path_buf())?; + let source_file_name = "untracked-file.txt"; + let contents = std::fs::read(worktree_path.join(source_file_name)).expect("file to be present and readable"); + + let lines_blamed = fixture + .blame_untracked_changes( + source_file_name.into(), + contents, + gix_blame::Options { + diff_algorithm: gix_diff::blob::Algorithm::Histogram, + ranges: BlameRanges::default(), + since: None, + rewrites: Some(gix_diff::Rewrites::default()), + debug_track_path: false, + }, + )? + .entries; + + assert_eq!(lines_blamed.len(), 1); + + let git_dir = worktree_path.join(".git"); + let baseline = Baseline::collect(git_dir.join("untracked-file.baseline"), source_file_name.into())?; + + pretty_assertions::assert_eq!(lines_blamed, baseline); + + Ok(()) + } + + #[test] + fn untracked_lines_with_ranges() -> gix_testtools::Result { + let worktree_path = gix_testtools::scripted_fixture_read_only("make_blame_repo.sh")?; + + let mut fixture = Fixture::for_worktree_path(worktree_path.to_path_buf())?; + let source_file_name = "untracked-lines.txt"; + let contents = std::fs::read(worktree_path.join(source_file_name)).expect("file to be present and readable"); + + let lines_blamed = fixture + .blame_untracked_changes( + source_file_name.into(), + contents, + gix_blame::Options { + diff_algorithm: gix_diff::blob::Algorithm::Histogram, + ranges: BlameRanges::from_one_based_inclusive_range(3..=7).unwrap(), + since: None, + rewrites: Some(gix_diff::Rewrites::default()), + debug_track_path: false, + }, + )? + .entries; + + assert_eq!(lines_blamed.len(), 2); + + let git_dir = worktree_path.join(".git"); + let baseline = Baseline::collect(git_dir.join("untracked-lines-ranges.baseline"), source_file_name.into())?; + + pretty_assertions::assert_eq!(lines_blamed, baseline); + + Ok(()) + } +} + fn fixture_path() -> gix_testtools::Result { gix_testtools::scripted_fixture_read_only("make_blame_repo.sh") } diff --git a/gix-blame/tests/fixtures/make_blame_repo.sh b/gix-blame/tests/fixtures/make_blame_repo.sh index 5846f9326db..e521734aa22 100755 --- a/gix-blame/tests/fixtures/make_blame_repo.sh +++ b/gix-blame/tests/fixtures/make_blame_repo.sh @@ -259,6 +259,15 @@ git commit -q -m c15.2 git merge branch-that-has-earlier-commit || true +seq 1 4 > untracked-lines.txt +git add untracked-lines.txt +git commit -q -m c16 + +seq 5 7 >> untracked-lines.txt + +seq 1 4 >> untracked-file.txt +git add --intent-to-add untracked-file.txt + git blame --porcelain simple.txt > .git/simple.baseline git blame --porcelain -L 1,2 simple.txt > .git/simple-lines-1-2.baseline git blame --porcelain -L 1,2 -L 4 simple.txt > .git/simple-lines-multiple-1-2-and-4.baseline @@ -274,6 +283,9 @@ git blame --porcelain switched-lines.txt > .git/switched-lines.baseline git blame --porcelain added-line-before-changed-line.txt > .git/added-line-before-changed-line.baseline git blame --porcelain same-line-changed-twice.txt > .git/same-line-changed-twice.baseline git blame --porcelain coalesce-adjacent-hunks.txt > .git/coalesce-adjacent-hunks.baseline +git blame --porcelain untracked-lines.txt > .git/untracked-lines.baseline +git blame --porcelain -L 3,7 untracked-lines.txt > .git/untracked-lines-ranges.baseline +git blame --porcelain untracked-file.txt > .git/untracked-file.baseline mkdir .git/sub-directory git blame --porcelain sub-directory/sub-directory.txt > .git/sub-directory/sub-directory.baseline From 93d4019fdd97200bebcd7b2197da7db74e678877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Wed, 15 Jul 2026 20:44:01 +0200 Subject: [PATCH 2/4] Adapt to changes in `gix-blame` --- gitoxide-core/src/repository/blame.rs | 4 ++-- gix/src/repository/blame.rs | 3 ++- tests/it/src/commands/blame_copy_royal.rs | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/gitoxide-core/src/repository/blame.rs b/gitoxide-core/src/repository/blame.rs index 8c074315910..7566a25ee04 100644 --- a/gitoxide-core/src/repository/blame.rs +++ b/gitoxide-core/src/repository/blame.rs @@ -1,6 +1,6 @@ use std::ffi::OsStr; -use gix::{bstr::BStr, config::tree}; +use gix::{blame::Start, bstr::BStr, config::tree}; pub fn blame_file( mut repo: gix::Repository, @@ -26,7 +26,7 @@ pub fn blame_file( let mut resource_cache = repo.diff_resource_cache_for_tree_diff()?; let outcome = gix::blame::file( &repo.objects, - suspect, + Start::Commit(suspect), cache, &mut resource_cache, file.as_ref(), diff --git a/gix/src/repository/blame.rs b/gix/src/repository/blame.rs index 62d088d127a..4f3177f4fb0 100644 --- a/gix/src/repository/blame.rs +++ b/gix/src/repository/blame.rs @@ -1,3 +1,4 @@ +use gix_blame::Start; use gix_hash::ObjectId; use gix_ref::bstr::BStr; @@ -39,7 +40,7 @@ impl Repository { let outcome = gix_blame::file( &self.objects, - suspect.into(), + Start::Commit(suspect.into()), cache, &mut resource_cache, file_path, diff --git a/tests/it/src/commands/blame_copy_royal.rs b/tests/it/src/commands/blame_copy_royal.rs index 2fd519ca28d..8e3dae733fa 100644 --- a/tests/it/src/commands/blame_copy_royal.rs +++ b/tests/it/src/commands/blame_copy_royal.rs @@ -66,7 +66,7 @@ pub(super) mod function { let outcome = gix::blame::file( &repo.objects, - suspect, + gix::blame::Start::Commit(suspect), cache, &mut resource_cache, file.as_bstr(), From 86e90d8626c01613220ae88100eb3815b31f30ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Sun, 19 Jul 2026 19:53:24 +0200 Subject: [PATCH 3/4] Align line numbers in output and start `gix blame` from worktree contents --- gitoxide-core/src/repository/blame.rs | 49 ++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/gitoxide-core/src/repository/blame.rs b/gitoxide-core/src/repository/blame.rs index 7566a25ee04..bef4a5781ae 100644 --- a/gitoxide-core/src/repository/blame.rs +++ b/gitoxide-core/src/repository/blame.rs @@ -21,12 +21,11 @@ pub fn blame_file( let file = gix::path::os_str_into_bstr(file)?; let file = repo.normalize_path(file)?; - let suspect: gix::ObjectId = repo.head()?.into_peeled_id()?.into(); let cache: Option = repo.commit_graph_if_enabled()?; let mut resource_cache = repo.diff_resource_cache_for_tree_diff()?; let outcome = gix::blame::file( &repo.objects, - Start::Commit(suspect), + start_for_blame(&repo, file.as_bstr())?, cache, &mut resource_cache, file.as_ref(), @@ -41,11 +40,49 @@ pub fn blame_file( Ok(()) } +fn start_for_blame<'a>(repo: &'a gix::Repository, file: &'a gix::bstr::BStr) -> anyhow::Result> { + let worktree_roots = gix::diff::blob::pipeline::WorktreeRoots { + old_root: repo.workdir().map(ToOwned::to_owned), + new_root: None, + }; + let mut filter = gix::diff::blob::Pipeline::new(worktree_roots, Default::default(), vec![], Default::default()); + let mut buf = Vec::new(); + let outcome = filter.convert_to_diffable( + &repo.object_hash().null(), + gix::objs::tree::EntryKind::Blob, + file, + gix::diff::blob::ResourceKind::OldOrSource, + &mut |_, _| {}, + &repo.objects, + gix::diff::blob::pipeline::Mode::ToGitUnlessBinaryToTextIsPresent, + &mut buf, + )?; + + let first_suspect: gix::ObjectId = repo.head()?.into_peeled_id()?.into(); + + Ok(outcome + .data + .and_then(|data| match data { + gix::diff::blob::pipeline::Data::Buffer { .. } => Some(Start::Contents { + first_suspect, + contents: buf.into(), + }), + gix::diff::blob::pipeline::Data::Binary { .. } => None, + }) + .unwrap_or(Start::Commit(first_suspect))) +} + fn show_blame_entries( mut out: impl std::io::Write, outcome: gix::blame::Outcome, source_file_name: &BStr, ) -> Result<(), std::io::Error> { + let last_line_no = outcome + .entries + .last() + .map_or(0, |entry| entry.range_in_blamed_file().end); + let number_of_digits = (last_line_no.ilog10() + 1) as usize; + for (entry, lines_in_hunk) in outcome.entries_with_lines() { for ((actual_lno, source_lno), line) in entry .range_in_blamed_file() @@ -54,7 +91,7 @@ fn show_blame_entries( { write!( out, - "{short_id} {line_no} ", + "{short_id} {line_no:>number_of_digits$} ", short_id = entry.commit_id.to_hex_with_len(8), line_no = actual_lno + 1, )?; @@ -62,7 +99,11 @@ fn show_blame_entries( let source_file_name = entry.source_file_name.as_ref().map_or(source_file_name, BStr::new); write!(out, "{source_file_name} ")?; - write!(out, "{src_line_no} {line}", src_line_no = source_lno + 1)?; + write!( + out, + "{src_line_no:>number_of_digits$} {line}", + src_line_no = source_lno + 1 + )?; } } From 2ea191fffb63a2c21f5ed8c9f9f77cc91a763bcc Mon Sep 17 00:00:00 2001 From: Byron Date: Thu, 6 Aug 2026 09:12:58 +0200 Subject: [PATCH 4/4] review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Including some unrelated modifications to the contribution guidelines. - [P1] Avoid taking ilog10 of an empty outcome — /Users/byron/dev/github.com/GitoxideLabs/gitoxide.blame-untracked-changes/gitoxide-core/src/repository/blame.rs:87-87 When the blamed file is empty, or the requested ranges are wholly outside it, `entries.last()` is `None`, so this calls `0.ilog10()` and panics before producing output. For example, `gix blame empty.txt` exits with status 101 instead of returning no entries; special-case an empty outcome or use a checked logarithm. - [P1] Apply repository filters before blaming worktree bytes — /Users/byron/dev/github.com/GitoxideLabs/gitoxide.blame-untracked-changes/gitoxide-core/src/repository/blame.rs:50-50 When a worktree file requires repository-configured clean or EOL conversion, this default pipeline has neither the repository filter options nor diff drivers, and the empty attribute callback prevents path attributes from applying. For example, an unchanged file governed by `*.txt text eol=crlf` is read with CRLF and compared with the LF blob, causing every line to receive the null ID; construct the pipeline from the repository configuration and attribute stack. - [P2] Preserve symlink entry mode when reading the worktree — /Users/byron/dev/github.com/GitoxideLabs/gitoxide.blame-untracked-changes/gitoxide-core/src/repository/blame.rs:54-54 On systems supporting symlinks, hard-coding `EntryKind::Blob` makes the pipeline follow a worktree symlink and read the target file rather than reading the link target as blob data. An unchanged `link -> target` is therefore shown as the target file's lines attributed to the null ID instead of one committed line containing `target`; obtain the actual entry kind from the index or HEAD. - [P2] Convert the suspect blob with the same textconv driver — /Users/byron/dev/github.com/GitoxideLabs/gitoxide.blame-untracked-changes/gix-blame/src/file/function.rs:994-996 When `Start::Contents` contains binary-to-text-derived data from the documented `convert_to_diffable()` route, this compares that derived text against the raw blob from the object database. Even an unchanged file using a textconv driver is then reported as uncommitted; prepare the first suspect through the same diff pipeline or otherwise ensure both sides use the same representation. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 --- AGENTS.md | 6 ++ CONTRIBUTING.md | 4 +- gitoxide-core/src/repository/blame.rs | 105 ++++++++++++++++---------- gix-blame/src/file/function.rs | 8 +- gix-blame/src/types.rs | 21 +++++- 5 files changed, 100 insertions(+), 44 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6298539fd72..f62e24b6b75 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,6 +11,12 @@ This repository contains `gitoxide` - a pure Rust implementation of Git. This do ## Development Practices +### AI Agent Communication + +- AI agents communicating through a person's account must identify themselves, for example in issue or PR descriptions and comments. +- AI assistance that does not replace the person as the speaker, such as proofreading or wording polish, does not require identification. +- Attributing AI assistance in commit metadata, for example with an `Assisted-by:` or `Co-authored-by:` trailer, is welcome but not required. + ### Test-First Development - Protect against regression and make implementing features easy diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 311c8abd275..3d2b102eec0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,8 +4,8 @@ AI agents communicating through a person's account must identify themselves, for PR descriptions and comments. AI assistance that does not replace the person as the speaker, such as proofreading or wording polish, does not require identification. -Attributing AI assistance in commit metadata, for example with a `Co-authored-by` trailer, is welcome but not -required. Code is reviewed the same way regardless of its origin. +Attributing AI assistance in commit metadata, for example with an `Assisted-by:` or `Co-authored-by:` trailer, +is welcome but not required. For everything else, please have a look at the respective section in the [README] file. diff --git a/gitoxide-core/src/repository/blame.rs b/gitoxide-core/src/repository/blame.rs index bef4a5781ae..4cac5c588f3 100644 --- a/gitoxide-core/src/repository/blame.rs +++ b/gitoxide-core/src/repository/blame.rs @@ -1,6 +1,7 @@ use std::ffi::OsStr; -use gix::{blame::Start, bstr::BStr, config::tree}; +use anyhow::Context; +use gix::{blame::Start, bstr::BStr, config::tree, utils::AsBStr}; pub fn blame_file( mut repo: gix::Repository, @@ -22,15 +23,22 @@ pub fn blame_file( let file = repo.normalize_path(file)?; let cache: Option = repo.commit_graph_if_enabled()?; - let mut resource_cache = repo.diff_resource_cache_for_tree_diff()?; - let outcome = gix::blame::file( - &repo.objects, - start_for_blame(&repo, file.as_bstr())?, - cache, - &mut resource_cache, - file.as_ref(), - options, + let mut resource_cache = repo.diff_resource_cache( + // TODO(blame): Git uses something akin to `ToGitUnlessBinaryToTextIsPresent` here, but with a specialty: textconv output is converted + // to Git, which isn't happening for normal diffing. In theory, this shouldn't be a problem as it's always apples to apples, + // at least in theory. + gix::diff::blob::pipeline::Mode::ToGit, + gix::diff::blob::pipeline::WorktreeRoots { + old_root: repo.workdir().map(ToOwned::to_owned), + new_root: None, + }, )?; + let start = start_for_blame(&repo, file.as_bstr(), &mut resource_cache)?; + // The worktree root is only for constructing `start`; historical `OldOrSource` resources must be loaded by + // object ID, and we make sure that worktree contents can't possibly be used. + resource_cache.filter.roots = Default::default(); + resource_cache.clear_resource_cache_keep_allocation(); + let outcome = gix::blame::file(&repo.objects, start, cache, &mut resource_cache, file.as_ref(), options)?; let statistics = outcome.statistics; show_blame_entries(out, outcome, file.as_ref())?; @@ -40,34 +48,53 @@ pub fn blame_file( Ok(()) } -fn start_for_blame<'a>(repo: &'a gix::Repository, file: &'a gix::bstr::BStr) -> anyhow::Result> { - let worktree_roots = gix::diff::blob::pipeline::WorktreeRoots { - old_root: repo.workdir().map(ToOwned::to_owned), - new_root: None, +/// Start at `HEAD`, overlaying diffable worktree contents so uncommitted changes are included. +/// Missing or binary worktree files fall back to blaming `HEAD` directly. +fn start_for_blame<'a>( + repo: &'a gix::Repository, + file: &'a gix::bstr::BStr, + resources: &mut gix::diff::blob::Platform, +) -> anyhow::Result> { + let first_suspect: gix::ObjectId = repo.head()?.into_peeled_id()?.into(); + let Some(workdir) = repo.workdir() else { + return Ok(Start::Commit(first_suspect)); + }; + let path = workdir.join(gix::path::from_bstr(file)); + let metadata = match std::fs::symlink_metadata(&path) { + Ok(metadata) => metadata, + Err(err) if gix::fs::io_err::is_not_found(err.kind(), err.raw_os_error()) => { + return Ok(Start::Commit(first_suspect)); + } + Err(err) => return Err(err).with_context(|| format!("Could not read metadata of '{}'", path.display())), }; - let mut filter = gix::diff::blob::Pipeline::new(worktree_roots, Default::default(), vec![], Default::default()); - let mut buf = Vec::new(); - let outcome = filter.convert_to_diffable( - &repo.object_hash().null(), - gix::objs::tree::EntryKind::Blob, + // State the correct type here so that the resource cache and its possibly converted bytes match the actual type, i.e. + // - read the file for blobs + // - read the symlink bytes themselves, the target path for symlinks. + let entry_kind = if metadata.file_type().is_symlink() { + gix::objs::tree::EntryKind::Link + } else { + // executable bits don't matter. + gix::objs::tree::EntryKind::Blob + }; + resources.set_resource( + repo.object_hash().null(), + entry_kind, file, gix::diff::blob::ResourceKind::OldOrSource, - &mut |_, _| {}, &repo.objects, - gix::diff::blob::pipeline::Mode::ToGitUnlessBinaryToTextIsPresent, - &mut buf, )?; + let contents = resources + .resource(gix::diff::blob::ResourceKind::OldOrSource) + .and_then(|resource| match resource.data { + gix::diff::blob::platform::resource::Data::Buffer { buf, .. } => Some(buf.to_owned()), + gix::diff::blob::platform::resource::Data::Binary { .. } + | gix::diff::blob::platform::resource::Data::Missing => None, + }); - let first_suspect: gix::ObjectId = repo.head()?.into_peeled_id()?.into(); - - Ok(outcome - .data - .and_then(|data| match data { - gix::diff::blob::pipeline::Data::Buffer { .. } => Some(Start::Contents { - first_suspect, - contents: buf.into(), - }), - gix::diff::blob::pipeline::Data::Binary { .. } => None, + Ok(contents + .map(|contents| Start::Contents { + first_suspect, + contents: contents.into(), }) .unwrap_or(Start::Commit(first_suspect))) } @@ -77,11 +104,13 @@ fn show_blame_entries( outcome: gix::blame::Outcome, source_file_name: &BStr, ) -> Result<(), std::io::Error> { - let last_line_no = outcome - .entries - .last() - .map_or(0, |entry| entry.range_in_blamed_file().end); - let number_of_digits = (last_line_no.ilog10() + 1) as usize; + let num_digits_for_line_number = { + let largest_line_number = outcome + .entries + .last() + .map_or(0, |entry| entry.range_in_blamed_file().end); + (largest_line_number.checked_ilog10().unwrap_or(0) + 1) as usize + }; for (entry, lines_in_hunk) in outcome.entries_with_lines() { for ((actual_lno, source_lno), line) in entry @@ -91,7 +120,7 @@ fn show_blame_entries( { write!( out, - "{short_id} {line_no:>number_of_digits$} ", + "{short_id} {line_no:>num_digits_for_line_number$} ", short_id = entry.commit_id.to_hex_with_len(8), line_no = actual_lno + 1, )?; @@ -101,7 +130,7 @@ fn show_blame_entries( write!( out, - "{src_line_no:>number_of_digits$} {line}", + "{src_line_no:>num_digits_for_line_number$} {line}", src_line_no = source_lno + 1 )?; } diff --git a/gix-blame/src/file/function.rs b/gix-blame/src/file/function.rs index 1b6cc198ccc..ef6acb4c6a1 100644 --- a/gix-blame/src/file/function.rs +++ b/gix-blame/src/file/function.rs @@ -24,7 +24,7 @@ use crate::{ /// - Access to database objects, also for used for diffing. /// - Should have an object cache for good diff performance. /// * `start` -/// - Where to start the blame. Can be either a commit or a file in a worktree that contains +/// - Where to start the blame. Can be either a commit or contents of a worktree file that contains /// untracked changes. /// * `cache` /// - Optionally, the commitgraph cache. @@ -764,6 +764,7 @@ fn blob_changes( ) -> Result, Error> { resource_cache.set_resource( previous_oid, + // TODO(blame): add a test to show of symlink blaming works. gix_object::tree::EntryKind::Blob, previous_file_path, gix_diff::blob::ResourceKind::OldOrSource, @@ -896,10 +897,15 @@ pub(crate) fn tokens_for_diffing(data: &[u8]) -> impl TokenSource gix_diff::blob::sources::byte_lines(data) } +/// The blame input after resolving [`Start`] and before traversing commit history. struct InitialState { + /// The final file contents whose lines are being attributed. blamed_file_blob: Vec, + /// Requested ranges that still need attribution through commit traversal. hunks_to_blame: Vec, + /// Entries already resolved while comparing worktree contents to the first suspect. out: Vec, + /// The first commit to traverse, or `None` if no history traversal is needed. first_suspect: Option, } diff --git a/gix-blame/src/types.rs b/gix-blame/src/types.rs index 09efb912191..c82cf6d103c 100644 --- a/gix-blame/src/types.rs +++ b/gix-blame/src/types.rs @@ -191,11 +191,9 @@ pub struct BlamePathEntry { } /// The starting point for [`file()`](crate::file()). -#[derive(Debug)] pub enum Start<'a> { /// Start from a specific commit. Commit(ObjectId), - /// Start from `contents`, then continue from `first_suspect`. /// /// Lines that only exist in `contents` are attributed to the null id, @@ -210,11 +208,28 @@ pub enum Start<'a> { Contents { /// The commit to start from after it has been compared to `contents`. first_suspect: ObjectId, - /// The contents to start the blame from. + /// The contents to start the blame from, typically read from the worktree. + // TODO(blame): add a type so rename tracking can avoid comparing blobs with symlinks. + // Blob is hard-coded in at least once place. contents: std::borrow::Cow<'a, [u8]>, }, } +impl std::fmt::Debug for Start<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Start::Commit(id) => f.debug_tuple("Commit").field(id).finish(), + Start::Contents { + first_suspect, + contents, + } => f + .debug_struct("Contents") + .field("first_suspect", first_suspect) + .field("contents_len", &contents.len()) + .finish(), + } + } +} /// The outcome of [`file()`](crate::file()). #[derive(Debug, Default, Clone)] pub struct Outcome {