From f6c24fbceffcfc838dfcd7defd972c96bd11d874 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 4 Sep 2023 07:26:18 -0700 Subject: [PATCH] git: return refs that failed to export in sorted order 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. --- lib/src/git.rs | 1 + lib/tests/test_git.rs | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/src/git.rs b/lib/src/git.rs index 4ec483b8c..554562b3a 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -557,6 +557,7 @@ pub fn export_some_refs( failed_branches.push(parsed_ref_name); } } + failed_branches.sort(); Ok(failed_branches) } diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 18f0f7070..7d25231f2 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -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())