Merge pull request #2041 from cruessler/add-blame-extraction

Add `it blame-copy-royal`

Author: Byron

gix-attributes/tests/parse/mod.rs

@@ -322,15 +322,15 @@ fn trailing_whitespace_in_attributes_is_ignored() {
 
 type ExpandedAttribute<'a> = (parse::Kind, Vec<(BString, gix_attributes::StateRef<'a>)>, usize);
 
-fn set(attr: &str) -> (BString, StateRef) {
+fn set(attr: &str) -> (BString, StateRef<'_>) {
     (attr.into(), StateRef::Set)
 }
 
-fn unset(attr: &str) -> (BString, StateRef) {
+fn unset(attr: &str) -> (BString, StateRef<'_>) {
     (attr.into(), StateRef::Unset)
 }
 
-fn unspecified(attr: &str) -> (BString, StateRef) {
+fn unspecified(attr: &str) -> (BString, StateRef<'_>) {
     (attr.into(), StateRef::Unspecified)
 }
 
@@ -350,36 +350,36 @@ fn pattern(name: &str, flags: gix_glob::pattern::Mode, first_wildcard_pos: Optio
     })
 }
 
-fn try_line(input: &str) -> Result<ExpandedAttribute, parse::Error> {
+fn try_line(input: &str) -> Result<ExpandedAttribute<'_>, parse::Error> {
     let mut lines = gix_attributes::parse(input.as_bytes());
     let res = expand(lines.next().unwrap())?;
     assert!(lines.next().is_none(), "expected only one line");
     Ok(res)
 }
 
-fn line(input: &str) -> ExpandedAttribute {
+fn line(input: &str) -> ExpandedAttribute<'_> {
     try_line(input).unwrap()
 }
 
-fn byte_line(input: &[u8]) -> ExpandedAttribute {
+fn byte_line(input: &[u8]) -> ExpandedAttribute<'_> {
     try_byte_line(input).unwrap()
 }
 
-fn try_byte_line(input: &[u8]) -> Result<ExpandedAttribute, parse::Error> {
+fn try_byte_line(input: &[u8]) -> Result<ExpandedAttribute<'_>, parse::Error> {
     let mut lines = gix_attributes::parse(input);
     let res = expand(lines.next().unwrap())?;
     assert!(lines.next().is_none(), "expected only one line");
     Ok(res)
 }
 
-fn lenient_lines(input: &str) -> Vec<ExpandedAttribute> {
+fn lenient_lines(input: &str) -> Vec<ExpandedAttribute<'_>> {
     gix_attributes::parse(input.as_bytes())
         .map(expand)
         .filter_map(Result::ok)
         .collect()
 }
 
-fn try_lines(input: &str) -> Result<Vec<ExpandedAttribute>, parse::Error> {
+fn try_lines(input: &str) -> Result<Vec<ExpandedAttribute<'_>>, parse::Error> {
     gix_attributes::parse(input.as_bytes()).map(expand).collect()
 }
 

gix-blame/src/file/function.rs

@@ -10,7 +10,7 @@ use gix_traverse::commit::find as find_commit;
 use smallvec::SmallVec;
 
 use super::{process_changes, Change, UnblamedHunk};
-use crate::{BlameEntry, Error, Options, Outcome, Statistics};
+use crate::{types::BlamePathEntry, BlameEntry, Error, Options, Outcome, Statistics};
 
 /// Produce a list of consecutive [`BlameEntry`] instances to indicate in which commits the ranges of the file
 /// at `suspect:<file_path>` originated in.
@@ -115,6 +115,12 @@ pub fn file(
     let mut out = Vec::new();
     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 {
+        Some(Vec::new())
+    } else {
+        None
+    };
+
     'outer: while let Some(suspect) = queue.pop_value() {
         stats.commits_traversed += 1;
         if hunks_to_blame.is_empty() {
@@ -156,6 +162,23 @@ pub fn file(
                 // true here. We could perhaps use diff-tree-to-tree to compare `suspect` against
                 // an empty tree to validate this assumption.
                 if unblamed_to_out_is_done(&mut hunks_to_blame, &mut out, suspect) {
+                    if let Some(ref mut blame_path) = blame_path {
+                        let entry = previous_entry
+                            .take()
+                            .filter(|(id, _)| *id == suspect)
+                            .map(|(_, entry)| entry);
+
+                        let blame_path_entry = BlamePathEntry {
+                            source_file_path: current_file_path.clone(),
+                            previous_source_file_path: None,
+                            commit_id: suspect,
+                            blob_id: entry.unwrap_or(ObjectId::null(gix_hash::Kind::Sha1)),
+                            previous_blob_id: ObjectId::null(gix_hash::Kind::Sha1),
+                            parent_index: 0,
+                        };
+                        blame_path.push(blame_path_entry);
+                    }
+
                     break 'outer;
                 }
             }
@@ -241,13 +264,13 @@ pub fn file(
         }
 
         let more_than_one_parent = parent_ids.len() > 1;
-        for (parent_id, parent_commit_time) in parent_ids {
-            queue.insert(parent_commit_time, parent_id);
+        for (index, (parent_id, parent_commit_time)) in parent_ids.iter().enumerate() {
+            queue.insert(*parent_commit_time, *parent_id);
             let changes_for_file_path = tree_diff_at_file_path(
                 &odb,
                 current_file_path.as_ref(),
                 suspect,
-                parent_id,
+                *parent_id,
                 cache.as_ref(),
                 &mut stats,
                 &mut diff_state,
@@ -262,21 +285,33 @@ pub fn file(
                     // None of the changes affected the file we’re currently blaming.
                     // Copy blame to parent.
                     for unblamed_hunk in &mut hunks_to_blame {
-                        unblamed_hunk.clone_blame(suspect, parent_id);
+                        unblamed_hunk.clone_blame(suspect, *parent_id);
                     }
                 } else {
-                    pass_blame_from_to(suspect, parent_id, &mut hunks_to_blame);
+                    pass_blame_from_to(suspect, *parent_id, &mut hunks_to_blame);
                 }
                 continue;
             };
 
             match modification {
-                TreeDiffChange::Addition => {
+                TreeDiffChange::Addition { id } => {
                     if more_than_one_parent {
                         // Do nothing under the assumption that this always (or almost always)
                         // implies that the file comes from a different parent, compared to which
                         // it was modified, not added.
                     } else if unblamed_to_out_is_done(&mut hunks_to_blame, &mut out, suspect) {
+                        if let Some(ref mut blame_path) = blame_path {
+                            let blame_path_entry = BlamePathEntry {
+                                source_file_path: current_file_path.clone(),
+                                previous_source_file_path: None,
+                                commit_id: suspect,
+                                blob_id: id,
+                                previous_blob_id: ObjectId::null(gix_hash::Kind::Sha1),
+                                parent_index: index,
+                            };
+                            blame_path.push(blame_path_entry);
+                        }
+
                         break 'outer;
                     }
                 }
@@ -294,7 +329,22 @@ pub fn file(
                         options.diff_algorithm,
                         &mut stats,
                     )?;
-                    hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent_id);
+                    hunks_to_blame = process_changes(hunks_to_blame, changes.clone(), suspect, *parent_id);
+                    if let Some(ref mut blame_path) = blame_path {
+                        let has_blame_been_passed = hunks_to_blame.iter().any(|hunk| hunk.has_suspect(parent_id));
+
+                        if has_blame_been_passed {
+                            let blame_path_entry = BlamePathEntry {
+                                source_file_path: current_file_path.clone(),
+                                previous_source_file_path: Some(current_file_path.clone()),
+                                commit_id: suspect,
+                                blob_id: id,
+                                previous_blob_id: previous_id,
+                                parent_index: index,
+                            };
+                            blame_path.push(blame_path_entry);
+                        }
+                    }
                 }
                 TreeDiffChange::Rewrite {
                     source_location,
@@ -311,11 +361,29 @@ pub fn file(
                         options.diff_algorithm,
                         &mut stats,
                     )?;
-                    hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent_id);
+                    hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, *parent_id);
+
+                    let mut has_blame_been_passed = false;
 
                     for hunk in hunks_to_blame.iter_mut() {
-                        if hunk.has_suspect(&parent_id) {
+                        if hunk.has_suspect(parent_id) {
                             hunk.source_file_name = Some(source_location.clone());
+
+                            has_blame_been_passed = true;
+                        }
+                    }
+
+                    if has_blame_been_passed {
+                        if let Some(ref mut blame_path) = blame_path {
+                            let blame_path_entry = BlamePathEntry {
+                                source_file_path: current_file_path.clone(),
+                                previous_source_file_path: Some(source_location.clone()),
+                                commit_id: suspect,
+                                blob_id: id,
+                                previous_blob_id: source_id,
+                                parent_index: index,
+                            };
+                            blame_path.push(blame_path_entry);
                         }
                     }
                 }
@@ -351,6 +419,7 @@ pub fn file(
         entries: coalesce_blame_entries(out),
         blob: blamed_file_blob,
         statistics: stats,
+        blame_path,
     })
 }
 
@@ -435,7 +504,9 @@ fn coalesce_blame_entries(lines_blamed: Vec<BlameEntry>) -> Vec<BlameEntry> {
 /// The union of [`gix_diff::tree::recorder::Change`] and [`gix_diff::tree_with_rewrites::Change`],
 /// keeping only the blame-relevant information.
 enum TreeDiffChange {
-    Addition,
+    Addition {
+        id: ObjectId,
+    },
     Deletion,
     Modification {
         previous_id: ObjectId,
@@ -453,7 +524,7 @@ impl From<gix_diff::tree::recorder::Change> for TreeDiffChange {
         use gix_diff::tree::recorder::Change;
 
         match value {
-            Change::Addition { .. } => Self::Addition,
+            Change::Addition { oid, .. } => Self::Addition { id: oid },
             Change::Deletion { .. } => Self::Deletion,
             Change::Modification { previous_oid, oid, .. } => Self::Modification {
                 previous_id: previous_oid,
@@ -468,7 +539,7 @@ impl From<gix_diff::tree_with_rewrites::Change> for TreeDiffChange {
         use gix_diff::tree_with_rewrites::Change;
 
         match value {
-            Change::Addition { .. } => Self::Addition,
+            Change::Addition { id, .. } => Self::Addition { id },
             Change::Deletion { .. } => Self::Deletion,
             Change::Modification { previous_id, id, .. } => Self::Modification { previous_id, id },
             Change::Rewrite {

gix-blame/src/lib.rs

@@ -17,7 +17,7 @@
 mod error;
 pub use error::Error;
 mod types;
-pub use types::{BlameEntry, BlameRanges, Options, Outcome, Statistics};
+pub use types::{BlameEntry, BlamePathEntry, BlameRanges, Options, Outcome, Statistics};
 
 mod file;
 pub use file::function::file;

gix-blame/src/types.rs

@@ -149,6 +149,33 @@ pub struct Options {
     pub since: Option<gix_date::Time>,
     /// Determine if rename tracking should be performed, and how.
     pub rewrites: Option<gix_diff::Rewrites>,
+    /// Collect debug information whenever there's a diff or rename that affects the outcome of a
+    /// blame.
+    pub debug_track_path: bool,
+}
+
+/// Represents a change during history traversal for blame. It is supposed to capture enough
+/// information to allow reconstruction of the way a blame was performed, i. e. the path the
+/// history traversal, combined with repeated diffing of two subsequent states in this history, has
+/// taken.
+///
+/// This is intended for debugging purposes.
+#[derive(Clone, Debug)]
+pub struct BlamePathEntry {
+    /// The path to the *Source File* in the blob after the change.
+    pub source_file_path: BString,
+    /// The path to the *Source File* in the blob before the change. Allows
+    /// detection of renames. `None` for root commits.
+    pub previous_source_file_path: Option<BString>,
+    /// The commit id associated with the state after the change.
+    pub commit_id: ObjectId,
+    /// The blob id associated with the state after the change.
+    pub blob_id: ObjectId,
+    /// The blob id associated with the state before the change.
+    pub previous_blob_id: ObjectId,
+    /// When there is more than one `BlamePathEntry` for a commit, this indicates to which parent
+    /// commit the change is related.
+    pub parent_index: usize,
 }
 
 /// The outcome of [`file()`](crate::file()).
@@ -161,6 +188,8 @@ pub struct Outcome {
     pub blob: Vec<u8>,
     /// Additional information about the amount of work performed to produce the blame.
     pub statistics: Statistics,
+    /// Contains a log of all changes that affected the outcome of this blame.
+    pub blame_path: Option<Vec<BlamePathEntry>>,
 }
 
 /// Additional information about the performed operations.

gix-blame/tests/blame.rs

@@ -232,6 +232,7 @@ macro_rules! mktest {
                     range: BlameRanges::default(),
                     since: None,
                     rewrites: Some(gix_diff::Rewrites::default()),
+                    debug_track_path: false,
                 },
             )?
             .entries;
@@ -317,6 +318,7 @@ fn diff_disparity() {
                 range: BlameRanges::default(),
                 since: None,
                 rewrites: Some(gix_diff::Rewrites::default()),
+                debug_track_path: false,
             },
         )
         .unwrap()
@@ -352,6 +354,7 @@ fn since() -> gix_testtools::Result {
             range: BlameRanges::default(),
             since: Some(gix_date::parse("2025-01-31", None)?),
             rewrites: Some(gix_diff::Rewrites::default()),
+            debug_track_path: false,
         },
     )?
     .entries;
@@ -391,6 +394,7 @@ mod blame_ranges {
                 range: BlameRanges::from_range(1..=2),
                 since: None,
                 rewrites: Some(gix_diff::Rewrites::default()),
+                debug_track_path: false,
             },
         )?
         .entries;
@@ -431,6 +435,7 @@ mod blame_ranges {
                 range: ranges,
                 since: None,
                 rewrites: None,
+                debug_track_path: false,
             },
         )?
         .entries;
@@ -471,6 +476,7 @@ mod blame_ranges {
                 range: ranges,
                 since: None,
                 rewrites: None,
+                debug_track_path: false,
             },
         )?
         .entries;
@@ -516,6 +522,7 @@ mod rename_tracking {
                 range: BlameRanges::default(),
                 since: None,
                 rewrites: Some(gix_diff::Rewrites::default()),
+                debug_track_path: false,
             },
         )?
         .entries;

gix-config/tests/config/parse/section.rs

@@ -11,7 +11,7 @@ mod header {
 
     use bstr::BStr;
 
-    fn cow_section(name: &str) -> Option<Cow<BStr>> {
+    fn cow_section(name: &str) -> Option<Cow<'_, BStr>> {
         Some(Cow::Borrowed(name.into()))
     }
     mod write_to {

gix-odb/src/store_impls/dynamic/load_index.rs

@@ -734,6 +734,7 @@ impl PartialEq<Self> for Either {
     }
 }
 
+#[allow(clippy::non_canonical_partial_ord_impl)]
 impl PartialOrd<Self> for Either {
     fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
         Some(self.path().cmp(other.path()))

gix-ref/src/name.rs

@@ -221,6 +221,7 @@ impl<'a> convert::TryFrom<&'a str> for PartialName {
     }
 }
 
+#[allow(clippy::infallible_try_from)]
 impl<'a> convert::TryFrom<&'a FullName> for &'a PartialNameRef {
     type Error = Infallible;
 

gix-refspec/src/spec.rs

@@ -69,6 +69,7 @@ mod impls {
         }
     }
 
+    #[allow(clippy::non_canonical_partial_ord_impl)]
     impl PartialOrd for RefSpec {
         fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
             Some(self.to_ref().cmp(&other.to_ref()))