ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

git: return refs that failed to export in sorted order

Browse source 
Before this patch, the order would depend on the reason we failed to
export a ref, because we would add to the `failed_branches` list in
several different places. What's worse, when the export failed because
the branch was conflicted or had an invalid name (from Git's
perspective), it was non-deterministic because we iterated over a
HashSet. This patch fixes that by sorting at the end.

Note that we still want the `branches_to_update` map to be a
`BTreeMap` so we update branches in deterministic order. Otherwise the
error when trying to export both branches `main` and `main/sub` will
become non-deterministic.
This commit is contained in:
Martin von Zweigbergk 2023-09-04 07:26:18 -07:00 committed by Martin von Zweigbergk
parent b0c8e9ef62
commit f6c24fbcef
2 changed files with 4 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
lib/src/git.rs
View file

@ -557,6 +557,7 @@ pub fn export_some_refs(
failed_branches.push(parsed_ref_name);
}
}
failed_branches.sort();
Ok(failed_branches)
}

6
lib/tests/test_git.rs
View file

@ -1351,8 +1351,8 @@ fn test_export_partial_failure() {
assert_eq!(
git::export_refs(mut_repo, &git_repo),
Ok(vec![
RefName::LocalBranch("HEAD".to_string()),
RefName::LocalBranch("".to_string()),
RefName::LocalBranch("HEAD".to_string()),
RefName::LocalBranch("main/sub".to_string())
])
);
@ -1376,8 +1376,8 @@ fn test_export_partial_failure() {
assert_eq!(
git::export_refs(mut_repo, &git_repo),
Ok(vec![
RefName::LocalBranch("".to_string()),
RefName::LocalBranch("HEAD".to_string()),
RefName::LocalBranch("".to_string())
])
);
assert!(git_repo.find_reference("refs/heads/").is_err());
@ -1472,7 +1472,7 @@ fn test_export_reexport_transitions() {
// mut_repo.view().git_refs().
assert_eq!(
git::export_refs(mut_repo, &git_repo),
Ok(["AXB", "ABC", "ABX", "XAB"]
Ok(["ABC", "ABX", "AXB", "XAB"]
.into_iter()
.map(|s| RefName::LocalBranch(s.to_string()))
.collect_vec())