[F] Fix branch deletion awareness

This commit is contained in:
2026-05-07 02:58:43 +00:00
parent 44c660e6c2
commit 09657d9adf
3 changed files with 389 additions and 22 deletions
+1 -1
View File
@@ -160,7 +160,7 @@ Branch conflict handling is intentionally conservative:
- If branch tips diverged, that branch is skipped and reported. - If branch tips diverged, that branch is skipped and reported.
- If `allow_force = true` or `git-sync sync --force` is used, a diverged branch chooses the newest commit timestamp and force-pushes it. - If `allow_force = true` or `git-sync sync --force` is used, a diverged branch chooses the newest commit timestamp and force-pushes it.
Branch deletion is not propagated. If a branch exists on one endpoint and is missing elsewhere, it is recreated elsewhere. This avoids accidental data loss in a bidirectional mirror. Branch deletion is propagated only when it is safe to infer intent. If a branch existed on every endpoint in the previous successful sync, then disappears from one endpoint while the remaining endpoints still have the previous tip, `git-sync` deletes it from the remaining endpoints instead of recreating it. If the branch was deleted on one endpoint but changed elsewhere, it is treated as a conflict and skipped.
Tags are fetched into provider-specific cache refs and pushed only when the tag object agrees across providers or exists on one side. Divergent tags are skipped and reported. Tag deletion is not propagated. Tags are fetched into provider-specific cache refs and pushed only when the tag object agrees across providers or exists on one side. Divergent tags are skipped and reported. Tag deletion is not propagated.
+98 -3
View File
@@ -19,6 +19,8 @@ pub struct RemoteSpec {
pub struct RemoteRefSnapshot { pub struct RemoteRefSnapshot {
pub hash: String, pub hash: String,
pub refs: usize, pub refs: usize,
pub branches: BTreeMap<String, String>,
pub tags: BTreeMap<String, String>,
} }
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
@@ -35,6 +37,13 @@ pub struct BranchConflict {
pub tips: Vec<(String, String)>, pub tips: Vec<(String, String)>,
} }
#[derive(Clone, Debug)]
pub struct BranchDeletion {
pub branch: String,
pub deleted_remotes: Vec<String>,
pub target_remotes: Vec<String>,
}
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
pub struct TagDecision { pub struct TagDecision {
pub tag: String, pub tag: String,
@@ -114,8 +123,17 @@ impl GitMirror {
remote: &RemoteSpec, remote: &RemoteSpec,
expected: &RemoteRefSnapshot, expected: &RemoteRefSnapshot,
) -> Result<bool> { ) -> Result<bool> {
Ok(self
.cached_remote_ref_snapshot(remote)?
.is_some_and(|snapshot| &snapshot == expected))
}
pub fn cached_remote_ref_snapshot(
&self,
remote: &RemoteSpec,
) -> Result<Option<RemoteRefSnapshot>> {
if !self.path.exists() || self.dry_run { if !self.path.exists() || self.dry_run {
return Ok(false); return Ok(None);
} }
let branches = self.remote_branches(&remote.name)?; let branches = self.remote_branches(&remote.name)?;
let tags = self.remote_tags(&remote.name)?; let tags = self.remote_tags(&remote.name)?;
@@ -126,8 +144,7 @@ impl GitMirror {
for (tag, sha) in tags { for (tag, sha) in tags {
refs.push(format!("{sha}\trefs/tags/{tag}")); refs.push(format!("{sha}\trefs/tags/{tag}"));
} }
let snapshot = snapshot_from_refs(refs); Ok(Some(snapshot_from_refs(refs)))
Ok(&snapshot == expected)
} }
pub fn branch_decisions( pub fn branch_decisions(
@@ -302,6 +319,30 @@ impl GitMirror {
Ok(()) Ok(())
} }
pub fn delete_branches(
&self,
remotes: &[RemoteSpec],
deletions: &[BranchDeletion],
) -> Result<()> {
for remote in remotes {
for deletion in deletions {
if !deletion.target_remotes.contains(&remote.name) {
continue;
}
let refspec = format!(":refs/heads/{}", deletion.branch);
crate::logln!(
" {} {} {} {}",
style("delete").red().bold(),
style("branch").dim(),
style(&deletion.branch).cyan(),
style(format!("-> {}", remote.display)).dim()
);
self.run(["push", &remote.name, &refspec])?;
}
}
Ok(())
}
fn remote_url(&self, name: &str) -> Result<Option<String>> { fn remote_url(&self, name: &str) -> Result<Option<String>> {
let output = Command::new("git") let output = Command::new("git")
.arg("--git-dir") .arg("--git-dir")
@@ -465,10 +506,24 @@ pub fn ls_remote_refs(remote: &RemoteSpec, redactor: &Redactor) -> Result<Remote
fn snapshot_from_refs(mut refs: Vec<String>) -> RemoteRefSnapshot { fn snapshot_from_refs(mut refs: Vec<String>) -> RemoteRefSnapshot {
refs.sort(); refs.sort();
let mut branches = BTreeMap::new();
let mut tags = BTreeMap::new();
for line in &refs {
let Some((sha, reference)) = line.split_once('\t') else {
continue;
};
if let Some(branch) = reference.strip_prefix("refs/heads/") {
branches.insert(branch.to_string(), sha.to_string());
} else if let Some(tag) = reference.strip_prefix("refs/tags/") {
tags.insert(tag.to_string(), sha.to_string());
}
}
RemoteRefSnapshot { RemoteRefSnapshot {
hash: stable_ref_hash(&refs), hash: stable_ref_hash(&refs),
refs: refs.len(), refs: refs.len(),
branches,
tags,
} }
} }
@@ -822,6 +877,29 @@ mod tests {
); );
} }
#[test]
fn delete_branches_removes_branch_from_target_remotes() {
let fixture = GitFixture::new();
fixture.commit("base", "base", 1_700_000_000);
fixture.push_head(&fixture.remote_a, "main");
fixture.push_head(&fixture.remote_b, "main");
let mirror = fixture.mirror();
mirror
.delete_branches(
&fixture.remotes(),
&[BranchDeletion {
branch: "main".to_string(),
deleted_remotes: vec!["a".to_string()],
target_remotes: vec!["b".to_string()],
}],
)
.unwrap();
assert!(fixture.remote_ref_exists(&fixture.remote_a, "refs/heads/main"));
assert!(!fixture.remote_ref_exists(&fixture.remote_b, "refs/heads/main"));
}
#[test] #[test]
fn tag_decisions_mirror_matching_or_missing_tags_and_skip_divergent_tags() { fn tag_decisions_mirror_matching_or_missing_tags_and_skip_divergent_tags() {
let fixture = GitFixture::new(); let fixture = GitFixture::new();
@@ -1016,6 +1094,23 @@ mod tests {
], ],
) )
} }
fn remote_ref_exists(&self, remote: &Path, reference: &str) -> bool {
git_command(
None,
[
"--git-dir",
remote.to_str().unwrap(),
"rev-parse",
"--verify",
reference,
],
)
.output()
.unwrap()
.status
.success()
}
} }
fn git<const N: usize>(current_dir: Option<&Path>, args: [&str; N]) { fn git<const N: usize>(current_dir: Option<&Path>, args: [&str; N]) {
+290 -18
View File
@@ -11,8 +11,8 @@ use serde::{Deserialize, Serialize};
use crate::config::{Config, EndpointConfig, MirrorConfig, default_work_dir, validate_config}; use crate::config::{Config, EndpointConfig, MirrorConfig, default_work_dir, validate_config};
use crate::git::{ use crate::git::{
GitMirror, Redactor, RemoteRefSnapshot, RemoteSpec, is_disabled_repository_error, BranchDeletion, GitMirror, Redactor, RemoteRefSnapshot, RemoteSpec,
ls_remote_refs, safe_remote_name, is_disabled_repository_error, ls_remote_refs, safe_remote_name,
}; };
use crate::logging; use crate::logging;
use crate::provider::{EndpointRepo, ProviderClient, repos_by_name}; use crate::provider::{EndpointRepo, ProviderClient, repos_by_name};
@@ -219,6 +219,10 @@ fn failure_state_path(work_dir: &Path) -> PathBuf {
struct RemoteRefState { struct RemoteRefState {
hash: String, hash: String,
refs: usize, refs: usize,
#[serde(default)]
branches: BTreeMap<String, String>,
#[serde(default)]
tags: BTreeMap<String, String>,
} }
impl From<RemoteRefSnapshot> for RemoteRefState { impl From<RemoteRefSnapshot> for RemoteRefState {
@@ -226,6 +230,8 @@ impl From<RemoteRefSnapshot> for RemoteRefState {
Self { Self {
hash: value.hash, hash: value.hash,
refs: value.refs, refs: value.refs,
branches: value.branches,
tags: value.tags,
} }
} }
} }
@@ -235,6 +241,8 @@ impl From<&RemoteRefState> for RemoteRefSnapshot {
Self { Self {
hash: value.hash.clone(), hash: value.hash.clone(),
refs: value.refs, refs: value.refs,
branches: value.branches.clone(),
tags: value.tags.clone(),
} }
} }
} }
@@ -261,6 +269,10 @@ impl RefState {
.or_default() .or_default()
.insert(repo.to_string(), refs); .insert(repo.to_string(), refs);
} }
fn repo(&self, group: &str, repo: &str) -> Option<&BTreeMap<String, RemoteRefState>> {
self.repos.get(group).and_then(|repos| repos.get(repo))
}
} }
fn load_ref_state(work_dir: &Path) -> Result<RefState> { fn load_ref_state(work_dir: &Path) -> Result<RefState> {
@@ -687,6 +699,7 @@ fn sync_repo(
let mirror_repo = GitMirror::open(path, context.redactor.clone(), context.dry_run)?; let mirror_repo = GitMirror::open(path, context.redactor.clone(), context.dry_run)?;
mirror_repo.configure_remotes(&initial_remotes)?; mirror_repo.configure_remotes(&initial_remotes)?;
let cached_ref_state = cached_ref_state(&mirror_repo, &initial_remotes)?;
if !context.dry_run if !context.dry_run
&& all_endpoints_present && all_endpoints_present
&& cached_refs_match(&mirror_repo, &initial_remotes, &initial_ref_state)? && cached_refs_match(&mirror_repo, &initial_remotes, &initial_ref_state)?
@@ -758,7 +771,14 @@ fn sync_repo(
} }
} }
let result = push_repo_refs(context, &mirror_repo, &remotes)?; let result = push_repo_refs(
context,
&mirror_repo,
&remotes,
detailed_repo_ref_state(ref_state.repo(&context.mirror.name, repo_name))
.or(cached_ref_state.as_ref()),
&initial_ref_state,
)?;
if !context.dry_run && !result.had_conflicts { if !context.dry_run && !result.had_conflicts {
let refs = if result.pushed { let refs = if result.pushed {
let Some(refs) = check_remote_refs(context, repo_name, &remotes)? else { let Some(refs) = check_remote_refs(context, repo_name, &remotes)? else {
@@ -802,6 +822,29 @@ fn cached_refs_match(
Ok(true) Ok(true)
} }
fn cached_ref_state(
mirror_repo: &GitMirror,
remotes: &[RemoteSpec],
) -> Result<Option<BTreeMap<String, RemoteRefState>>> {
let mut refs = BTreeMap::new();
for remote in remotes {
let Some(snapshot) = mirror_repo.cached_remote_ref_snapshot(remote)? else {
return Ok(None);
};
refs.insert(remote.name.clone(), snapshot.into());
}
Ok(Some(refs))
}
fn detailed_repo_ref_state(
refs: Option<&BTreeMap<String, RemoteRefState>>,
) -> Option<&BTreeMap<String, RemoteRefState>> {
refs.filter(|refs| {
refs.values()
.any(|remote| !remote.branches.is_empty() || !remote.tags.is_empty())
})
}
fn check_remote_refs( fn check_remote_refs(
context: &RepoSyncContext<'_>, context: &RepoSyncContext<'_>,
repo_name: &str, repo_name: &str,
@@ -868,14 +911,35 @@ fn push_repo_refs(
context: &RepoSyncContext<'_>, context: &RepoSyncContext<'_>,
mirror_repo: &GitMirror, mirror_repo: &GitMirror,
remotes: &[RemoteSpec], remotes: &[RemoteSpec],
previous_refs: Option<&BTreeMap<String, RemoteRefState>>,
current_refs: &BTreeMap<String, RemoteRefState>,
) -> Result<RepoRefSyncResult> { ) -> Result<RepoRefSyncResult> {
let (branch_deletions, deletion_conflicts, blocked_branches) =
branch_deletion_decisions(remotes, previous_refs, current_refs);
let had_deletion_conflicts = !deletion_conflicts.is_empty();
for conflict in deletion_conflicts {
crate::logln!(
" {} branch {} was deleted on {} but changed on {} ({})",
style("conflict").yellow().bold(),
style(conflict.branch).cyan(),
conflict.deleted_remotes.join("+"),
conflict.changed_remotes.join("+"),
style("skipped").dim()
);
}
let (branches, conflicts) = mirror_repo.branch_decisions(remotes, context.allow_force)?; let (branches, conflicts) = mirror_repo.branch_decisions(remotes, context.allow_force)?;
let had_branch_conflicts = !conflicts.is_empty();
let branches_to_push = branches let branches_to_push = branches
.into_iter() .into_iter()
.filter(|branch| !blocked_branches.contains(&branch.branch))
.filter(|branch| !branch.target_remotes.is_empty()) .filter(|branch| !branch.target_remotes.is_empty())
.collect::<Vec<_>>(); .collect::<Vec<_>>();
let mut had_branch_conflicts = false;
for conflict in conflicts { for conflict in conflicts {
if blocked_branches.contains(&conflict.branch) {
continue;
}
had_branch_conflicts = true;
let details = conflict let details = conflict
.tips .tips
.iter() .iter()
@@ -914,15 +978,33 @@ fn push_repo_refs(
} }
if branches_to_push.is_empty() && tags_to_push.is_empty() { if branches_to_push.is_empty() && tags_to_push.is_empty() {
if !branch_deletions.is_empty() {
print_branch_deletions(&branch_deletions);
mirror_repo.delete_branches(remotes, &branch_deletions)?;
return Ok(RepoRefSyncResult {
pushed: true,
had_conflicts: had_deletion_conflicts,
});
}
if had_branch_conflicts || had_tag_conflicts || had_deletion_conflicts {
return Ok(RepoRefSyncResult {
pushed: false,
had_conflicts: true,
});
}
crate::logln!( crate::logln!(
" {} branches and tags already match all endpoints", " {} branches and tags already match all endpoints",
style("up-to-date").green().bold() style("up-to-date").green().bold()
); );
return Ok(RepoRefSyncResult { return Ok(RepoRefSyncResult {
pushed: false, pushed: false,
had_conflicts: had_branch_conflicts || had_tag_conflicts, had_conflicts: had_branch_conflicts || had_tag_conflicts || had_deletion_conflicts,
}); });
} }
if !branch_deletions.is_empty() {
print_branch_deletions(&branch_deletions);
mirror_repo.delete_branches(remotes, &branch_deletions)?;
}
if !branches_to_push.is_empty() { if !branches_to_push.is_empty() {
print_branch_decisions(&branches_to_push); print_branch_decisions(&branches_to_push);
mirror_repo.push_branches(remotes, &branches_to_push, context.allow_force)?; mirror_repo.push_branches(remotes, &branches_to_push, context.allow_force)?;
@@ -933,10 +1015,102 @@ fn push_repo_refs(
} }
Ok(RepoRefSyncResult { Ok(RepoRefSyncResult {
pushed: true, pushed: true,
had_conflicts: had_branch_conflicts || had_tag_conflicts, had_conflicts: had_branch_conflicts || had_tag_conflicts || had_deletion_conflicts,
}) })
} }
struct BranchDeletionConflict {
branch: String,
deleted_remotes: Vec<String>,
changed_remotes: Vec<String>,
}
fn branch_deletion_decisions(
remotes: &[RemoteSpec],
previous_refs: Option<&BTreeMap<String, RemoteRefState>>,
current_refs: &BTreeMap<String, RemoteRefState>,
) -> (
Vec<BranchDeletion>,
Vec<BranchDeletionConflict>,
BTreeSet<String>,
) {
let Some(previous_refs) = previous_refs else {
return (Vec::new(), Vec::new(), BTreeSet::new());
};
let remote_names = remotes
.iter()
.map(|remote| remote.name.clone())
.collect::<Vec<_>>();
let mut branches = BTreeSet::new();
for refs in previous_refs.values() {
branches.extend(refs.branches.keys().cloned());
}
let mut deletions = Vec::new();
let mut conflicts = Vec::new();
let mut blocked = BTreeSet::new();
for branch in branches {
let previous_remotes = remote_names
.iter()
.filter(|remote| {
previous_refs
.get(*remote)
.is_some_and(|refs| refs.branches.contains_key(&branch))
})
.cloned()
.collect::<Vec<_>>();
if previous_remotes.len() != remote_names.len() {
continue;
}
let mut deleted_remotes = Vec::new();
let mut target_remotes = Vec::new();
let mut changed_remotes = Vec::new();
for remote in &remote_names {
let current = current_refs
.get(remote)
.and_then(|refs| refs.branches.get(&branch));
let previous = previous_refs
.get(remote)
.and_then(|refs| refs.branches.get(&branch));
match (previous, current) {
(Some(_), None) => deleted_remotes.push(remote.clone()),
(Some(previous), Some(current)) if previous == current => {
target_remotes.push(remote.clone());
}
(Some(_), Some(_)) => {
target_remotes.push(remote.clone());
changed_remotes.push(remote.clone());
}
_ => {}
}
}
if deleted_remotes.is_empty() {
continue;
}
blocked.insert(branch.clone());
if target_remotes.is_empty() {
continue;
}
if changed_remotes.is_empty() {
deletions.push(BranchDeletion {
branch,
deleted_remotes,
target_remotes,
});
} else {
conflicts.push(BranchDeletionConflict {
branch,
deleted_remotes,
changed_remotes,
});
}
}
(deletions, conflicts, blocked)
}
struct RepoRefSyncResult { struct RepoRefSyncResult {
pushed: bool, pushed: bool,
had_conflicts: bool, had_conflicts: bool,
@@ -963,6 +1137,26 @@ fn print_branch_decisions(branches: &[crate::git::BranchDecision]) {
} }
} }
fn print_branch_deletions(deletions: &[BranchDeletion]) {
crate::logln!(
" {} {}",
style("deleted branches").red().bold(),
style(format!("({})", deletions.len())).dim()
);
for deletion in deletions {
crate::logln!(
" {} {}",
style(&deletion.branch).cyan(),
style(format!(
"deleted on {} -> {}",
deletion.deleted_remotes.join("+"),
deletion.target_remotes.join("+")
))
.dim()
);
}
}
fn print_tag_decisions(tags: &[crate::git::TagDecision]) { fn print_tag_decisions(tags: &[crate::git::TagDecision]) {
crate::logln!( crate::logln!(
" {} {}", " {} {}",
@@ -1052,17 +1246,11 @@ mod tests {
let mut refs = BTreeMap::new(); let mut refs = BTreeMap::new();
refs.insert( refs.insert(
"github_alice".to_string(), "github_alice".to_string(),
RemoteRefState { remote_ref_state("abc", &[("main", "111")]),
hash: "abc".to_string(),
refs: 2,
},
); );
refs.insert( refs.insert(
"gitea_alice".to_string(), "gitea_alice".to_string(),
RemoteRefState { remote_ref_state("def", &[("main", "111")]),
hash: "def".to_string(),
refs: 2,
},
); );
let mut state = RefState::default(); let mut state = RefState::default();
state.set_repo("sync-1", "repo-a", refs.clone()); state.set_repo("sync-1", "repo-a", refs.clone());
@@ -1075,10 +1263,7 @@ mod tests {
let mut changed_hash = refs.clone(); let mut changed_hash = refs.clone();
changed_hash.insert( changed_hash.insert(
"github_alice".to_string(), "github_alice".to_string(),
RemoteRefState { remote_ref_state("changed", &[("main", "111")]),
hash: "changed".to_string(),
refs: 2,
},
); );
assert!(!loaded.repo_matches("sync-1", "repo-a", &changed_hash)); assert!(!loaded.repo_matches("sync-1", "repo-a", &changed_hash));
@@ -1086,4 +1271,91 @@ mod tests {
missing_remote.remove("gitea_alice"); missing_remote.remove("gitea_alice");
assert!(!loaded.repo_matches("sync-1", "repo-a", &missing_remote)); assert!(!loaded.repo_matches("sync-1", "repo-a", &missing_remote));
} }
#[test]
fn branch_deletion_decisions_propagate_previous_synced_branch_deletion() {
let remotes = test_remotes();
let mut previous = BTreeMap::new();
previous.insert(
"github".to_string(),
remote_ref_state("a", &[("main", "111")]),
);
previous.insert(
"gitea".to_string(),
remote_ref_state("b", &[("main", "111")]),
);
let mut current = BTreeMap::new();
current.insert("github".to_string(), remote_ref_state("c", &[]));
current.insert(
"gitea".to_string(),
remote_ref_state("d", &[("main", "111")]),
);
let (deletions, conflicts, blocked) =
branch_deletion_decisions(&remotes, Some(&previous), &current);
assert!(conflicts.is_empty());
assert!(blocked.contains("main"));
assert_eq!(deletions.len(), 1);
assert_eq!(deletions[0].branch, "main");
assert_eq!(deletions[0].deleted_remotes, vec!["github".to_string()]);
assert_eq!(deletions[0].target_remotes, vec!["gitea".to_string()]);
}
#[test]
fn branch_deletion_decisions_conflict_when_branch_changed_elsewhere() {
let remotes = test_remotes();
let mut previous = BTreeMap::new();
previous.insert(
"github".to_string(),
remote_ref_state("a", &[("main", "111")]),
);
previous.insert(
"gitea".to_string(),
remote_ref_state("b", &[("main", "111")]),
);
let mut current = BTreeMap::new();
current.insert("github".to_string(), remote_ref_state("c", &[]));
current.insert(
"gitea".to_string(),
remote_ref_state("d", &[("main", "222")]),
);
let (deletions, conflicts, blocked) =
branch_deletion_decisions(&remotes, Some(&previous), &current);
assert!(deletions.is_empty());
assert!(blocked.contains("main"));
assert_eq!(conflicts.len(), 1);
assert_eq!(conflicts[0].branch, "main");
assert_eq!(conflicts[0].deleted_remotes, vec!["github".to_string()]);
assert_eq!(conflicts[0].changed_remotes, vec!["gitea".to_string()]);
}
fn remote_ref_state(hash: &str, branches: &[(&str, &str)]) -> RemoteRefState {
RemoteRefState {
hash: hash.to_string(),
refs: branches.len(),
branches: branches
.iter()
.map(|(branch, sha)| ((*branch).to_string(), (*sha).to_string()))
.collect(),
tags: BTreeMap::new(),
}
}
fn test_remotes() -> Vec<RemoteSpec> {
vec![
RemoteSpec {
name: "github".to_string(),
url: "https://github.invalid/alice/repo.git".to_string(),
display: "github:alice:User".to_string(),
},
RemoteSpec {
name: "gitea".to_string(),
url: "https://gitea.invalid/alice/repo.git".to_string(),
display: "gitea:alice:User".to_string(),
},
]
}
} }