Skip to content

Commit f2575a8

Browse files
committed
feat(sync): pause .git synchronization during active git operations
- Detect active git operations via rebase, merge, and lock markers - Prevent incoming .git updates when local git is busy - Suppress outgoing .git events during local git operations - Exclude .git from manifest walk during active operations - Avoid ref locking conflicts and transactional state corruption - Ensure per-op failures do not terminate the sync session - Improve internal documentation for event timing and error handling
1 parent b9f1286 commit f2575a8

3 files changed

Lines changed: 120 additions & 25 deletions

File tree

src/peer.rs

Lines changed: 95 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,51 @@ fn tmp_path() -> PathBuf {
179179
dir.join(format!("{}.{}", std::process::id(), nanos))
180180
}
181181

182+
/// True iff `path` (relative to sync root) is `.git` or lies under `.git/`.
183+
/// Cheap path-component check — no filesystem access.
184+
pub fn is_under_git(rel: &Path) -> bool {
185+
rel.components()
186+
.next()
187+
.map(|c| c.as_os_str() == ".git")
188+
.unwrap_or(false)
189+
}
190+
191+
/// True iff the sync root has a git operation in progress that would race
192+
/// with file-level sync of `.git/`. While any of these markers exist, we
193+
/// pause syncing of paths under `.git/` on both walk, push, and apply.
194+
///
195+
/// Why: git treats `.git/` as transactional state. Atomically renaming a
196+
/// ref while we mid-stream a different version of that ref from the peer
197+
/// causes "cannot lock ref" failures and breaks rebase/merge/cherry-pick.
198+
/// Pausing only `.git/` (not the working tree) is correct — your source
199+
/// edits keep syncing, only the VCS metadata is held back until the
200+
/// in-progress operation finishes.
201+
pub fn git_busy(root: &Path) -> bool {
202+
let git_dir = root.join(".git");
203+
// `.git` may not exist, may be a worktree pointer file, or a real dir.
204+
// We only handle the regular-dir case; worktrees are uncommon enough
205+
// that paying for them isn't worth the complexity now.
206+
if !git_dir.is_dir() {
207+
return false;
208+
}
209+
const MARKERS: &[&str] = &[
210+
"rebase-merge",
211+
"rebase-apply",
212+
"MERGE_HEAD",
213+
"CHERRY_PICK_HEAD",
214+
"REVERT_HEAD",
215+
"BISECT_LOG",
216+
"index.lock",
217+
"HEAD.lock",
218+
];
219+
for m in MARKERS {
220+
if git_dir.join(m).exists() {
221+
return true;
222+
}
223+
}
224+
false
225+
}
226+
182227
/// Remove tmp files left over from a previous crashed run.
183228
/// Age-based (> 1 hour) so we don't step on a concurrently-running synx.
184229
/// Cheap; safe to call at startup of both client and agent.
@@ -634,19 +679,15 @@ pub async fn live_loop<R, W>(
634679
compress: bool,
635680
is_client: bool,
636681
ignores: Arc<IgnoreStack>,
637-
// Suppression + Pending are passed in (not constructed here) so any
638-
// entries recorded during the initial sync — i.e. files we just wrote
639-
// to disk — remain in scope when the watcher starts firing events.
640-
// Without this, FSEvents/inotify events for init-sync writes have
641-
// nothing to match in the suppression map and bounce back to the peer
642-
// as spurious "local changes".
682+
// Carried over from the init-sync apply phase: marks for every file we
683+
// wrote during initial sync stay valid here (TTL 60s) so the watcher's
684+
// FSEvents/inotify echoes for those writes are filtered, not bounced
685+
// back to the peer as spurious local changes.
643686
suppress: Suppression,
644687
pending: Pending,
645688
// The watcher is spawned BEFORE the initial sync so events for files
646-
// the user modifies during the walk/exchange window are captured and
647-
// replayed after init sync completes (otherwise they're lost: notify
648-
// uses "events since now" at registration time). Caller owns spawning
649-
// it; we receive the live channel + keepalive here.
689+
// the user modifies during the walk/exchange/apply window aren't lost.
690+
// Caller owns spawning; we receive the live channel + keepalive here.
650691
watcher_handle: watcher::WatcherHandle,
651692
) -> Result<()>
652693
where
@@ -698,8 +739,8 @@ where
698739
Some(Ok(Message::Bye)) => break,
699740
Some(Ok(m)) => {
700741
// Per-op apply errors are non-fatal — log and
701-
// continue. Connection-level failures show up as
702-
// Err(...) from the reader task (next arm).
742+
// continue. Connection-level failures appear as
743+
// Err from the reader task (the next arm).
703744
if let Err(e) = handle_incoming(&root, m, &suppress, &pending, compress, &writer, apply_remote, Some(&ignores), is_client).await {
704745
tracing::warn!("apply failed: {}", e);
705746
let mut w = writer.lock().await;
@@ -753,6 +794,33 @@ where
753794
// forwarded over SSH to the same terminal, so any logs there would just
754795
// duplicate the client's transcript.
755796
let log_event = is_client;
797+
// If git is mid-operation locally, refuse to apply any change under
798+
// `.git/`. Otherwise the peer (who may NOT be busy) would clobber our
799+
// in-progress rebase/merge state and break ref locking.
800+
let busy = git_busy(root);
801+
let path_of = |m: &Message| -> Option<PathBuf> {
802+
match m {
803+
Message::FileData { entry, .. } => Some(entry.path.clone()),
804+
Message::FileStart { entry, .. } => Some(entry.path.clone()),
805+
Message::FileChunk { path, .. } => Some(path.clone()),
806+
Message::FileEnd { path } => Some(path.clone()),
807+
Message::MkDir { entry } => Some(entry.path.clone()),
808+
Message::MkSymlink { entry } => Some(entry.path.clone()),
809+
Message::Delete { path } => Some(path.clone()),
810+
Message::Rename { from: _, to } => Some(to.clone()),
811+
Message::Delta { entry, .. } => Some(entry.path.clone()),
812+
Message::Touch { path, .. } => Some(path.clone()),
813+
_ => None,
814+
}
815+
};
816+
if busy {
817+
if let Some(p) = path_of(&msg) {
818+
if is_under_git(&p) {
819+
tracing::debug!("git busy: skip incoming for {}", p.display());
820+
return Ok(());
821+
}
822+
}
823+
}
756824
match msg {
757825
Message::FileData { entry, content } => {
758826
if !apply_remote {
@@ -1015,7 +1083,7 @@ where
10151083
Message::Pong => {}
10161084
// Per-op error reported by the peer (type conflict, perm denied,
10171085
// etc.). Log and keep the session alive — bailing would just
1018-
// trigger a reconnect that would repeat the same failure.
1086+
// trigger a reconnect that repeats the same failure.
10191087
Message::Error(e) => tracing::warn!("peer error: {e}"),
10201088
other => {
10211089
tracing::debug!(
@@ -1092,11 +1160,25 @@ where
10921160
// forwarded over SSH stderr and duplicate every transfer line.
10931161
let log_event = is_client;
10941162
let events = coalesce(events);
1163+
// Once per batch: if git is mid-operation, suppress every event that
1164+
// touches .git/. Prevents partial rebase/merge state from leaking to
1165+
// the peer where it would race with the peer's own ref updates.
1166+
let pause_git = git_busy(root);
10951167
for ev in events {
10961168
if suppress.is_echo(root, &ev) {
10971169
tracing::trace!("echo suppressed: {:?}", ev);
10981170
continue;
10991171
}
1172+
if pause_git {
1173+
let key = match &ev {
1174+
FsEvent::Created(p) | FsEvent::Modified(p) | FsEvent::Removed(p) => p,
1175+
FsEvent::Renamed { to, .. } => to,
1176+
};
1177+
if is_under_git(key) {
1178+
tracing::debug!("git busy: skip event {:?}", ev);
1179+
continue;
1180+
}
1181+
}
11001182

11011183
match ev {
11021184
FsEvent::Created(p) | FsEvent::Modified(p) => {

src/sync.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,12 @@ async fn run_inner(
151151
writer: Arc<Mutex<BufWriter<ChildStdin>>>,
152152
mut child: tokio::process::Child,
153153
) -> Result<()> {
154-
// Suppression and the watcher are constructed BEFORE the walk so that
155-
// any user edits during the walk / manifest exchange / init-sync apply
156-
// window are captured by the watcher (notify uses "events since now"
157-
// at registration time — events before then are lost). We hold them in
158-
// their channel until init sync completes, then drain and forward.
154+
// Spawn the watcher BEFORE the walk so events for files the user
155+
// modifies during walk / manifest exchange / init-sync apply aren't
156+
// lost (notify uses "events since now" at registration; events from
157+
// before are never delivered). Events queue in the unbounded channel
158+
// until init sync completes, then we drain + replay them with the
159+
// suppress map populated.
159160
let suppress = Suppression::default();
160161
let pending = Pending::default();
161162
let mut watcher_handle = watcher::spawn(local_root.clone(), suppress.clone())?;
@@ -465,12 +466,11 @@ async fn run_inner(
465466
});
466467

467468
// Receive: apply incoming server responses until peer SyncDone.
469+
// Per-op apply errors during init sync are non-fatal — bailing here
470+
// would tear down the session and the reconnect loop would hit the
471+
// same error forever.
468472
let mut bytes_recv: u64 = 0;
469473
let mut received_files: u64 = 0;
470-
// Per-op apply errors during init-sync are non-fatal. A single bad file
471-
// (type conflict, permission denied) should not tear down the whole
472-
// session — the outer reconnect loop would just retry it and hit the
473-
// exact same failure forever. Log and continue.
474474
let warn_apply = |path: &std::path::Path, e: &anyhow::Error| {
475475
tracing::warn!("apply {} failed: {}", path.display(), e);
476476
};
@@ -603,9 +603,9 @@ async fn run_inner(
603603
}
604604
}
605605
Message::SyncDone => break,
606-
// Remote reported a per-op failure (file conflict, perm denied,
607-
// etc.). Log and continue — bailing here would just trigger a
608-
// reconnect that hits the same error.
606+
// Remote reported a per-op failure (type conflict, perm denied,
607+
// git busy on its side). Log and continue — bailing here would
608+
// tear down the session and retry forever.
609609
Message::Error(e) => tracing::warn!("remote: {e}"),
610610
Message::Bye => return Ok(()),
611611
_ => tracing::debug!("ignored msg in init-sync recv"),

src/walker.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,18 @@ pub fn build_entry(
122122
/// Walk `root` in parallel (multi-threaded via `ignore`), returning a
123123
/// fully-hashed manifest sorted by path. The cache is updated in-place; the
124124
/// caller should call `HashCache::save` afterwards.
125+
///
126+
/// If git is mid-operation (rebase / merge / cherry-pick / pending ref
127+
/// lock — see `peer::git_busy`), `.git/` is excluded from the walk. The
128+
/// manifest exchange and diff plan won't see VCS metadata in this state,
129+
/// so no sync of `.git/` is attempted until git finishes.
125130
pub fn walk_manifest(root: &Path, cache: &Arc<Mutex<HashCache>>) -> Result<Vec<Entry>> {
126131
let (tx, rx) = std::sync::mpsc::channel::<Entry>();
127132
let root_arc = Arc::new(root.to_path_buf());
133+
let skip_git = crate::peer::git_busy(root);
134+
if skip_git {
135+
tracing::info!("git operation in progress — excluding .git/ from this walk");
136+
}
128137

129138
build_walker(root).build_parallel().run(|| {
130139
let tx = tx.clone();
@@ -146,6 +155,10 @@ pub fn walk_manifest(root: &Path, cache: &Arc<Mutex<HashCache>>) -> Result<Vec<E
146155
Ok(r) => r.to_path_buf(),
147156
Err(_) => return WalkState::Continue,
148157
};
158+
if skip_git && crate::peer::is_under_git(&rel) {
159+
// Skip the .git entry itself AND all descendants.
160+
return WalkState::Skip;
161+
}
149162
match build_entry(&root, &rel, Some(&*cache)) {
150163
Ok(Some(e)) => {
151164
let _ = tx.send(e);

0 commit comments

Comments
 (0)