mirror of
https://relay.ngit.dev/npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/ngit-grasp.git
synced 2026-10-05 15:08:24 +00:00
refactor(grasp06): remove empty /prs/ repositories through one helper
Four runtime paths each carried their own copy of "no push in flight and no refs left, so remove the repository": push completion, discarding a mismatched scoped placeholder, purgatory expiry and PR deletion. The copies differed only in log wording. Move the rule into remove_idle_empty_repo, for callers that hold the path mutex, and remove_prs_repo_if_empty, for callers that do not. Later changes to the rule then apply everywhere. Behaviour is unchanged. PR deletion still removes the then-empty submitter directory itself. The startup scan runs before the server accepts traffic and keeps its own lock-free loop. Validation: nix develop -c cargo test --test grasp06_pr_hosting --test purgatory passed 67 and 66 tests; the purgatory, grasp06 and deletion unit tests passed; cargo clippy --all-targets -- -D warnings was clean. Assisted-by: Claude Fable 5.1
This commit is contained in:
+41
-16
@@ -436,26 +436,51 @@ fn ensure_repo_initialised(
|
||||
fn finish_prs_receive_pack(state: &PrsPathState, repo_path: &Path) {
|
||||
let _g = state.mu.lock().expect("prs path mutex poisoned");
|
||||
state.in_flight.fetch_sub(1, Ordering::Relaxed);
|
||||
if state.in_flight.load(Ordering::Relaxed) == 0 {
|
||||
if let Ok(refs) = list_refs(repo_path) {
|
||||
if refs.is_empty() {
|
||||
if let Err(e) = std::fs::remove_dir_all(repo_path) {
|
||||
warn!(
|
||||
"/prs/ receive-pack: failed to clean up empty repo {}: {}",
|
||||
repo_path.display(),
|
||||
e
|
||||
);
|
||||
} else {
|
||||
debug!(
|
||||
"/prs/ receive-pack: removed empty repo {} (no refs after push)",
|
||||
repo_path.display()
|
||||
);
|
||||
}
|
||||
}
|
||||
remove_idle_empty_repo(state, repo_path);
|
||||
}
|
||||
|
||||
/// Remove a `/prs/` repository that has no refs and no push in flight, so
|
||||
/// abandoned repositories do not accumulate. Returns whether it was removed.
|
||||
///
|
||||
/// The caller holds `state.mu`. That mutex also gates `in_flight` updates, so
|
||||
/// a repository is never removed while a push is being received.
|
||||
pub(crate) fn remove_idle_empty_repo(state: &PrsPathState, repo_path: &Path) -> bool {
|
||||
if state.in_flight.load(Ordering::Relaxed) != 0
|
||||
|| !matches!(list_refs(repo_path), Ok(refs) if refs.is_empty())
|
||||
{
|
||||
return false;
|
||||
}
|
||||
match std::fs::remove_dir_all(repo_path) {
|
||||
Ok(()) => {
|
||||
debug!(repo = %repo_path.display(), "Removed zero-ref /prs/ repository");
|
||||
true
|
||||
}
|
||||
Err(error) => {
|
||||
warn!(
|
||||
repo = %repo_path.display(),
|
||||
%error,
|
||||
"Failed to remove zero-ref /prs/ repository"
|
||||
);
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// [`remove_idle_empty_repo`] for callers that do not already hold the path
|
||||
/// mutex. Paths outside `/prs/` are left alone.
|
||||
pub fn remove_prs_repo_if_empty(
|
||||
locks: &RepoInitLocks,
|
||||
git_data_path: &Path,
|
||||
repo_path: &Path,
|
||||
) -> bool {
|
||||
if !crate::grasp06::paths::is_prs_repo_path(repo_path, git_data_path) {
|
||||
return false;
|
||||
}
|
||||
let state = path_state(locks, repo_path);
|
||||
let _g = state.mu.lock().expect("prs path mutex poisoned");
|
||||
remove_idle_empty_repo(&state, repo_path)
|
||||
}
|
||||
|
||||
/// Own the live `/prs/` receive-pack after the request handler has returned its
|
||||
/// streaming response.
|
||||
///
|
||||
|
||||
@@ -93,34 +93,16 @@ impl DeletionPolicy {
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn cleanup_zero_ref_grasp06_repo_if_idle(&self, repo_path: &PathBuf) {
|
||||
if !crate::grasp06::paths::is_prs_repo_path(repo_path, &self.ctx.git_data_path) {
|
||||
return;
|
||||
}
|
||||
|
||||
let state = crate::grasp06::receive::path_state(&self.ctx.repo_init_locks, repo_path);
|
||||
let _guard = state.mu.lock().expect("prs path mutex poisoned");
|
||||
|
||||
if state.in_flight.load(std::sync::atomic::Ordering::Relaxed) != 0 {
|
||||
return;
|
||||
}
|
||||
|
||||
let is_zero_ref = matches!(git::list_refs(repo_path), Ok(refs) if refs.is_empty());
|
||||
if !is_zero_ref {
|
||||
return;
|
||||
}
|
||||
|
||||
if let Err(e) = std::fs::remove_dir_all(repo_path) {
|
||||
tracing::warn!(
|
||||
repo = %repo_path.display(),
|
||||
error = %e,
|
||||
"Failed to remove zero-ref /prs/ repo while processing deletion request"
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
if let Some(parent) = repo_path.parent() {
|
||||
let _ = std::fs::remove_dir(parent);
|
||||
fn cleanup_zero_ref_grasp06_repo_if_idle(&self, repo_path: &std::path::Path) {
|
||||
let removed = crate::grasp06::receive::remove_prs_repo_if_empty(
|
||||
&self.ctx.repo_init_locks,
|
||||
&self.ctx.git_data_path,
|
||||
repo_path,
|
||||
);
|
||||
if removed {
|
||||
if let Some(parent) = repo_path.parent() {
|
||||
let _ = std::fs::remove_dir(parent);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -179,29 +179,8 @@ impl PrEventPolicy {
|
||||
error = %e,
|
||||
"Failed to delete /prs/ ref while discarding scoped placeholder",
|
||||
);
|
||||
} else if state.in_flight.load(std::sync::atomic::Ordering::Relaxed) == 0
|
||||
&& matches!(
|
||||
crate::git::list_refs(&prs_repo),
|
||||
Ok(refs) if refs.is_empty()
|
||||
)
|
||||
{
|
||||
// No refs left and no push in flight — the
|
||||
// repo is now an empty husk. Drop the bare
|
||||
// dir so /prs/ doesn't accumulate abandoned
|
||||
// repos. Equivalent to the cleanup the
|
||||
// receive handler does on push completion.
|
||||
if let Err(e) = std::fs::remove_dir_all(&prs_repo) {
|
||||
tracing::warn!(
|
||||
repo = %prs_repo.display(),
|
||||
error = %e,
|
||||
"Failed to remove zero-ref /prs/ repo after discarding scoped placeholder",
|
||||
);
|
||||
} else {
|
||||
tracing::debug!(
|
||||
repo = %prs_repo.display(),
|
||||
"Removed zero-ref /prs/ repo after discarding scoped placeholder",
|
||||
);
|
||||
}
|
||||
} else {
|
||||
crate::grasp06::receive::remove_idle_empty_repo(&state, &prs_repo);
|
||||
}
|
||||
}
|
||||
self.ctx.purgatory.remove_pr(&event_id);
|
||||
|
||||
+2
-18
@@ -1589,24 +1589,8 @@ impl Purgatory {
|
||||
error = %e,
|
||||
"Failed to delete dangling /prs/ ref during purgatory expiry",
|
||||
);
|
||||
} else if state.in_flight.load(std::sync::atomic::Ordering::Relaxed) == 0
|
||||
&& matches!(
|
||||
crate::git::list_refs(&repo_path),
|
||||
Ok(refs) if refs.is_empty()
|
||||
)
|
||||
{
|
||||
if let Err(e) = std::fs::remove_dir_all(&repo_path) {
|
||||
tracing::warn!(
|
||||
repo = %repo_path.display(),
|
||||
error = %e,
|
||||
"Failed to remove zero-ref /prs/ repo during purgatory expiry",
|
||||
);
|
||||
} else {
|
||||
tracing::debug!(
|
||||
repo = %repo_path.display(),
|
||||
"Removed zero-ref /prs/ repo during purgatory expiry",
|
||||
);
|
||||
}
|
||||
} else {
|
||||
crate::grasp06::receive::remove_idle_empty_repo(&state, &repo_path);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user