From 39ba96051cc8eb51cc86ee0c707384512f5bea26 Mon Sep 17 00:00:00 2001 From: Azalea Date: Thu, 7 May 2026 02:58:43 +0000 Subject: [PATCH] [F] Fix branch deletion awareness --- README.md | 2 +- src/git.rs | 101 ++++++++++++++++- src/sync.rs | 308 +++++++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 389 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index dba7c70..219ce2b 100644 --- a/README.md +++ b/README.md @@ -160,7 +160,7 @@ Branch conflict handling is intentionally conservative: - 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. -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. diff --git a/src/git.rs b/src/git.rs index 3ad4861..ea2b6ae 100644 --- a/src/git.rs +++ b/src/git.rs @@ -19,6 +19,8 @@ pub struct RemoteSpec { pub struct RemoteRefSnapshot { pub hash: String, pub refs: usize, + pub branches: BTreeMap, + pub tags: BTreeMap, } #[derive(Clone, Debug)] @@ -35,6 +37,13 @@ pub struct BranchConflict { pub tips: Vec<(String, String)>, } +#[derive(Clone, Debug)] +pub struct BranchDeletion { + pub branch: String, + pub deleted_remotes: Vec, + pub target_remotes: Vec, +} + #[derive(Clone, Debug)] pub struct TagDecision { pub tag: String, @@ -114,8 +123,17 @@ impl GitMirror { remote: &RemoteSpec, expected: &RemoteRefSnapshot, ) -> Result { + Ok(self + .cached_remote_ref_snapshot(remote)? + .is_some_and(|snapshot| &snapshot == expected)) + } + + pub fn cached_remote_ref_snapshot( + &self, + remote: &RemoteSpec, + ) -> Result> { if !self.path.exists() || self.dry_run { - return Ok(false); + return Ok(None); } let branches = self.remote_branches(&remote.name)?; let tags = self.remote_tags(&remote.name)?; @@ -126,8 +144,7 @@ impl GitMirror { for (tag, sha) in tags { refs.push(format!("{sha}\trefs/tags/{tag}")); } - let snapshot = snapshot_from_refs(refs); - Ok(&snapshot == expected) + Ok(Some(snapshot_from_refs(refs))) } pub fn branch_decisions( @@ -302,6 +319,30 @@ impl GitMirror { 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> { let output = Command::new("git") .arg("--git-dir") @@ -465,10 +506,24 @@ pub fn ls_remote_refs(remote: &RemoteSpec, redactor: &Redactor) -> Result) -> RemoteRefSnapshot { 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 { hash: stable_ref_hash(&refs), 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] fn tag_decisions_mirror_matching_or_missing_tags_and_skip_divergent_tags() { 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(current_dir: Option<&Path>, args: [&str; N]) { diff --git a/src/sync.rs b/src/sync.rs index 143e4d7..a3c94bd 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -11,8 +11,8 @@ use serde::{Deserialize, Serialize}; use crate::config::{Config, EndpointConfig, MirrorConfig, default_work_dir, validate_config}; use crate::git::{ - GitMirror, Redactor, RemoteRefSnapshot, RemoteSpec, is_disabled_repository_error, - ls_remote_refs, safe_remote_name, + BranchDeletion, GitMirror, Redactor, RemoteRefSnapshot, RemoteSpec, + is_disabled_repository_error, ls_remote_refs, safe_remote_name, }; use crate::logging; use crate::provider::{EndpointRepo, ProviderClient, repos_by_name}; @@ -219,6 +219,10 @@ fn failure_state_path(work_dir: &Path) -> PathBuf { struct RemoteRefState { hash: String, refs: usize, + #[serde(default)] + branches: BTreeMap, + #[serde(default)] + tags: BTreeMap, } impl From for RemoteRefState { @@ -226,6 +230,8 @@ impl From for RemoteRefState { Self { hash: value.hash, refs: value.refs, + branches: value.branches, + tags: value.tags, } } } @@ -235,6 +241,8 @@ impl From<&RemoteRefState> for RemoteRefSnapshot { Self { hash: value.hash.clone(), refs: value.refs, + branches: value.branches.clone(), + tags: value.tags.clone(), } } } @@ -261,6 +269,10 @@ impl RefState { .or_default() .insert(repo.to_string(), refs); } + + fn repo(&self, group: &str, repo: &str) -> Option<&BTreeMap> { + self.repos.get(group).and_then(|repos| repos.get(repo)) + } } fn load_ref_state(work_dir: &Path) -> Result { @@ -687,6 +699,7 @@ fn sync_repo( let mirror_repo = GitMirror::open(path, context.redactor.clone(), context.dry_run)?; mirror_repo.configure_remotes(&initial_remotes)?; + let cached_ref_state = cached_ref_state(&mirror_repo, &initial_remotes)?; if !context.dry_run && all_endpoints_present && 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 { let refs = if result.pushed { let Some(refs) = check_remote_refs(context, repo_name, &remotes)? else { @@ -802,6 +822,29 @@ fn cached_refs_match( Ok(true) } +fn cached_ref_state( + mirror_repo: &GitMirror, + remotes: &[RemoteSpec], +) -> Result>> { + 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>, +) -> Option<&BTreeMap> { + refs.filter(|refs| { + refs.values() + .any(|remote| !remote.branches.is_empty() || !remote.tags.is_empty()) + }) +} + fn check_remote_refs( context: &RepoSyncContext<'_>, repo_name: &str, @@ -868,14 +911,35 @@ fn push_repo_refs( context: &RepoSyncContext<'_>, mirror_repo: &GitMirror, remotes: &[RemoteSpec], + previous_refs: Option<&BTreeMap>, + current_refs: &BTreeMap, ) -> Result { + 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 had_branch_conflicts = !conflicts.is_empty(); let branches_to_push = branches .into_iter() + .filter(|branch| !blocked_branches.contains(&branch.branch)) .filter(|branch| !branch.target_remotes.is_empty()) .collect::>(); + let mut had_branch_conflicts = false; for conflict in conflicts { + if blocked_branches.contains(&conflict.branch) { + continue; + } + had_branch_conflicts = true; let details = conflict .tips .iter() @@ -914,15 +978,33 @@ fn push_repo_refs( } 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!( " {} branches and tags already match all endpoints", style("up-to-date").green().bold() ); return Ok(RepoRefSyncResult { 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() { print_branch_decisions(&branches_to_push); mirror_repo.push_branches(remotes, &branches_to_push, context.allow_force)?; @@ -933,10 +1015,102 @@ fn push_repo_refs( } Ok(RepoRefSyncResult { 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, + changed_remotes: Vec, +} + +fn branch_deletion_decisions( + remotes: &[RemoteSpec], + previous_refs: Option<&BTreeMap>, + current_refs: &BTreeMap, +) -> ( + Vec, + Vec, + BTreeSet, +) { + 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::>(); + 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::>(); + 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 { pushed: 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]) { crate::logln!( " {} {}", @@ -1052,17 +1246,11 @@ mod tests { let mut refs = BTreeMap::new(); refs.insert( "github_alice".to_string(), - RemoteRefState { - hash: "abc".to_string(), - refs: 2, - }, + remote_ref_state("abc", &[("main", "111")]), ); refs.insert( "gitea_alice".to_string(), - RemoteRefState { - hash: "def".to_string(), - refs: 2, - }, + remote_ref_state("def", &[("main", "111")]), ); let mut state = RefState::default(); state.set_repo("sync-1", "repo-a", refs.clone()); @@ -1075,10 +1263,7 @@ mod tests { let mut changed_hash = refs.clone(); changed_hash.insert( "github_alice".to_string(), - RemoteRefState { - hash: "changed".to_string(), - refs: 2, - }, + remote_ref_state("changed", &[("main", "111")]), ); assert!(!loaded.repo_matches("sync-1", "repo-a", &changed_hash)); @@ -1086,4 +1271,91 @@ mod tests { missing_remote.remove("gitea_alice"); 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), ¤t); + + 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), ¤t); + + 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 { + 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(), + }, + ] + } }