diff --git a/gix-diff/src/rewrites/tracker.rs b/gix-diff/src/rewrites/tracker.rs index bd10edf4ffd..df166bd43e3 100644 --- a/gix-diff/src/rewrites/tracker.rs +++ b/gix-diff/src/rewrites/tracker.rs @@ -172,7 +172,10 @@ impl Tracker { .relation() .filter(|_| matches!(change_kind, ChangeKind::Addition | ChangeKind::Deletion)); let entry_kind = change.entry_mode().kind(); - if let (None | Some(Relation::ChildOfParent(_)), EntryKind::Commit | EntryKind::Tree) = (relation, entry_kind) { + if entry_kind == EntryKind::Commit { + return Some(change); + } + if let (None, EntryKind::Tree) = (relation, entry_kind) { return Some(change); }; @@ -221,20 +224,38 @@ impl Tracker { PushSourceTreeFn: FnMut(&mut dyn FnMut(T, &BStr)) -> Result<(), E>, E: std::error::Error + Send + Sync + 'static, { + fn is_parent(change: &impl Change) -> bool { + matches!(change.relation(), Some(Relation::Parent(_))) + } diff_cache.options.skip_internal_diff_if_external_is_configured = false; + // The point of this is to optimize for identity-based lookups, which should be easy to find + // by partitioning. fn by_id_and_location(a: &Item, b: &Item) -> std::cmp::Ordering { a.change .id() .cmp(b.change.id()) .then_with(|| a.path.start.cmp(&b.path.start).then(a.path.end.cmp(&b.path.end))) } - self.items.sort_by(by_id_and_location); let mut out = Outcome { options: self.rewrites, ..Default::default() }; + self.items.sort_by(by_id_and_location); + + // Rewrites by directory can be pruned out quickly, quickly pruning candidates + // for the following per-item steps. + self.match_pairs_of_kind( + visit::SourceKind::Rename, + &mut cb, + None, /* by identity for parents */ + &mut out, + diff_cache, + objects, + Some(is_parent), + )?; + self.match_pairs_of_kind( visit::SourceKind::Rename, &mut cb, @@ -242,6 +263,7 @@ impl Tracker { &mut out, diff_cache, objects, + None, )?; if let Some(copies) = self.rewrites.copies { @@ -252,6 +274,7 @@ impl Tracker { &mut out, diff_cache, objects, + None, )?; match copies.source { @@ -275,6 +298,7 @@ impl Tracker { &mut out, diff_cache, objects, + None, )?; } } @@ -299,6 +323,7 @@ impl Tracker { } impl Tracker { + #[allow(clippy::too_many_arguments)] fn match_pairs_of_kind( &mut self, kind: visit::SourceKind, @@ -307,10 +332,11 @@ impl Tracker { out: &mut Outcome, diff_cache: &mut crate::blob::Platform, objects: &impl gix_object::FindObjectOrHeader, + filter: Option bool>, ) -> Result<(), emit::Error> { // we try to cheaply reduce the set of possibilities first, before possibly looking more exhaustively. let needs_second_pass = !needs_exact_match(percentage); - if self.match_pairs(cb, None /* by identity */, kind, out, diff_cache, objects)? == Action::Cancel { + if self.match_pairs(cb, None /* by identity */, kind, out, diff_cache, objects, filter)? == Action::Cancel { return Ok(()); } if needs_second_pass { @@ -335,12 +361,13 @@ impl Tracker { } }; if !is_limited { - self.match_pairs(cb, percentage, kind, out, diff_cache, objects)?; + self.match_pairs(cb, percentage, kind, out, diff_cache, objects, None)?; } } Ok(()) } + #[allow(clippy::too_many_arguments)] fn match_pairs( &mut self, cb: &mut impl FnMut(visit::Destination<'_, T>, Option>) -> Action, @@ -349,10 +376,14 @@ impl Tracker { stats: &mut Outcome, diff_cache: &mut crate::blob::Platform, objects: &impl gix_object::FindObjectOrHeader, + filter: Option bool>, ) -> Result { let mut dest_ofs = 0; while let Some((mut dest_idx, dest)) = self.items[dest_ofs..].iter().enumerate().find_map(|(idx, item)| { - (!item.emitted && matches!(item.change.kind(), ChangeKind::Addition)).then_some((idx, item)) + (!item.emitted + && matches!(item.change.kind(), ChangeKind::Addition) + && filter.map_or(true, |f| f(&item.change))) + .then_some((idx, item)) }) { dest_idx += dest_ofs; dest_ofs = dest_idx + 1; diff --git a/gix-diff/tests/diff/rewrites/tracker.rs b/gix-diff/tests/diff/rewrites/tracker.rs index 1aeb0f442c4..fb6da4c5b47 100644 --- a/gix-diff/tests/diff/rewrites/tracker.rs +++ b/gix-diff/tests/diff/rewrites/tracker.rs @@ -650,9 +650,9 @@ fn simple_directory_rename_by_id() -> crate::Result { }, "d/subdir".into(), ) - .is_some(), - "trees that are children are simply ignored. It's easier to compare views of worktrees (`gix-dirwalk`) \ - and trees/indices that way." + .is_none(), + "trees that are children are kept and matched. That way, they can quickly be pruned which is done first.\ + Those who don't need them can prune them in a later step." ); assert!(track .try_push_change( @@ -664,7 +664,7 @@ fn simple_directory_rename_by_id() -> crate::Result { }, "d-renamed/subdir".into(), ) - .is_some()); + .is_none()); let _odb = util::add_retained_blobs( &mut track, [ @@ -692,22 +692,23 @@ fn simple_directory_rename_by_id() -> crate::Result { assert_eq!(dst.change.relation, Some(Relation::Parent(1))); assert_eq!(dst.change.mode.kind(), EntryKind::Tree); } - 1..=4 => { + 1..=5 => { let src = src.unwrap(); let (expected_src, expected_dst) = &[ ("d/a", "d-renamed/a"), ("d/c", "d-renamed/c"), ("d/b", "d-renamed/b"), + ("d/subdir", "d-renamed/subdir"), ("d/subdir/d", "d-renamed/subdir/d"), ][calls - 1]; assert_eq!(src.location, expected_src); assert_eq!(dst.location, expected_dst); } - 5 => { + 6 => { assert_eq!(src, None); assert_eq!(dst.location, "a"); } - 6 => { + 7 => { assert_eq!(src, None); assert_eq!(dst.location, "b"); } @@ -723,7 +724,7 @@ fn simple_directory_rename_by_id() -> crate::Result { ..Default::default() } ); - assert_eq!(calls, 7, "Should not have too few calls"); + assert_eq!(calls, 8, "Should not have too few calls"); Ok(()) } diff --git a/gix/tests/fixtures/make_diff_repos.sh b/gix/tests/fixtures/make_diff_repos.sh index 356d1d941f9..e085c848d2d 100755 --- a/gix/tests/fixtures/make_diff_repos.sh +++ b/gix/tests/fixtures/make_diff_repos.sh @@ -14,4 +14,10 @@ git init jj-trackcopy-1 rm -f "$index" git update-index --index-info < "$ROOT/assets/jj-trackcopy-1/47bd6f4aa4a7eeef8b01ce168c6c771bdfffcbd3.tree" git commit --allow-empty -F "$ROOT/assets/jj-trackcopy-1/47bd6f4aa4a7eeef8b01ce168c6c771bdfffcbd3.msg" + + git checkout -f HEAD + git mv cli c + git commit -m "renamed cli to c" + + rm -Rf c/ ) \ No newline at end of file diff --git a/gix/tests/gix/object/tree/diff.rs b/gix/tests/gix/object/tree/diff.rs index 1a2f41249e7..cf44645d7cd 100644 --- a/gix/tests/gix/object/tree/diff.rs +++ b/gix/tests/gix/object/tree/diff.rs @@ -168,8 +168,8 @@ mod track_rewrites { ("cli/tests/test_cat_command.rs".into(), 77), ); - let from = tree_named(&repo, "@~1"); - let to = tree_named(&repo, "@"); + let from = tree_named(&repo, "@~2"); + let to = tree_named(&repo, "@~1"); let rewrites = Rewrites { copies: Some(Copies { source: CopySource::FromSetOfModifiedFiles, @@ -489,6 +489,559 @@ mod track_rewrites { Ok(()) } + + #[test] + #[cfg_attr( + windows, + ignore = "Fails on some Window systems, like the fixture doesn't get set up correctly." + )] + fn jj_realistic_directory_rename() -> crate::Result { + let repo = named_subrepo_opts("make_diff_repos.sh", "jj-trackcopy-1", gix::open::Options::isolated())?; + + let from = tree_named(&repo, "@~1"); + let to = tree_named(&repo, "@"); + let actual: Vec<_> = repo + .diff_tree_to_tree( + &from, + &to, + Some(gix::diff::Options::default().with_rewrites(Some(Rewrites::default()))), + )? + .into_iter() + .collect(); + insta::assert_debug_snapshot!(actual, @r#" + [ + Rewrite { + source_location: "cli", + source_entry_mode: EntryMode( + 16384, + ), + source_relation: Some( + Parent( + 2, + ), + ), + source_id: Sha1(f203064a6a81df47498fb415a2064a8ec568ed67), + diff: None, + entry_mode: EntryMode( + 16384, + ), + id: Sha1(f203064a6a81df47498fb415a2064a8ec568ed67), + location: "c", + relation: Some( + Parent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/src/commands/file/print.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(081093be2ba0d2be62d14363f43859355bee2aa2), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(081093be2ba0d2be62d14363f43859355bee2aa2), + location: "c/src/commands/file/print.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/src/commands/file", + source_entry_mode: EntryMode( + 16384, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(0f3bc154b577b84fb5ce31383e25acc99c2f24a5), + diff: None, + entry_mode: EntryMode( + 16384, + ), + id: Sha1(0f3bc154b577b84fb5ce31383e25acc99c2f24a5), + location: "c/src/commands/file", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/tests", + source_entry_mode: EntryMode( + 16384, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(17be3b367831653883a36a2f2a8dea418b8d96b7), + diff: None, + entry_mode: EntryMode( + 16384, + ), + id: Sha1(17be3b367831653883a36a2f2a8dea418b8d96b7), + location: "c/tests", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/tests/test_immutable_commits.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(3d7598b4e4c570eef701f40853ef3e3b0fb224f7), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(3d7598b4e4c570eef701f40853ef3e3b0fb224f7), + location: "c/tests/test_immutable_commits.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/tests/test_file_print_command.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(45bb2cf6b7fa96a39c95301f619ca3e4cc3eb0f3), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(45bb2cf6b7fa96a39c95301f619ca3e4cc3eb0f3), + location: "c/tests/test_file_print_command.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/tests/runner.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(5253f0ff160e8b7001a7bd271ca4a07968ff81a3), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(5253f0ff160e8b7001a7bd271ca4a07968ff81a3), + location: "c/tests/runner.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/src", + source_entry_mode: EntryMode( + 16384, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(80e5b08f25f75c2050afbcb794e8434f4cf082f1), + diff: None, + entry_mode: EntryMode( + 16384, + ), + id: Sha1(80e5b08f25f75c2050afbcb794e8434f4cf082f1), + location: "c/src", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/tests/test_file_chmod_command.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(8defe631bc82bf35a53cd25083f85664516f412f), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(8defe631bc82bf35a53cd25083f85664516f412f), + location: "c/tests/test_file_chmod_command.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/tests/cli-reference@.md.snap", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(92853cde19b20cadd74113ea3566c87d4def591b), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(92853cde19b20cadd74113ea3566c87d4def591b), + location: "c/tests/cli-reference@.md.snap", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/src/commands/file/chmod.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(94f78deb408d181ccea9da574d0e45ac32a98092), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(94f78deb408d181ccea9da574d0e45ac32a98092), + location: "c/src/commands/file/chmod.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/tests/test_new_command.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(a03b50a8a9c23c68d641b51b7c887ea088cd0d2b), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(a03b50a8a9c23c68d641b51b7c887ea088cd0d2b), + location: "c/tests/test_new_command.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/tests/test_global_opts.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(a0c0340e495fa759c0b705dd46cee322aa0d80c8), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(a0c0340e495fa759c0b705dd46cee322aa0d80c8), + location: "c/tests/test_global_opts.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/tests/test_move_command.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(ac9ad5761637cd731abe1bf4a075fedda7bfc61f), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(ac9ad5761637cd731abe1bf4a075fedda7bfc61f), + location: "c/tests/test_move_command.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/tests/test_unsquash_command.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(b8b29cc0ca0176fafaa97c7421a10ed116bcba8a), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(b8b29cc0ca0176fafaa97c7421a10ed116bcba8a), + location: "c/tests/test_unsquash_command.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/src/commands/file/mod.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(d67f782327ea286136b8532eaf9a509806a87e83), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(d67f782327ea286136b8532eaf9a509806a87e83), + location: "c/src/commands/file/mod.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/tests/test_fix_command.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(e0baefc79038fed0bcf56f2d8c3588a26d5bf985), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(e0baefc79038fed0bcf56f2d8c3588a26d5bf985), + location: "c/tests/test_fix_command.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/src/commands/mod.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(e3a9ec4524d27aa7035a38fd7c5db414809623c4), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(e3a9ec4524d27aa7035a38fd7c5db414809623c4), + location: "c/src/commands/mod.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/src/commands", + source_entry_mode: EntryMode( + 16384, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(f414de88468352d59c129d0e7686fb1e1f387929), + diff: None, + entry_mode: EntryMode( + 16384, + ), + id: Sha1(f414de88468352d59c129d0e7686fb1e1f387929), + location: "c/src/commands", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/tests/test_acls.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(f644e4c8dd0be6fbe5493b172ce10839bcd9e25c), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(f644e4c8dd0be6fbe5493b172ce10839bcd9e25c), + location: "c/tests/test_acls.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/tests/test_diffedit_command.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(fd57f61e92d4d49b4920c08c3522c066cb03ecd2), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(fd57f61e92d4d49b4920c08c3522c066cb03ecd2), + location: "c/tests/test_diffedit_command.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + Rewrite { + source_location: "cli/tests/test_squash_command.rs", + source_entry_mode: EntryMode( + 33188, + ), + source_relation: Some( + ChildOfParent( + 2, + ), + ), + source_id: Sha1(ff1c247d4312adb5b372c6d9ff93fa71846ca527), + diff: None, + entry_mode: EntryMode( + 33188, + ), + id: Sha1(ff1c247d4312adb5b372c6d9ff93fa71846ca527), + location: "c/tests/test_squash_command.rs", + relation: Some( + ChildOfParent( + 1, + ), + ), + copy: false, + }, + ] + "#); + Ok(()) + } } fn tree_named(repo: &gix::Repository, rev_spec: impl AsRef) -> gix::Tree {