diff options
| -rw-r--r-- | CHANGELOG.md | 6 | ||||
| -rw-r--r-- | src/bin/git_remote_nostr/fetch.rs | 65 | ||||
| -rw-r--r-- | src/bin/git_remote_nostr/list.rs | 82 |
3 files changed, 86 insertions, 67 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 2995cd3..b3a93bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md | |||
| @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
| 7 | 7 | ||
| 8 | ## [Unreleased] | 8 | ## [Unreleased] |
| 9 | 9 | ||
| 10 | ## [2.4.1] - 2026-04-22 | ||
| 11 | |||
| 12 | ### Fixed | ||
| 13 | |||
| 14 | - `fatal` errors during clone/fetch when an open PR's git data isn't available on the repository's specified git servers | ||
| 15 | |||
| 10 | ## [2.4.0] - 2026-04-10 | 16 | ## [2.4.0] - 2026-04-10 |
| 11 | 17 | ||
| 12 | ### Added | 18 | ### Added |
diff --git a/src/bin/git_remote_nostr/fetch.rs b/src/bin/git_remote_nostr/fetch.rs index dd80f0f..7847777 100644 --- a/src/bin/git_remote_nostr/fetch.rs +++ b/src/bin/git_remote_nostr/fetch.rs | |||
| @@ -1,17 +1,11 @@ | |||
| 1 | use core::str; | 1 | use core::str; |
| 2 | use std::{ | 2 | use std::{collections::HashMap, io::Stdin}; |
| 3 | collections::{HashMap, HashSet}, | ||
| 4 | io::Stdin, | ||
| 5 | }; | ||
| 6 | 3 | ||
| 7 | use anyhow::{Context, Result, bail}; | 4 | use anyhow::{Context, Result, bail}; |
| 8 | use ngit::{ | 5 | use ngit::{ |
| 9 | fetch::fetch_from_git_server, | 6 | fetch::fetch_from_git_server, |
| 10 | git::{Repo, RepoActions}, | 7 | git::{Repo, RepoActions}, |
| 11 | git_events::{ | 8 | git_events::{KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE}, |
| 12 | KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE, | ||
| 13 | identify_clone_urls_for_oids_from_pr_pr_update_events, | ||
| 14 | }, | ||
| 15 | login::get_curent_user, | 9 | login::get_curent_user, |
| 16 | repo_ref::{RepoRef, is_grasp_server_in_list}, | 10 | repo_ref::{RepoRef, is_grasp_server_in_list}, |
| 17 | utils::{ | 11 | utils::{ |
| @@ -37,56 +31,29 @@ pub async fn run_fetch( | |||
| 37 | .map(|(_, oid)| oid.clone()) | 31 | .map(|(_, oid)| oid.clone()) |
| 38 | .collect::<Vec<String>>(); | 32 | .collect::<Vec<String>>(); |
| 39 | 33 | ||
| 40 | let pr_oid_clone_url_map = identify_clone_urls_for_oids_from_pr_pr_update_events( | ||
| 41 | fetch_batch.values().collect::<Vec<&String>>(), | ||
| 42 | git_repo, | ||
| 43 | repo_ref, | ||
| 44 | ) | ||
| 45 | .await?; | ||
| 46 | |||
| 47 | let oids_to_fetch_from_git_servers = [ | ||
| 48 | oids_from_state.clone(), | ||
| 49 | pr_oid_clone_url_map | ||
| 50 | .keys() | ||
| 51 | .cloned() | ||
| 52 | .collect::<Vec<String>>(), | ||
| 53 | ] | ||
| 54 | .concat(); | ||
| 55 | |||
| 56 | let git_servers = { | ||
| 57 | let mut seen: HashSet<String> = HashSet::new(); | ||
| 58 | let mut out: Vec<String> = vec![]; | ||
| 59 | for server in &repo_ref.git_server { | ||
| 60 | if seen.insert(server.clone()) { | ||
| 61 | out.push(server.clone()); | ||
| 62 | } | ||
| 63 | } | ||
| 64 | for url in pr_oid_clone_url_map.values().flatten() { | ||
| 65 | if seen.insert(url.clone()) { | ||
| 66 | out.push(url.clone()); | ||
| 67 | } | ||
| 68 | } | ||
| 69 | out | ||
| 70 | }; | ||
| 71 | |||
| 72 | let mut errors = vec![]; | 34 | let mut errors = vec![]; |
| 73 | let term = console::Term::stderr(); | 35 | let term = console::Term::stderr(); |
| 74 | 36 | ||
| 75 | for git_server_url in &git_servers { | 37 | // Only repo git servers are tried here. PR tip OIDs are guaranteed local by the |
| 76 | let oids_to_fetch_from_server = oids_to_fetch_from_git_servers | 38 | // list phase (see get_open_and_draft_proposals_state), so run_fetch only needs |
| 77 | .clone() | 39 | // to resolve state OIDs that may have been missed by the bulk prefetch. |
| 78 | .into_iter() | 40 | // We intentionally do not fall back to git-server URLs in PR event `clone` |
| 41 | // tags; see the note in get_open_and_draft_proposals_state for the | ||
| 42 | // rationale. | ||
| 43 | for git_server_url in &repo_ref.git_server { | ||
| 44 | let missing = oids_from_state | ||
| 45 | .iter() | ||
| 79 | .filter(|oid| !git_repo.does_commit_exist(oid).unwrap_or(false)) | 46 | .filter(|oid| !git_repo.does_commit_exist(oid).unwrap_or(false)) |
| 47 | .cloned() | ||
| 80 | .collect::<Vec<String>>(); | 48 | .collect::<Vec<String>>(); |
| 81 | 49 | ||
| 82 | if oids_to_fetch_from_server.is_empty() { | 50 | if missing.is_empty() { |
| 83 | continue; | 51 | break; |
| 84 | } | 52 | } |
| 85 | 53 | ||
| 86 | let term = console::Term::stderr(); | ||
| 87 | if let Err(error) = fetch_from_git_server( | 54 | if let Err(error) = fetch_from_git_server( |
| 88 | git_repo, | 55 | git_repo, |
| 89 | &oids_from_state, | 56 | &missing, |
| 90 | git_server_url, | 57 | git_server_url, |
| 91 | &repo_ref.to_nostr_git_url(&None), | 58 | &repo_ref.to_nostr_git_url(&None), |
| 92 | &term, | 59 | &term, |
| @@ -98,7 +65,7 @@ pub async fn run_fetch( | |||
| 98 | 65 | ||
| 99 | if oids_from_state | 66 | if oids_from_state |
| 100 | .iter() | 67 | .iter() |
| 101 | .any(|oid| !git_repo.does_commit_exist(oid).unwrap()) | 68 | .any(|oid| !git_repo.does_commit_exist(oid).unwrap_or(false)) |
| 102 | && !errors.is_empty() | 69 | && !errors.is_empty() |
| 103 | { | 70 | { |
| 104 | bail!( | 71 | bail!( |
diff --git a/src/bin/git_remote_nostr/list.rs b/src/bin/git_remote_nostr/list.rs index a32ed67..23b3b98 100644 --- a/src/bin/git_remote_nostr/list.rs +++ b/src/bin/git_remote_nostr/list.rs | |||
| @@ -32,13 +32,14 @@ pub async fn run_list( | |||
| 32 | if is_verbose() { | 32 | if is_verbose() { |
| 33 | term.write_line("git servers: listing refs...")?; | 33 | term.write_line("git servers: listing refs...")?; |
| 34 | } | 34 | } |
| 35 | let nostr_git_url = repo_ref.to_nostr_git_url(&None); | ||
| 35 | // nostr_state is passed to list_from_remotes only for the sync-status | 36 | // nostr_state is passed to list_from_remotes only for the sync-status |
| 36 | // display; the actual ref state we advertise is determined below. | 37 | // display; the actual ref state we advertise is determined below. |
| 37 | let remote_states = list_from_remotes( | 38 | let remote_states = list_from_remotes( |
| 38 | &term, | 39 | &term, |
| 39 | git_repo, | 40 | git_repo, |
| 40 | &repo_ref.git_server, | 41 | &repo_ref.git_server, |
| 41 | &repo_ref.to_nostr_git_url(&None), | 42 | &nostr_git_url, |
| 42 | nostr_state.as_ref(), | 43 | nostr_state.as_ref(), |
| 43 | ) | 44 | ) |
| 44 | .await; | 45 | .await; |
| @@ -134,6 +135,7 @@ pub async fn run_list( | |||
| 134 | 135 | ||
| 135 | /// fetches branches and tags from git servers so patch parent commits can be | 136 | /// fetches branches and tags from git servers so patch parent commits can be |
| 136 | /// used to build patches with correct commit ids | 137 | /// used to build patches with correct commit ids |
| 138 | #[allow(clippy::too_many_lines)] | ||
| 137 | async fn get_open_and_draft_proposals_state( | 139 | async fn get_open_and_draft_proposals_state( |
| 138 | term: &console::Term, | 140 | term: &console::Term, |
| 139 | git_repo: &Repo, | 141 | git_repo: &Repo, |
| @@ -156,8 +158,6 @@ async fn get_open_and_draft_proposals_state( | |||
| 156 | .filter(|v| !v.starts_with("ref: ")) | 158 | .filter(|v| !v.starts_with("ref: ")) |
| 157 | .cloned() | 159 | .cloned() |
| 158 | .collect::<Vec<String>>(), | 160 | .collect::<Vec<String>>(), |
| 159 | // TODO we could fetch the oids of Pull Requests and Pull Request Updates to prevent | ||
| 160 | // having repeat fetching during the git remote helper fetch phase | ||
| 161 | git_server_url, | 161 | git_server_url, |
| 162 | &repo_ref.to_nostr_git_url(&None), | 162 | &repo_ref.to_nostr_git_url(&None), |
| 163 | term, | 163 | term, |
| @@ -169,8 +169,62 @@ async fn get_open_and_draft_proposals_state( | |||
| 169 | } | 169 | } |
| 170 | } | 170 | } |
| 171 | 171 | ||
| 172 | let mut state = HashMap::new(); | ||
| 173 | let open_and_draft_proposals = get_open_or_draft_proposals(git_repo, repo_ref).await?; | 172 | let open_and_draft_proposals = get_open_or_draft_proposals(git_repo, repo_ref).await?; |
| 173 | |||
| 174 | // Collect PR/PR-update tip OIDs that are still missing after the bulk prefetch. | ||
| 175 | // We borrow proposals here so we can move them in the state-building loop | ||
| 176 | // below. | ||
| 177 | let mut missing_pr_oids: Vec<String> = open_and_draft_proposals | ||
| 178 | .values() | ||
| 179 | .filter_map(|(_, events)| { | ||
| 180 | events | ||
| 181 | .iter() | ||
| 182 | .find(|e| e.kind.eq(&KIND_PULL_REQUEST) || e.kind.eq(&KIND_PULL_REQUEST_UPDATE)) | ||
| 183 | .and_then(|e| tag_value(e, "c").ok()) | ||
| 184 | }) | ||
| 185 | .filter(|tip| !git_repo.does_commit_exist(tip).unwrap_or(false)) | ||
| 186 | .collect(); | ||
| 187 | |||
| 188 | // For each repo git server, batch-fetch the PR tip OIDs it carries that are | ||
| 189 | // still missing locally. Only OIDs the server has advertised are included in | ||
| 190 | // each batch (avoids all-or-nothing batch-poisoning). We mop up across servers | ||
| 191 | // until all missing OIDs are satisfied or all servers are exhausted. | ||
| 192 | // | ||
| 193 | // NOTE: we intentionally restrict mop-up to the repo's declared git servers | ||
| 194 | // (remote_states) and do NOT try the git-server URL carried in the PR event's | ||
| 195 | // `clone` tag. A PR submitter could include an arbitrary server URL there; | ||
| 196 | // fetching from it unconditionally would let a malicious or slow server | ||
| 197 | // delay every clone/fetch. If we later want to support PR-supplied servers, | ||
| 198 | // it should be opt-in (e.g. an explicit `--include-pr-servers` flag) so | ||
| 199 | // users consciously accept the trust/performance trade-off. PRs whose tip | ||
| 200 | // OID isn't carried by any repo git server will simply not be advertised as | ||
| 201 | // `refs/heads/pr/*` refs; they are still accessible via their patch events. | ||
| 202 | if !missing_pr_oids.is_empty() { | ||
| 203 | for (server_url, (server_state, is_grasp)) in remote_states { | ||
| 204 | let batch: Vec<String> = missing_pr_oids | ||
| 205 | .iter() | ||
| 206 | .filter(|oid| server_state.values().any(|v| v == *oid)) | ||
| 207 | .cloned() | ||
| 208 | .collect(); | ||
| 209 | if batch.is_empty() { | ||
| 210 | continue; | ||
| 211 | } | ||
| 212 | let _ = fetch_from_git_server( | ||
| 213 | git_repo, | ||
| 214 | &batch, | ||
| 215 | server_url, | ||
| 216 | &repo_ref.to_nostr_git_url(&None), | ||
| 217 | term, | ||
| 218 | *is_grasp, | ||
| 219 | ); | ||
| 220 | missing_pr_oids.retain(|oid| !git_repo.does_commit_exist(oid).unwrap_or(false)); | ||
| 221 | if missing_pr_oids.is_empty() { | ||
| 222 | break; | ||
| 223 | } | ||
| 224 | } | ||
| 225 | } | ||
| 226 | |||
| 227 | let mut state = HashMap::new(); | ||
| 174 | let current_user = get_curent_user(git_repo)?; | 228 | let current_user = get_curent_user(git_repo)?; |
| 175 | for (_, (proposal, events_to_apply)) in open_and_draft_proposals { | 229 | for (_, (proposal, events_to_apply)) in open_and_draft_proposals { |
| 176 | if let Ok(cl) = event_to_cover_letter(&proposal) { | 230 | if let Ok(cl) = event_to_cover_letter(&proposal) { |
| @@ -192,17 +246,9 @@ async fn get_open_and_draft_proposals_state( | |||
| 192 | { | 246 | { |
| 193 | match tag_value(pr_or_pr_update, "c") { | 247 | match tag_value(pr_or_pr_update, "c") { |
| 194 | Ok(tip) => { | 248 | Ok(tip) => { |
| 195 | // only list Pull Requests as refs/heads/pr/* if data is commit is | 249 | // Only advertise once confirmed locally available — this |
| 196 | // advertised as tip of a ref on a repo git server or | 250 | // guarantees the subsequent fetch phase can serve the object. |
| 197 | // available locally. Otherwise the standard cmd: | 251 | if git_repo.does_commit_exist(&tip).is_ok_and(|r| r) { |
| 198 | // `git clone nostr://` will fail as it assumes all /refs/heads | ||
| 199 | // returned by list are accessable | ||
| 200 | let tip_oid_is_on_a_repo_git_server = | ||
| 201 | remote_states.iter().any(|(_url, (state, _is_grasp))| { | ||
| 202 | state.iter().any(|(_, oid)| tip == *oid) | ||
| 203 | }) || git_repo.does_commit_exist(&tip).is_ok_and(|r| r); | ||
| 204 | |||
| 205 | if tip_oid_is_on_a_repo_git_server { | ||
| 206 | state.insert(format!("refs/heads/{branch_name}"), tip); | 252 | state.insert(format!("refs/heads/{branch_name}"), tip); |
| 207 | } | 253 | } |
| 208 | } | 254 | } |
| @@ -254,8 +300,8 @@ async fn get_all_proposals_state( | |||
| 254 | let mut state = HashMap::new(); | 300 | let mut state = HashMap::new(); |
| 255 | let all_proposals = get_all_proposals(git_repo, repo_ref).await?; | 301 | let all_proposals = get_all_proposals(git_repo, repo_ref).await?; |
| 256 | let current_user = get_curent_user(git_repo)?; | 302 | let current_user = get_curent_user(git_repo)?; |
| 257 | for (_, (proposal, events_to_apply)) in all_proposals { | 303 | for (proposal, events_to_apply) in all_proposals.values() { |
| 258 | if let Ok(cl) = event_to_cover_letter(&proposal) { | 304 | if let Ok(cl) = event_to_cover_letter(proposal) { |
| 259 | if let Ok(mut branch_name) = cl.get_branch_name_with_pr_prefix_and_shorthand_id() { | 305 | if let Ok(mut branch_name) = cl.get_branch_name_with_pr_prefix_and_shorthand_id() { |
| 260 | branch_name = if let Some(public_key) = current_user { | 306 | branch_name = if let Some(public_key) = current_user { |
| 261 | if proposal.pubkey.eq(&public_key) { | 307 | if proposal.pubkey.eq(&public_key) { |
| @@ -275,7 +321,7 @@ async fn get_all_proposals_state( | |||
| 275 | state.insert(format!("refs/pr/{}/head", proposal.id), tip); | 321 | state.insert(format!("refs/pr/{}/head", proposal.id), tip); |
| 276 | } | 322 | } |
| 277 | } else if let Ok(tip) = | 323 | } else if let Ok(tip) = |
| 278 | make_commits_for_proposal(git_repo, repo_ref, &events_to_apply) | 324 | make_commits_for_proposal(git_repo, repo_ref, events_to_apply) |
| 279 | { | 325 | { |
| 280 | state.insert(format!("refs/{branch_name}"), tip.clone()); | 326 | state.insert(format!("refs/{branch_name}"), tip.clone()); |
| 281 | state.insert(format!("refs/pr/{}/head", proposal.id), tip); | 327 | state.insert(format!("refs/pr/{}/head", proposal.id), tip); |