From e6bb9effa194fe63b5e969c090dbe6e93f13d312 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Wed, 18 Feb 2026 12:38:37 +0000 Subject: feat: support PR-format proposals in ngit apply Instead of erroring when a proposal uses PR format (kind 1618/1619), ngit apply now fetches the tip commit from git servers, determines the base commit via the merge-base tag or by walking ahead of main, generates patch text for each commit using git2, and applies via git am. Also fixes a bug where clone-tag server hints were silently skipped in the fetch fallback, and refactors the git-am invocation into a shared helper to avoid duplication between the patch and PR code paths. --- src/bin/ngit/sub_commands/apply.rs | 169 ++++++++++++++++--- src/bin/ngit/sub_commands/init.rs | 321 ++++++++++++++++++++----------------- 2 files changed, 321 insertions(+), 169 deletions(-) (limited to 'src/bin/ngit') diff --git a/src/bin/ngit/sub_commands/apply.rs b/src/bin/ngit/sub_commands/apply.rs index 4b13975..3700c37 100644 --- a/src/bin/ngit/sub_commands/apply.rs +++ b/src/bin/ngit/sub_commands/apply.rs @@ -1,4 +1,5 @@ use std::{ + collections::HashSet, io::Write, process::{Command, Stdio}, time::Duration, @@ -8,7 +9,13 @@ use anyhow::{Context, Result, bail}; use indicatif::{ProgressBar, ProgressStyle}; use ngit::{ client::get_all_proposal_patch_pr_pr_update_events_from_cache, - git_events::get_pr_tip_event_or_most_recent_patch_with_ancestors, + fetch::fetch_from_git_server, + git::str_to_sha1, + git_events::{ + KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE, + get_pr_tip_event_or_most_recent_patch_with_ancestors, tag_value, + }, + repo_ref::{RepoRef, is_grasp_server_in_list}, }; use nostr::nips::nip19::Nip19; use nostr_sdk::{EventId, FromBech32}; @@ -123,17 +130,15 @@ pub async fn launch(id: &str, stdout: bool, offline: bool) -> Result<()> { let patches = get_pr_tip_event_or_most_recent_patch_with_ancestors(commits_events.clone()) .context("failed to find any PR or patch events on this proposal")?; - if patches.iter().any(|e| { - [ - ngit::git_events::KIND_PULL_REQUEST, - ngit::git_events::KIND_PULL_REQUEST_UPDATE, - ] - .contains(&e.kind) - }) { - bail!( - "this proposal uses PR format (not patches). Use `ngit checkout {}` instead.", - event_id.to_hex() - ); + if patches + .iter() + .any(|e| [KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE].contains(&e.kind)) + { + let pr_event = patches + .first() + .context("patch chain should contain at least one event")?; + apply_pr(&git_repo, &repo_ref, pr_event, stdout)?; + return Ok(()); } if stdout { @@ -159,16 +164,124 @@ fn parse_event_id(id: &str) -> Result { bail!("invalid event-id or nevent: {id}") } -fn output_patches_to_stdout(mut patches: Vec) { - patches.reverse(); - for patch in patches { - print!("{}\n\n", patch.content); +fn fetch_oid_for_pr( + oid: &str, + git_repo: &Repo, + repo_ref: &RepoRef, + pr_event: &nostr::Event, +) -> Result<()> { + let git_servers = { + let mut seen: HashSet = HashSet::new(); + let mut out: Vec = vec![]; + for tag in pr_event.tags.as_slice() { + if tag.kind().eq(&nostr::event::TagKind::Clone) { + for clone_url in tag.as_slice().iter().skip(1) { + seen.insert(clone_url.clone()); + out.push(clone_url.clone()); + } + } + } + for server in &repo_ref.git_server { + if seen.insert(server.clone()) { + out.push(server.clone()); + } + } + out + }; + + let term = console::Term::stderr(); + for git_server_url in &git_servers { + if fetch_from_git_server( + git_repo, + &[oid.to_string()], + git_server_url, + &repo_ref.to_nostr_git_url(&None), + &term, + is_grasp_server_in_list(git_server_url, &repo_ref.grasp_servers()), + ) + .is_ok() + { + return Ok(()); + } + } + if !git_repo.does_commit_exist(oid)? { + bail!( + "cannot find proposal git data from proposal git server hint or repository git servers" + ); } + Ok(()) } -fn launch_git_am_with_patches(mut patches: Vec) -> Result<()> { +fn apply_pr( + git_repo: &Repo, + repo_ref: &RepoRef, + pr_event: &nostr::Event, + stdout: bool, +) -> Result<()> { + let tip_oid = tag_value(pr_event, "c").context("PR event is missing 'c' (tip commit) tag")?; + + // Ensure the tip commit is available locally + if !git_repo.does_commit_exist(&tip_oid)? { + fetch_oid_for_pr(&tip_oid, git_repo, repo_ref, pr_event)?; + } + + let tip = str_to_sha1(&tip_oid).context("invalid tip commit OID in PR event")?; + + // Determine the base commit: prefer the merge-base tag, fall back to + // computing the divergence point from main/master. + let base = if let Ok(merge_base_oid) = tag_value(pr_event, "merge-base") { + str_to_sha1(&merge_base_oid).context("invalid merge-base OID in PR event")? + } else { + let (_, main_tip) = git_repo + .get_main_or_master_branch() + .context("could not determine main branch to compute PR base commit")?; + let (ahead, _behind) = git_repo + .get_commits_ahead_behind(&main_tip, &tip) + .context("failed to compute commits between main and PR tip")?; + // ahead is youngest-first; the last element is the oldest PR commit, + // whose parent is the effective base. + let oldest_pr_commit = ahead + .last() + .context("no commits found between main and PR tip")?; + git_repo + .get_commit_parent(oldest_pr_commit) + .context("failed to get parent of the oldest PR commit")? + }; + + // Collect commits from base..tip (youngest-first from get_commits_ahead_behind) + let (commits_youngest_first, _) = git_repo + .get_commits_ahead_behind(&base, &tip) + .context("failed to enumerate commits in PR")?; + + if commits_youngest_first.is_empty() { + bail!("no commits found between base and PR tip"); + } + + let total = commits_youngest_first.len() as u64; + + // Generate patches oldest-first + let mut patch_texts: Vec = Vec::with_capacity(commits_youngest_first.len()); + for (i, commit) in commits_youngest_first.iter().rev().enumerate() { + let series_count = Some((i as u64 + 1, total)); + let patch = git_repo + .make_patch_from_commit(commit, &series_count) + .with_context(|| format!("failed to generate patch for commit {commit}"))?; + patch_texts.push(patch); + } + + if stdout { + for patch in &patch_texts { + print!("{patch}\n\n"); + } + } else { + apply_patch_texts(patch_texts)?; + } + + Ok(()) +} + +fn apply_patch_texts(patch_texts: Vec) -> Result<()> { println!("applying to current branch with `git am`"); - patches.reverse(); let mut am = std::process::Command::new("git") .arg("am") @@ -183,15 +296,25 @@ fn launch_git_am_with_patches(mut patches: Vec) -> Result<()> { .as_mut() .context("git am process failed to take stdin")?; - for patch in patches { + for patch in patch_texts { stdin - .write(format!("{}\n\n", patch.content).as_bytes()) + .write(format!("{patch}\n\n").as_bytes()) .context("failed to write patch content into git am stdin buffer")?; } stdin.flush()?; - let output = am - .wait_with_output() + am.wait_with_output() .context("failed to read git am stdout")?; - print!("{:?}", output.stdout); Ok(()) } + +fn output_patches_to_stdout(mut patches: Vec) { + patches.reverse(); + for patch in patches { + print!("{}\n\n", patch.content); + } +} + +fn launch_git_am_with_patches(mut patches: Vec) -> Result<()> { + patches.reverse(); + apply_patch_texts(patches.into_iter().map(|p| p.content).collect()) +} diff --git a/src/bin/ngit/sub_commands/init.rs b/src/bin/ngit/sub_commands/init.rs index 1b577ed..75306d1 100644 --- a/src/bin/ngit/sub_commands/init.rs +++ b/src/bin/ngit/sub_commands/init.rs @@ -274,8 +274,7 @@ fn apply_grasp_infrastructure( public_key: &PublicKey, identifier: &str, ) -> Result<()> { - let mut grasp_relay_insert_idx = 0; - for grasp_server in grasp_servers { + for (grasp_relay_insert_idx, grasp_server) in grasp_servers.iter().enumerate() { // Always add grasp-derived clone URL let clone_url = format_grasp_server_url_as_clone_url(grasp_server, public_key, identifier)?; @@ -312,7 +311,6 @@ fn apply_grasp_infrastructure( if !relays.contains(&relay_url) { relays.insert(grasp_relay_insert_idx, relay_url); } - grasp_relay_insert_idx += 1; } Ok(()) } @@ -1713,15 +1711,90 @@ struct DeferredServerFinish { message: String, } -/// Coordinates the delayed reveal of per-server detail bars. -/// Bars that finish before the expand timer fires store their final -/// style+message here. The timer applies them all at reveal time so -/// every bar — completed or still waiting — appears in the expanded view. struct ServerRevealState { revealed: AtomicBool, deferred: Mutex>, } +struct PollContext { + timeout_secs: u64, + total: u64, + ready_count: Arc, + spinner_pb: ProgressBar, + reveal_state: Arc, +} + +fn create_server_bars( + clone_urls: &[String], + detail_multi: &MultiProgress, +) -> Vec { + let waiting_style = ProgressStyle::with_template(" {spinner} {msg}") + .unwrap() + .tick_chars("⠁⠂⠄⡀⢀⠠⠐⠈"); + clone_urls + .iter() + .map(|url| { + let name = url + .trim_start_matches("https://") + .trim_start_matches("http://") + .to_string(); + detail_multi.add( + ProgressBar::new_spinner() + .with_style(waiting_style.clone()) + .with_message( + console::style(format!("{name} - waiting")) + .for_stderr() + .dim() + .to_string(), + ), + ) + }) + .collect() +} + +fn spawn_expand_timer( + expand_delay_ms: u64, + spinner_pb: ProgressBar, + detail_multi: MultiProgress, + heading_bar: ProgressBar, + reveal_state: Arc, + server_bars: Vec, +) -> tokio::task::JoinHandle<()> { + tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(expand_delay_ms)).await; + spinner_pb.finish_and_clear(); + detail_multi.set_draw_target(ProgressDrawTarget::stderr()); + heading_bar.finish_with_message("waiting for servers to create bare git repo..."); + let mut deferred = reveal_state.deferred.lock().unwrap(); + reveal_state.revealed.store(true, Ordering::Release); + for df in deferred.drain(..) { + df.bar.set_style(df.style); + df.bar.finish_with_message(df.message); + } + for bar in &server_bars { + if !bar.is_finished() { + bar.enable_steady_tick(Duration::from_millis(100)); + } + } + }) +} + +fn finalize_spinner( + all_ready: bool, + spinner_pb: &ProgressBar, + final_ready: u64, + total: u64, +) { + if all_ready { + spinner_pb.finish_and_clear(); + } else { + spinner_pb.set_style(ProgressStyle::with_template("{msg}").unwrap()); + spinner_pb.finish_with_message(format!( + "timed out waiting for servers to create bare git repo ({final_ready}/{total} - complete), proceeding anyway" + )); + } +} + fn finish_server_bar( bar: &ProgressBar, style: ProgressStyle, @@ -1745,6 +1818,78 @@ fn finish_server_bar( } } +async fn poll_single_server( + url: String, + git_repo_path: std::path::PathBuf, + bar: ProgressBar, + ctx: Arc, +) -> bool { + let poll_interval = Duration::from_millis(500); + let deadline = tokio::time::Instant::now() + Duration::from_secs(ctx.timeout_secs); + let mut ready = false; + loop { + let is_ready = tokio::task::spawn_blocking({ + let url = url.clone(); + let path = git_repo_path.clone(); + move || check_git_server_ready(&path, &url) + }) + .await + .unwrap_or(false); + + if is_ready { + ready = true; + break; + } + + if tokio::time::Instant::now() >= deadline { + break; + } + + tokio::time::sleep(poll_interval).await; + } + + let count = if ready { + ctx.ready_count.fetch_add(1, Ordering::Relaxed) + 1 + } else { + ctx.ready_count.load(Ordering::Relaxed) + }; + + ctx.spinner_pb.set_message(format!( + "waiting for servers to create bare git repo... ({count}/{total} - complete)", + total = ctx.total + )); + + let name = url + .trim_start_matches("https://") + .trim_start_matches("http://") + .to_string(); + if ready { + let style = ProgressStyle::with_template(&format!( + " {} {{msg}}", + console::style("✔").for_stderr().green() + )) + .unwrap(); + let msg = console::style(format!("{name} - ready")) + .for_stderr() + .green() + .to_string(); + finish_server_bar(&bar, style, msg, &ctx.reveal_state); + } else { + let style = ProgressStyle::with_template(&format!( + " {} {{msg}}", + console::style("✘").for_stderr().red() + )) + .unwrap(); + let msg = console::style(format!("{name} - timeout")) + .for_stderr() + .red() + .to_string(); + finish_server_bar(&bar, style, msg, &ctx.reveal_state); + } + + ready +} + /// Poll grasp servers in parallel until all are ready or timeout is reached. /// /// Shows a concise spinner with `x/y - complete` progress. After 5s without @@ -1799,166 +1944,50 @@ async fn wait_for_grasp_servers( deferred: Mutex::new(Vec::new()), }); - // Per-server spinner bars (added to hidden detail_multi) - let waiting_style = ProgressStyle::with_template(" {spinner} {msg}") - .unwrap() - .tick_chars("⠁⠂⠄⡀⢀⠠⠐⠈"); - let server_bars: Vec = clone_urls - .iter() - .map(|url| { - let name = url - .trim_start_matches("https://") - .trim_start_matches("http://") - .to_string(); - detail_multi.add( - ProgressBar::new_spinner() - .with_style(waiting_style.clone()) - .with_message( - console::style(format!("{name} - waiting")) - .for_stderr() - .dim() - .to_string(), - ), - ) - }) - .collect(); + let server_bars = create_server_bars(&clone_urls, &detail_multi); - // Background timer: after expand_delay_ms reveal the detail view and - // flush any bars that already finished (the BarRevealState pattern). - let detail_multi_for_timer = detail_multi.clone(); - let spinner_for_timer = spinner_pb.clone(); - let reveal_state_for_timer = reveal_state.clone(); - let server_bars_for_timer = server_bars.clone(); - let heading_bar_for_timer = heading_bar.clone(); - let timer_handle = tokio::spawn(async move { - tokio::time::sleep(Duration::from_millis(expand_delay_ms)).await; - spinner_for_timer.finish_and_clear(); - detail_multi_for_timer.set_draw_target(ProgressDrawTarget::stderr()); - // Show the heading in the expanded view. - heading_bar_for_timer.finish_with_message("waiting for servers to create bare git repo..."); - // Lock deferred list, mark revealed, and flush bars that already - // finished. Must hold the lock across the revealed.store so that - // finish_server_bar cannot push after the drain. - let mut deferred = reveal_state_for_timer.deferred.lock().unwrap(); - reveal_state_for_timer - .revealed - .store(true, Ordering::Release); - for df in deferred.drain(..) { - df.bar.set_style(df.style); - df.bar.finish_with_message(df.message); - } - // Kick still-waiting bars into drawing by enabling their tick. - for bar in &server_bars_for_timer { - if !bar.is_finished() { - bar.enable_steady_tick(Duration::from_millis(100)); - } - } - }); + let timer_handle = spawn_expand_timer( + expand_delay_ms, + spinner_pb.clone(), + detail_multi.clone(), + heading_bar, + reveal_state.clone(), + server_bars.clone(), + ); // Poll each server in parallel let git_repo_path = git_repo.get_path()?.to_path_buf(); + let poll_ctx = Arc::new(PollContext { + timeout_secs, + total, + ready_count: ready_count.clone(), + spinner_pb: spinner_pb.clone(), + reveal_state: reveal_state.clone(), + }); let futures: Vec<_> = clone_urls .iter() .enumerate() .map(|(i, url)| { - let url = url.clone(); - let ready_count = ready_count.clone(); - let spinner_pb = spinner_pb.clone(); - let bar = server_bars[i].clone(); - let git_repo_path = git_repo_path.clone(); - let reveal_state = reveal_state.clone(); - async move { - let poll_interval = Duration::from_millis(500); - let deadline = tokio::time::Instant::now() + Duration::from_secs(timeout_secs); - let mut ready = false; - loop { - let is_ready = tokio::task::spawn_blocking({ - let url = url.clone(); - let path = git_repo_path.clone(); - move || check_git_server_ready(&path, &url) - }) - .await - .unwrap_or(false); - - if is_ready { - ready = true; - break; - } - - if tokio::time::Instant::now() >= deadline { - break; - } - - tokio::time::sleep(poll_interval).await; - } - - let count = if ready { - ready_count.fetch_add(1, Ordering::Relaxed) + 1 - } else { - ready_count.load(Ordering::Relaxed) - }; - - // Update spinner message - spinner_pb.set_message(format!( - "waiting for servers to create bare git repo... ({count}/{total} - complete)" - )); - - // Finish per-server bar (deferred if detail not yet visible) - let name = url - .trim_start_matches("https://") - .trim_start_matches("http://") - .to_string(); - if ready { - let style = ProgressStyle::with_template(&format!( - " {} {{msg}}", - console::style("✔").for_stderr().green() - )) - .unwrap(); - let msg = console::style(format!("{name} - ready")) - .for_stderr() - .green() - .to_string(); - finish_server_bar(&bar, style, msg, &reveal_state); - } else { - let style = ProgressStyle::with_template(&format!( - " {} {{msg}}", - console::style("✘").for_stderr().red() - )) - .unwrap(); - let msg = console::style(format!("{name} - timeout")) - .for_stderr() - .red() - .to_string(); - finish_server_bar(&bar, style, msg, &reveal_state); - } - - ready - } + poll_single_server( + url.clone(), + git_repo_path.clone(), + server_bars[i].clone(), + poll_ctx.clone(), + ) }) .collect(); let results = join_all(futures).await; let final_ready = ready_count.load(Ordering::Relaxed); - // Cancel the expand timer if it hasn't fired yet. timer_handle.abort(); - // If detail view was revealed, clear the detail bars. if reveal_state.revealed.load(Ordering::Acquire) { let _ = detail_multi.clear(); } let all_ready = results.iter().all(|&r| r); - if all_ready { - // Success — erase the spinner line entirely, leave nothing behind. - spinner_pb.finish_and_clear(); - } else { - // Partial timeout — leave a message so the user knows we proceeded. - spinner_pb.set_style(ProgressStyle::with_template("{msg}").unwrap()); - spinner_pb.finish_with_message(format!( - "timed out waiting for servers to create bare git repo ({final_ready}/{total} - complete), proceeding anyway" - )); - } + finalize_spinner(all_ready, &spinner_pb, final_ready, total); Ok(()) } -- cgit v1.2.3