1
1
mirror of https://github.com/Byron/gitoxide synced 2025-10-06 01:52:40 +02:00

Merge pull request #1630 from GitoxideLabs/diff-fix

diff fix
This commit is contained in:
Sebastian Thiel
2024-10-15 17:37:33 +02:00
committed by GitHub
4 changed files with 606 additions and 15 deletions

View File

@@ -172,7 +172,10 @@ impl<T: Change> Tracker<T> {
.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<T: Change> Tracker<T> {
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<T: Change>(a: &Item<T>, b: &Item<T>) -> 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<T: Change> Tracker<T> {
&mut out,
diff_cache,
objects,
None,
)?;
if let Some(copies) = self.rewrites.copies {
@@ -252,6 +274,7 @@ impl<T: Change> Tracker<T> {
&mut out,
diff_cache,
objects,
None,
)?;
match copies.source {
@@ -275,6 +298,7 @@ impl<T: Change> Tracker<T> {
&mut out,
diff_cache,
objects,
None,
)?;
}
}
@@ -299,6 +323,7 @@ impl<T: Change> Tracker<T> {
}
impl<T: Change> Tracker<T> {
#[allow(clippy::too_many_arguments)]
fn match_pairs_of_kind(
&mut self,
kind: visit::SourceKind,
@@ -307,10 +332,11 @@ impl<T: Change> Tracker<T> {
out: &mut Outcome,
diff_cache: &mut crate::blob::Platform,
objects: &impl gix_object::FindObjectOrHeader,
filter: Option<fn(&T) -> 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<T: Change> Tracker<T> {
}
};
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<visit::Source<'_, T>>) -> Action,
@@ -349,10 +376,14 @@ impl<T: Change> Tracker<T> {
stats: &mut Outcome,
diff_cache: &mut crate::blob::Platform,
objects: &impl gix_object::FindObjectOrHeader,
filter: Option<fn(&T) -> bool>,
) -> Result<Action, emit::Error> {
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;

View File

@@ -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(())
}

View File

@@ -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/
)

View File

@@ -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<str>) -> gix::Tree {