From 3eb2354edb8e76428625d5645e110c30aa1ccc2a Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 18 Jul 2025 11:56:15 +0100 Subject: feat(pr): list PR and PR updates remote will list the refs under `pr/*` namespace. `ngit list` will display in the list of open / draft proposals. it won't yet fetch the related oids to enable fetching or checking out the branch. --- src/bin/git_remote_nostr/list.rs | 56 ++++++++++++++++++++++++++++++++------- src/bin/git_remote_nostr/utils.rs | 25 ++++++++++------- 2 files changed, 63 insertions(+), 18 deletions(-) (limited to 'src/bin/git_remote_nostr') diff --git a/src/bin/git_remote_nostr/list.rs b/src/bin/git_remote_nostr/list.rs index b9fb0c0..7bdf170 100644 --- a/src/bin/git_remote_nostr/list.rs +++ b/src/bin/git_remote_nostr/list.rs @@ -11,7 +11,7 @@ use ngit::{ self, nostr_url::{CloneUrl, NostrUrlDecoded, ServerProtocol}, }, - git_events::event_to_cover_letter, + git_events::{KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE, event_to_cover_letter, tag_value}, login::get_curent_user, repo_ref::{self, is_grasp_server}, }; @@ -122,6 +122,16 @@ async fn get_open_and_draft_proposals_state( // without trusting commit_id we must apply each patch which requires the oid of // the parent so we much do a fetch + + // As we are fetching from git servers we mighgt as well get oids from pull + // request too + // TODO get Pull Request and Pull Request Update Events add these to + // refs/nostr/ + // TODO prepare PRs and PRS oids to try and fetch from repo servers that are or + // clone urls in PR/update event we are using anyway. TODO after we tried + // and failed to get them from these server we should fallback to fetch them + // from listed clone urls in PR/update but not during list, only during fetch + for (git_server_url, (oids_from_git_servers, is_grasp_server)) in remote_states { if fetch_from_git_server( git_repo, @@ -144,7 +154,7 @@ async fn get_open_and_draft_proposals_state( let mut state = HashMap::new(); let open_and_draft_proposals = get_open_or_draft_proposals(git_repo, repo_ref).await?; let current_user = get_curent_user(git_repo)?; - for (_, (proposal, patches)) in open_and_draft_proposals { + for (_, (proposal, events_to_apply)) in open_and_draft_proposals { if let Ok(cl) = event_to_cover_letter(&proposal) { if let Ok(mut branch_name) = cl.get_branch_name_with_pr_prefix_and_shorthand_id() { branch_name = if let Some(public_key) = current_user { @@ -156,15 +166,43 @@ async fn get_open_and_draft_proposals_state( } else { branch_name }; - match make_commits_for_proposal(git_repo, repo_ref, &patches) { - Ok(tip) => { - state.insert(format!("refs/heads/{branch_name}"), tip); + // if events_to_apply contains a PR or PR Update event it should be the only + // event in the Vec + if let Some(pr_or_pr_update) = events_to_apply + .iter() + .find(|e| e.kind.eq(&KIND_PULL_REQUEST) || e.kind.eq(&KIND_PULL_REQUEST_UPDATE)) + { + match tag_value(pr_or_pr_update, "c") { + Ok(tip) => { + state.insert(format!("refs/heads/{branch_name}"), tip); + } + Err(_) => { + let _ = term.write_line( + format!( + "WARNING: failed to fetch branch {branch_name} error: {} event poorly formatted", + if pr_or_pr_update.kind.eq(&KIND_PULL_REQUEST) { + "PR" + } else { + "PR update" + } + ) + .as_str(), + ); + } } - Err(error) => { - let _ = term.write_line( - format!("WARNING: failed to fetch branch {branch_name} error: {error}") + } else { + match make_commits_for_proposal(git_repo, repo_ref, &events_to_apply) { + Ok(tip) => { + state.insert(format!("refs/heads/{branch_name}"), tip); + } + Err(error) => { + let _ = term.write_line( + format!( + "WARNING: failed to fetch branch {branch_name} error: {error}" + ) .as_str(), - ); + ); + } } } } diff --git a/src/bin/git_remote_nostr/utils.rs b/src/bin/git_remote_nostr/utils.rs index dc75872..d0b4e7e 100644 --- a/src/bin/git_remote_nostr/utils.rs +++ b/src/bin/git_remote_nostr/utils.rs @@ -10,7 +10,7 @@ use anyhow::{Context, Result, bail}; use git2::Repository; use ngit::{ client::{ - get_all_proposal_patch_events_from_cache, get_events_from_local_cache, + get_all_proposal_patch_pr_pr_update_events_from_cache, get_events_from_local_cache, get_proposals_and_revisions_from_cache, }, git::{ @@ -18,7 +18,7 @@ use ngit::{ nostr_url::{CloneUrl, NostrUrlDecoded, ServerProtocol}, }, git_events::{ - event_is_revision_root, get_most_recent_patch_with_ancestors, + event_is_revision_root, get_pr_tip_event_or_most_recent_patch_with_ancestors, is_event_proposal_root_for_branch, status_kinds, }, repo_ref::RepoRef, @@ -140,12 +140,15 @@ pub async fn get_open_or_draft_proposals( Kind::GitStatusOpen }; if [Kind::GitStatusOpen, Kind::GitStatusDraft].contains(&status) { - if let Ok(commits_events) = - get_all_proposal_patch_events_from_cache(git_repo_path, repo_ref, &proposal.id) - .await + if let Ok(commits_events) = get_all_proposal_patch_pr_pr_update_events_from_cache( + git_repo_path, + repo_ref, + &proposal.id, + ) + .await { if let Ok(most_recent_proposal_patch_chain) = - get_most_recent_patch_with_ancestors(commits_events.clone()) + get_pr_tip_event_or_most_recent_patch_with_ancestors(commits_events.clone()) { open_or_draft_proposals .insert(proposal.id, (proposal, most_recent_proposal_patch_chain)); @@ -172,11 +175,15 @@ pub async fn get_all_proposals( let mut all_proposals = HashMap::new(); for proposal in proposals { - if let Ok(commits_events) = - get_all_proposal_patch_events_from_cache(git_repo_path, repo_ref, &proposal.id).await + if let Ok(commits_events) = get_all_proposal_patch_pr_pr_update_events_from_cache( + git_repo_path, + repo_ref, + &proposal.id, + ) + .await { if let Ok(most_recent_proposal_patch_chain) = - get_most_recent_patch_with_ancestors(commits_events.clone()) + get_pr_tip_event_or_most_recent_patch_with_ancestors(commits_events.clone()) { all_proposals.insert(proposal.id, (proposal, most_recent_proposal_patch_chain)); } -- cgit v1.2.3 From a3d4c8eaa263f4adb174ac81c4248fa200e1857e Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 18 Jul 2025 17:33:11 +0100 Subject: feat(pr): fetch pr and pr updates from clone urls we try and get them from clone urls of repo and fallback to those specified by contributor --- src/bin/git_remote_nostr/fetch.rs | 105 +++++++++++++++++++++++++++++++++----- src/bin/ngit/sub_commands/list.rs | 6 ++- src/lib/client.rs | 10 +++- src/lib/git_events.rs | 13 +++++ 4 files changed, 118 insertions(+), 16 deletions(-) (limited to 'src/bin/git_remote_nostr') diff --git a/src/bin/git_remote_nostr/fetch.rs b/src/bin/git_remote_nostr/fetch.rs index b191850..f3d4362 100644 --- a/src/bin/git_remote_nostr/fetch.rs +++ b/src/bin/git_remote_nostr/fetch.rs @@ -1,6 +1,6 @@ use core::str; use std::{ - collections::HashMap, + collections::{HashMap, HashSet}, io::Stdin, sync::{Arc, Mutex}, time::Instant, @@ -16,7 +16,7 @@ use ngit::{ nostr_url::{CloneUrl, NostrUrlDecoded, ServerProtocol}, utils::check_ssh_keys, }, - git_events::tag_value, + git_events::{KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE, tag_value}, login::get_curent_user, repo_ref::{RepoRef, is_grasp_server}, }; @@ -37,38 +37,78 @@ pub async fn run_fetch( ) -> Result<()> { let mut fetch_batch = get_oids_from_fetch_batch(stdin, oid, refstr)?; - let oids_from_git_servers = fetch_batch + let oids_from_state = fetch_batch .iter() .filter(|(refstr, _)| !refstr.contains("refs/heads/pr/")) .map(|(_, oid)| oid.clone()) .collect::>(); + let pr_oid_clone_url_map = identify_clone_urls_for_oids_from_pr_pr_update_events( + fetch_batch.values().collect::>(), + git_repo, + repo_ref, + ) + .await?; + + let oids_to_fetch_from_git_servers = [ + oids_from_state.clone(), + pr_oid_clone_url_map + .keys() + .cloned() + .collect::>(), + ] + .concat(); + + let git_servers = { + let mut seen: HashSet = HashSet::new(); + let mut out: Vec = vec![]; + for server in &repo_ref.git_server { + if seen.insert(server.clone()) { + out.push(server.clone()); + } + } + for url in pr_oid_clone_url_map.values().flatten() { + if seen.insert(url.clone()) { + out.push(url.clone()); + } + } + out + }; + let mut errors = vec![]; let term = console::Term::stderr(); - for git_server_url in &repo_ref.git_server { + for git_server_url in &git_servers { + let oids_to_fetch_from_server = oids_to_fetch_from_git_servers + .clone() + .into_iter() + .filter(|oid| !git_repo.does_commit_exist(oid).unwrap_or(false)) + .collect::>(); + + if oids_to_fetch_from_server.is_empty() { + continue; + } + let term = console::Term::stderr(); if let Err(error) = fetch_from_git_server( git_repo, - &oids_from_git_servers, + &oids_from_state, git_server_url, &repo_ref.to_nostr_git_url(&None), &term, is_grasp_server(git_server_url, &repo_ref.grasp_servers()), ) { errors.push(error); - } else { - break; } } - if oids_from_git_servers + if oids_from_state .iter() .any(|oid| !git_repo.does_commit_exist(oid).unwrap()) && !errors.is_empty() { bail!( - "fetch: failed to fetch objects in nostr state event from:\r\n{}", + "fetch: failed to fetch objects from:\r\n{}", errors .iter() .map(|e| format!(" - {e}")) @@ -79,12 +119,43 @@ pub async fn run_fetch( fetch_batch.retain(|refstr, _| refstr.contains("refs/heads/pr/")); - fetch_open_or_draft_proposals(git_repo, &term, repo_ref, &fetch_batch).await?; + fetch_open_or_draft_proposals_from_patches(git_repo, &term, repo_ref, &fetch_batch).await?; + // TODO fetch_open_or_draft_proposals just needs to do it for patches term.flush()?; println!(); Ok(()) } +async fn identify_clone_urls_for_oids_from_pr_pr_update_events( + oids: Vec<&String>, + git_repo: &Repo, + repo_ref: &RepoRef, +) -> Result>> { + let mut map: HashMap> = HashMap::new(); + + let open_and_draft_proposals = get_open_or_draft_proposals(git_repo, repo_ref).await?; + + for (_, (_, events)) in open_and_draft_proposals { + for event in events { + if [KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE].contains(&event.kind) { + if let Ok(c) = tag_value(&event, "c") { + if oids.contains(&&c) { + for tag in event.tags.as_slice() { + if tag.kind().eq(&nostr::event::TagKind::Clone) { + for clone_url in tag.as_slice().iter().skip(1) { + map.entry(c.clone()).or_default().push(clone_url.clone()); + } + } + } + } + } + } + } + } + + Ok(map) +} + pub fn make_commits_for_proposal( git_repo: &Repo, repo_ref: &RepoRef, @@ -128,7 +199,7 @@ pub fn make_commits_for_proposal( Ok(tip_commit_id) } -async fn fetch_open_or_draft_proposals( +async fn fetch_open_or_draft_proposals_from_patches( git_repo: &Repo, term: &console::Term, repo_ref: &RepoRef, @@ -140,12 +211,19 @@ async fn fetch_open_or_draft_proposals( let current_user = get_curent_user(git_repo)?; for refstr in proposal_refs.keys() { - if let Some((_, (_, patches))) = find_proposal_and_patches_by_branch_name( + if let Some((_, (_, events_to_apply))) = find_proposal_and_patches_by_branch_name( refstr, &open_and_draft_proposals, current_user.as_ref(), ) { - if let Err(error) = make_commits_for_proposal(git_repo, repo_ref, patches) { + if events_to_apply + .iter() + .any(|e| e.kind.eq(&KIND_PULL_REQUEST) || e.kind.eq(&KIND_PULL_REQUEST_UPDATE)) + { + // do nothing - we fetch these oids as part of run_fetch + } else if let Err(error) = + make_commits_for_proposal(git_repo, repo_ref, events_to_apply) + { term.write_line( format!("WARNING: failed to create branch for {refstr}, error: {error}",) .as_str(), @@ -429,6 +507,7 @@ fn fetch_from_git_server_url( remote_callbacks.credentials(auth.credentials(&git_config)); } fetch_options.remote_callbacks(remote_callbacks); + git_server_remote.download(oids, Some(&mut fetch_options))?; git_server_remote.disconnect()?; diff --git a/src/bin/ngit/sub_commands/list.rs b/src/bin/ngit/sub_commands/list.rs index a90b28e..9e35b33 100644 --- a/src/bin/ngit/sub_commands/list.rs +++ b/src/bin/ngit/sub_commands/list.rs @@ -219,7 +219,11 @@ pub async fn launch() -> Result<()> { PromptChoiceParms::default() .with_prompt("this is new PR event kind which ngit doesnt yet support") .with_default(0) - .with_choices(vec!["back to proposals".to_string()]), + .with_choices(vec![ + // TODO enable checkout by fetching oids, creating / updating branch and + // checking out + "back to proposals".to_string(), + ]), )? { 0 => continue, _ => { diff --git a/src/lib/client.rs b/src/lib/client.rs index 1f3b08c..091d68d 100644 --- a/src/lib/client.rs +++ b/src/lib/client.rs @@ -49,7 +49,8 @@ use crate::{ git::{Repo, RepoActions, get_git_config_item}, git_events::{ KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE, event_is_cover_letter, - event_is_patch_set_root, event_is_revision_root, status_kinds, + event_is_patch_set_root, event_is_revision_root, event_is_valid_pr_or_pr_update, + status_kinds, }, login::{get_likely_logged_in_user, user::get_user_ref_from_cache}, repo_ref::RepoRef, @@ -1824,6 +1825,7 @@ pub async fn get_proposals_and_revisions_from_cache( .await? .iter() .filter(|e| event_is_patch_set_root(e) || e.kind.eq(&KIND_PULL_REQUEST)) + .filter(|e| e.kind.eq(&Kind::GitPatch) || event_is_valid_pr_or_pr_update(e)) .cloned() .collect::>(); proposals.sort_by_key(|e| e.created_at); @@ -1874,7 +1876,11 @@ pub async fn get_all_proposal_patch_pr_pr_update_events_from_cache( .iter() .copied() .collect(); - commit_events.retain(|e| permissioned_users.contains(&e.pubkey)); + + commit_events.retain(|e| { + permissioned_users.contains(&e.pubkey) + && (e.kind.eq(&Kind::GitPatch) || event_is_valid_pr_or_pr_update(e)) + }); let revision_roots: HashSet = commit_events .iter() diff --git a/src/lib/git_events.rs b/src/lib/git_events.rs index 7b25daf..09ec040 100644 --- a/src/lib/git_events.rs +++ b/src/lib/git_events.rs @@ -90,6 +90,19 @@ pub fn patch_supports_commit_ids(event: &Event) -> bool { .any(|t| !t.as_slice().is_empty() && t.as_slice()[0].eq("commit-pgp-sig")) } +pub fn event_is_valid_pr_or_pr_update(event: &Event) -> bool { + [KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE].contains(&event.kind) + && event.tags.iter().any(|t| { + t.as_slice().len().gt(&1) + && t.as_slice()[0].eq("c") + && git2::Oid::from_str(&t.as_slice()[1]).is_ok() + }) + && event + .tags + .iter() + .any(|t| t.as_slice().len().gt(&1) && t.as_slice()[0].eq("clone")) +} + #[allow(clippy::too_many_arguments)] #[allow(clippy::too_many_lines)] pub async fn generate_patch_event( -- cgit v1.2.3 From f4e1df4c718a3755ffe50e99946996729f3504e9 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Tue, 22 Jul 2025 17:14:21 +0100 Subject: feat(pr): generate pr event > oversized patch but only for new proposals --- src/bin/git_remote_nostr/push.rs | 126 ++++++++++++++++++++++++++++++++----- src/lib/client.rs | 26 +++++++- src/lib/git_events.rs | 130 +++++++++++++++++++++++++++++++++------ 3 files changed, 247 insertions(+), 35 deletions(-) (limited to 'src/bin/git_remote_nostr') diff --git a/src/bin/git_remote_nostr/push.rs b/src/bin/git_remote_nostr/push.rs index 9ff8af0..b9e8571 100644 --- a/src/bin/git_remote_nostr/push.rs +++ b/src/bin/git_remote_nostr/push.rs @@ -12,12 +12,13 @@ use client::{get_events_from_local_cache, get_state_from_cache, send_events, sig use console::Term; use git::{RepoActions, sha1_to_oid}; use git_events::{ - generate_cover_letter_and_patch_events, generate_patch_event, get_commit_id_from_patch, + generate_cover_letter_and_patch_events, generate_patch_event, generate_unsigned_pr_event, + get_commit_id_from_patch, }; use git2::{Oid, Repository}; use ngit::{ cli_interactor::count_lines_per_msg_vec, - client::{self, get_event_from_cache_by_id}, + client::{self, get_event_from_cache_by_id, sign_draft_event}, git::{ self, nostr_url::{CloneUrl, NostrUrlDecoded}, @@ -25,10 +26,10 @@ use ngit::{ }, git_events::{self, event_to_cover_letter, get_event_root}, login::{self, user::UserRef}, - repo_ref::{self, get_repo_config_from_yaml, is_grasp_server}, + repo_ref::{self, get_repo_config_from_yaml, is_grasp_server, normalize_grasp_server_url}, repo_state, }; -use nostr::nips::nip10::Marker; +use nostr::{event::UnsignedEvent, nips::nip10::Marker}; use nostr_sdk::{ Event, EventBuilder, EventId, Kind, NostrSigner, PublicKey, RelayUrl, Tag, TagStandard, hashes::sha1::Hash as Sha1Hash, @@ -404,18 +405,11 @@ async fn process_proposal_refspecs( let (mut ahead, _) = git_repo.get_commits_ahead_behind(&main_tip, &tip_of_pushed_branch)?; ahead.reverse(); - for patch in generate_cover_letter_and_patch_events( - None, - git_repo, - &ahead, - signer, - repo_ref, - &None, - &[], - ) - .await? + for event in + generate_patches_or_pr_event(git_repo, repo_ref, &ahead, user_ref, signer, term) + .await? { - events.push(patch); + events.push(event); } } } @@ -423,6 +417,108 @@ async fn process_proposal_refspecs( Ok((events, rejected_proposal_refspecs)) } +async fn generate_patches_or_pr_event( + git_repo: &Repo, + repo_ref: &RepoRef, + ahead: &[Sha1Hash], + user_ref: &UserRef, + signer: &Arc, + term: &Term, +) -> Result> { + let mut events: Vec = vec![]; + let use_pr = ahead.iter().any(|commit| { + if let Ok(patch) = git_repo.make_patch_from_commit(commit, &None) { + patch.len() + > ((65 // max recomended patch event size specified in nip34 in kb + // allownace for nostr event wrapper (id, pubkey, tags, sig) + - 1) * 1024) + } else { + true + } + }); + + if use_pr { + let repo_grasps = repo_ref.grasp_servers(); + let repo_grasp_clone_urls = repo_ref + .git_server + .iter() + .filter(|s| is_grasp_server(s, &repo_grasps)); + + let mut unsigned_pr_event: Option = None; + let mut failed_clone_urls = vec![]; + for clone_url in repo_grasp_clone_urls { + let mut draft_pr_event = if let Some(ref unsigned_pr_event) = unsigned_pr_event { + unsigned_pr_event.clone() + } else { + generate_unsigned_pr_event( + git_repo, + repo_ref, + &user_ref.public_key, + ahead.first().context("no commits to push")?, + &[clone_url], + &[], + )? + }; + + let refspec = format!( + "{}:refs/nostr/{}", + ahead.first().unwrap(), + draft_pr_event.id() + ); + + if let Err(error) = push_to_remote_url(git_repo, clone_url, &[refspec], term) { + failed_clone_urls.push(clone_url); + term.write_line( + format!( + "push: error sending commit data to {}: {error}", + normalize_grasp_server_url(clone_url)? + ) + .as_str(), + )?; + } else { + term.write_line( + format!( + "push: commit data sent to {}", + normalize_grasp_server_url(clone_url)? + ) + .as_str(), + )?; + unsigned_pr_event = Some(draft_pr_event); + } + } + if unsigned_pr_event.is_none() { + // TODO get fallback grasp servers that aren't in repo_grasps cycle + // through until one succeeds TODO create personal-fork + // announcement with grasp servers and push, after a few seconds + // push ref/nostr/eventid. if one success break out of + // for loop and continue + } + if let Some(unsigned_pr_event) = unsigned_pr_event { + let pr_event = + sign_draft_event(unsigned_pr_event, signer, "Pull Request".to_string()).await?; + events.push(pr_event); + } else { + bail!("could not find a grasp server that accepts the Pull Request refs"); + } + } else { + for patch in generate_cover_letter_and_patch_events( + None, + git_repo, + ahead, + signer, + repo_ref, + &None, + &[], + ) + .await? + { + events.push(patch); + } + } + + Ok(events) +} + fn push_to_remote( git_repo: &Repo, git_server_url: &str, diff --git a/src/lib/client.rs b/src/lib/client.rs index 091d68d..3fe2b57 100644 --- a/src/lib/client.rs +++ b/src/lib/client.rs @@ -31,7 +31,7 @@ use indicatif::{MultiProgress, ProgressBar, ProgressDrawTarget, ProgressState, P use mockall::*; use nostr::{ Event, - event::{TagKind, TagStandard}, + event::{TagKind, TagStandard, UnsignedEvent}, filter::Alphabet, nips::{nip01::Coordinate, nip19::Nip19Coordinate}, signer::SignerBackend, @@ -814,6 +814,30 @@ pub async fn sign_event( } } +pub async fn sign_draft_event( + draft_event: UnsignedEvent, + signer: &Arc, + description: String, +) -> Result { + if signer.backend() == SignerBackend::NostrConnect { + let term = console::Term::stderr(); + term.write_line(&format!( + "signing event ({description}) with remote signer..." + ))?; + let event = signer + .sign_event(draft_event) + .await + .context("failed to sign event")?; + term.clear_last_lines(1)?; + Ok(event) + } else { + signer + .sign_event(draft_event) + .await + .context("failed to sign event") + } +} + pub async fn fetch_public_key(signer: &Arc) -> Result { if signer.backend() == SignerBackend::NostrConnect { let term = console::Term::stderr(); diff --git a/src/lib/git_events.rs b/src/lib/git_events.rs index 09ec040..86b9641 100644 --- a/src/lib/git_events.rs +++ b/src/lib/git_events.rs @@ -1,7 +1,10 @@ use std::{str::FromStr, sync::Arc}; use anyhow::{Context, Result, bail}; -use nostr::nips::{nip01::Coordinate, nip10::Marker, nip19::Nip19}; +use nostr::{ + event::UnsignedEvent, + nips::{nip01::Coordinate, nip10::Marker, nip19::Nip19}, +}; use nostr_sdk::{ Event, EventBuilder, EventId, FromBech32, Kind, NostrSigner, PublicKey, Tag, TagKind, TagStandard, hashes::sha1::Hash as Sha1Hash, @@ -347,6 +350,111 @@ pub fn event_tag_from_nip19_or_hex( } } +pub fn generate_unsigned_pr_event( + git_repo: &Repo, + repo_ref: &RepoRef, + signing_public_key: &PublicKey, + commit: &Sha1Hash, + clone_url_hint: &[&str], + mentions: &[nostr::Tag], +) -> Result { + let title = git_repo.get_commit_message_summary(commit)?; + + let description = { + let mut description = git_repo.get_commit_message(commit)?.trim().to_string(); + if let Some(remaining_description) = description.strip_prefix(&title) { + description = remaining_description.trim().to_string(); + } + description + }; + + let root_commit = git_repo + .get_root_commit() + .context("failed to get root commit of the repository")?; + + Ok(EventBuilder::new(KIND_PULL_REQUEST, description) + .tags( + [ + repo_ref + .maintainers + .iter() + .map(|m| { + Tag::from_standardized(TagStandard::Coordinate { + coordinate: Coordinate { + kind: nostr::Kind::GitRepoAnnouncement, + public_key: *m, + identifier: repo_ref.identifier.to_string(), + }, + relay_url: repo_ref.relays.first().cloned(), + uppercase: false, + }) + }) + .collect::>(), + mentions.to_vec(), + vec![ + Tag::from_standardized(TagStandard::Subject(title.clone())), + Tag::from_standardized(TagStandard::Reference(format!("{root_commit}"))), + Tag::custom( + nostr::TagKind::Custom(std::borrow::Cow::Borrowed("c")), + vec![format!("{commit}")], + ), + Tag::custom( + nostr::TagKind::Custom(std::borrow::Cow::Borrowed("clone")), + clone_url_hint + .iter() + .map(|s| s.to_string()) + .collect::>(), + ), + Tag::custom( + nostr::TagKind::Custom(std::borrow::Cow::Borrowed("alt")), + vec![format!("git Pull Request: {}", title.clone())], + ), + ], + if let Some(branch_name_tag) = make_branch_name_tag_from_check_out_branch(git_repo) + { + vec![branch_name_tag] + } else { + vec![] + }, + repo_ref + .maintainers + .iter() + .map(|pk| Tag::public_key(*pk)) + .collect(), + ] + .concat(), + ) + .build(*signing_public_key)) +} + +fn make_branch_name_tag_from_check_out_branch(git_repo: &Repo) -> Option { + if let Ok(branch_name) = git_repo.get_checked_out_branch_name() { + if !branch_name.eq("main") + && !branch_name.eq("master") + && !branch_name.eq("origin/main") + && !branch_name.eq("origin/master") + { + Some(Tag::custom( + nostr::TagKind::Custom(std::borrow::Cow::Borrowed("branch-name")), + vec![ + if let Some(branch_name) = branch_name.strip_prefix("pr/") { + branch_name.to_string() + } else { + branch_name + } + .chars() + .take(60) + .collect::(), + ], + )) + } else { + None + } + } else { + None + } +} + #[allow(clippy::too_many_lines)] pub async fn generate_cover_letter_and_patch_events( cover_letter_title_description: Option<(String, String)>, @@ -409,24 +517,8 @@ pub async fn generate_cover_letter_and_patch_events( // eventually a prefix will be needed of the event id to stop 2 proposals with the same name colliding // a change like this, or the removal of this tag will require the actual branch name to be tracked // so pulling and pushing still work - if let Ok(branch_name) = git_repo.get_checked_out_branch_name() { - if !branch_name.eq("main") - && !branch_name.eq("master") - && !branch_name.eq("origin/main") - && !branch_name.eq("origin/master") - { - vec![ - Tag::custom( - nostr::TagKind::Custom(std::borrow::Cow::Borrowed("branch-name")), - vec![if let Some(branch_name) = branch_name.strip_prefix("pr/") { - branch_name.to_string() - } else { - branch_name - }.chars().take(60).collect::()], - ), - ] - } - else { vec![] } + if let Some(branch_name_tag) = make_branch_name_tag_from_check_out_branch(git_repo) { + vec![branch_name_tag] } else { vec![] }, -- cgit v1.2.3 From 698b05be2e48b38a4f268bfabc3562e83e0c1363 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Wed, 23 Jul 2025 05:07:03 +0100 Subject: refactor(pr): rename functions to reflect there new role of also pushing prs to git severs --- src/bin/git_remote_nostr/push.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'src/bin/git_remote_nostr') diff --git a/src/bin/git_remote_nostr/push.rs b/src/bin/git_remote_nostr/push.rs index b9e8571..61f4f92 100644 --- a/src/bin/git_remote_nostr/push.rs +++ b/src/bin/git_remote_nostr/push.rs @@ -66,7 +66,7 @@ pub async fn run_push( .cloned() .collect::>(); - let mut git_server_refspecs = refspecs + let mut git_state_refspecs = refspecs .iter() .filter(|r| !r.contains("refs/heads/pr/")) .cloned() @@ -106,12 +106,12 @@ pub async fn run_push( let (rejected_refspecs, remote_refspecs) = create_rejected_refspecs_and_remotes_refspecs( &term, git_repo, - &git_server_refspecs, + &git_state_refspecs, &existing_state, &list_outputs, )?; - git_server_refspecs.retain(|refspec| { + git_state_refspecs.retain(|refspec| { if let Some(rejected) = rejected_refspecs.get(&refspec.to_string()) { let (_, to) = refspec_to_from_to(refspec).unwrap(); println!("error {to} {} out of sync with nostr", rejected.join(" ")); @@ -122,11 +122,11 @@ pub async fn run_push( }); // all refspecs aren't rejected - if !(git_server_refspecs.is_empty() && proposal_refspecs.is_empty()) { - let (rejected_proposal_refspecs, rejected) = create_and_publish_events( + if !(git_state_refspecs.is_empty() && proposal_refspecs.is_empty()) { + let (rejected_proposal_refspecs, rejected) = create_and_publish_events_and_proposals( git_repo, repo_ref, - &git_server_refspecs, + &git_state_refspecs, &proposal_refspecs, client, existing_state, @@ -135,7 +135,7 @@ pub async fn run_push( .await?; if !rejected { - for refspec in git_server_refspecs.iter().chain(proposal_refspecs.iter()) { + for refspec in git_state_refspecs.iter().chain(proposal_refspecs.iter()) { if rejected_proposal_refspecs.contains(refspec) { continue; } @@ -154,7 +154,7 @@ pub async fn run_push( for (git_server_url, remote_refspecs) in remote_refspecs { let remote_refspecs = remote_refspecs .iter() - .filter(|refspec| git_server_refspecs.contains(refspec)) + .filter(|refspec| git_state_refspecs.contains(refspec)) .cloned() .collect::>(); if !refspecs.is_empty() { @@ -175,7 +175,7 @@ pub async fn run_push( Ok(()) } -async fn create_and_publish_events( +async fn create_and_publish_events_and_proposals( git_repo: &Repo, repo_ref: &RepoRef, git_server_refspecs: &Vec, -- cgit v1.2.3 From ecfb54e1c89455590f816152b9efb722f0115bf1 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Wed, 23 Jul 2025 08:51:21 +0100 Subject: feat(pr): updates and pr as patch revision issue a pull request update if pushing or force pushing a pull request issue a pull request with an e tag for original patch and close status for the original patch when pushing or force pushing against a patch when the new commits are too big to be iussed as patches --- src/bin/git_remote_nostr/push.rs | 155 ++++++++++++++++++++++++++++++++------- src/lib/git_events.rs | 143 +++++++++++++++++++++++------------- 2 files changed, 220 insertions(+), 78 deletions(-) (limited to 'src/bin/git_remote_nostr') diff --git a/src/bin/git_remote_nostr/push.rs b/src/bin/git_remote_nostr/push.rs index 61f4f92..596cd68 100644 --- a/src/bin/git_remote_nostr/push.rs +++ b/src/bin/git_remote_nostr/push.rs @@ -12,8 +12,8 @@ use client::{get_events_from_local_cache, get_state_from_cache, send_events, sig use console::Term; use git::{RepoActions, sha1_to_oid}; use git_events::{ - generate_cover_letter_and_patch_events, generate_patch_event, generate_unsigned_pr_event, - get_commit_id_from_patch, + generate_cover_letter_and_patch_events, generate_patch_event, + generate_unsigned_pr_or_update_event, get_commit_id_from_patch, }; use git2::{Oid, Repository}; use ngit::{ @@ -24,7 +24,7 @@ use ngit::{ nostr_url::{CloneUrl, NostrUrlDecoded}, oid_to_shorthand_string, }, - git_events::{self, event_to_cover_letter, get_event_root}, + git_events::{self, KIND_PULL_REQUEST, event_to_cover_letter, get_event_root}, login::{self, user::UserRef}, repo_ref::{self, get_repo_config_from_yaml, is_grasp_server, normalize_grasp_server_url}, repo_state, @@ -325,14 +325,14 @@ async fn process_proposal_refspecs( let (mut ahead, _) = git_repo.get_commits_ahead_behind(&main_tip, &tip_of_pushed_branch)?; ahead.reverse(); - for patch in generate_cover_letter_and_patch_events( - None, + for patch in generate_patches_or_pr_event_or_pr_updates( git_repo, + repo_ref, &ahead, + user_ref, + Some(proposal), signer, - repo_ref, - &Some(proposal.id.to_string()), - &[], + term, ) .await? { @@ -356,6 +356,23 @@ async fn process_proposal_refspecs( }; let mut parent_patch = tip_patch.clone(); ahead.reverse(); + if proposal.kind.eq(&KIND_PULL_REQUEST) + || are_commits_too_big_for_patches(git_repo, &ahead) + { + for event in generate_patches_or_pr_event_or_pr_updates( + git_repo, + repo_ref, + &ahead, + user_ref, + Some(proposal), + signer, + term, + ) + .await? + { + events.push(event); + } + } for (i, commit) in ahead.iter().enumerate() { let new_patch = generate_patch_event( git_repo, @@ -405,9 +422,10 @@ async fn process_proposal_refspecs( let (mut ahead, _) = git_repo.get_commits_ahead_behind(&main_tip, &tip_of_pushed_branch)?; ahead.reverse(); - for event in - generate_patches_or_pr_event(git_repo, repo_ref, &ahead, user_ref, signer, term) - .await? + for event in generate_patches_or_pr_event_or_pr_updates( + git_repo, repo_ref, &ahead, user_ref, None, signer, term, + ) + .await? { events.push(event); } @@ -417,25 +435,32 @@ async fn process_proposal_refspecs( Ok((events, rejected_proposal_refspecs)) } -async fn generate_patches_or_pr_event( +fn are_commits_too_big_for_patches(git_repo: &Repo, commits: &[Sha1Hash]) -> bool { + commits.iter().any(|commit| { + if let Ok(patch) = git_repo.make_patch_from_commit(commit, &None) { + patch.len() + > ((65 // max recomended patch event size specified in nip34 in kb + // allownace for nostr event wrapper (id, pubkey, tags, sig) + - 1) * 1024) + } else { + true + } + }) +} + +#[allow(clippy::too_many_lines)] +async fn generate_patches_or_pr_event_or_pr_updates( git_repo: &Repo, repo_ref: &RepoRef, ahead: &[Sha1Hash], user_ref: &UserRef, + root_proposal: Option<&Event>, signer: &Arc, term: &Term, ) -> Result> { let mut events: Vec = vec![]; - let use_pr = ahead.iter().any(|commit| { - if let Ok(patch) = git_repo.make_patch_from_commit(commit, &None) { - patch.len() - > ((65 // max recomended patch event size specified in nip34 in kb - // allownace for nostr event wrapper (id, pubkey, tags, sig) - - 1) * 1024) - } else { - true - } - }); + let use_pr = root_proposal.is_some_and(|proposal| proposal.kind.eq(&KIND_PULL_REQUEST)) + || are_commits_too_big_for_patches(git_repo, ahead); if use_pr { let repo_grasps = repo_ref.grasp_servers(); @@ -450,10 +475,11 @@ async fn generate_patches_or_pr_event( let mut draft_pr_event = if let Some(ref unsigned_pr_event) = unsigned_pr_event { unsigned_pr_event.clone() } else { - generate_unsigned_pr_event( + generate_unsigned_pr_or_update_event( git_repo, repo_ref, &user_ref.public_key, + root_proposal, ahead.first().context("no commits to push")?, &[clone_url], &[], @@ -494,9 +520,30 @@ async fn generate_patches_or_pr_event( // for loop and continue } if let Some(unsigned_pr_event) = unsigned_pr_event { - let pr_event = - sign_draft_event(unsigned_pr_event, signer, "Pull Request".to_string()).await?; + let pr_event = sign_draft_event( + unsigned_pr_event, + signer, + if root_proposal.is_some_and(|proposal| proposal.kind.eq(&Kind::GitPatch)) { + "Pull Request Replacing Original Patch" + } else if root_proposal.is_some() { + "Pull Request Update" + } else { + "Pull Request" + } + .to_string(), + ) + .await?; events.push(pr_event); + if root_proposal.is_some_and(|proposal| proposal.kind.eq(&Kind::GitPatch)) { + events.push( + create_close_status_for_original_patch( + signer, + repo_ref, + root_proposal.unwrap(), + ) + .await?, + ); + } } else { bail!("could not find a grasp server that accepts the Pull Request refs"); } @@ -507,7 +554,7 @@ async fn generate_patches_or_pr_event( ahead, signer, repo_ref, - &None, + &root_proposal.map(|proposal| proposal.id.to_string()), &[], ) .await? @@ -1487,6 +1534,62 @@ async fn create_merge_status( .await } +async fn create_close_status_for_original_patch( + signer: &Arc, + repo_ref: &RepoRef, + proposal: &Event, +) -> Result { + let mut public_keys = repo_ref + .maintainers + .iter() + .copied() + .collect::>(); + public_keys.insert(proposal.pubkey); + + sign_event( + EventBuilder::new(nostr::event::Kind::GitStatusClosed, String::new()).tags( + [ + vec![ + Tag::custom( + nostr::TagKind::Custom(std::borrow::Cow::Borrowed("alt")), + vec![ + "Git patch closed as forthcoming update is too large. Replacing with Pull Request" + .to_string(), + ], + ), + Tag::from_standardized(nostr::TagStandard::Event { + event_id: proposal.id, + relay_url: repo_ref.relays.first().cloned(), + marker: Some(Marker::Root), + public_key: None, + uppercase: false, + }), + ], + public_keys.iter().map(|pk| Tag::public_key(*pk)).collect(), + repo_ref + .coordinates() + .iter() + .map(|c| { + Tag::from_standardized(TagStandard::Coordinate { + coordinate: c.coordinate.clone(), + relay_url: c.relays.first().cloned(), + uppercase: false, + }) + }) + .collect::>(), + vec![ + Tag::from_standardized(nostr::TagStandard::Reference( + repo_ref.root_commit.to_string(), + )), + ], + ] + .concat(), + ), + signer, + "close status for original patch".to_string(), + ) + .await +} async fn get_proposal_and_revision_root_from_patch( git_repo: &Repo, patch: &Event, diff --git a/src/lib/git_events.rs b/src/lib/git_events.rs index 86b9641..7bca63b 100644 --- a/src/lib/git_events.rs +++ b/src/lib/git_events.rs @@ -350,10 +350,11 @@ pub fn event_tag_from_nip19_or_hex( } } -pub fn generate_unsigned_pr_event( +pub fn generate_unsigned_pr_or_update_event( git_repo: &Repo, repo_ref: &RepoRef, signing_public_key: &PublicKey, + root_proposal: Option<&Event>, commit: &Sha1Hash, clone_url_hint: &[&str], mentions: &[nostr::Tag], @@ -372,59 +373,97 @@ pub fn generate_unsigned_pr_event( .get_root_commit() .context("failed to get root commit of the repository")?; - Ok(EventBuilder::new(KIND_PULL_REQUEST, description) - .tags( - [ - repo_ref - .maintainers - .iter() - .map(|m| { - Tag::from_standardized(TagStandard::Coordinate { - coordinate: Coordinate { - kind: nostr::Kind::GitRepoAnnouncement, - public_key: *m, - identifier: repo_ref.identifier.to_string(), - }, - relay_url: repo_ref.relays.first().cloned(), - uppercase: false, - }) + Ok(if root_proposal.is_some() { + EventBuilder::new(KIND_PULL_REQUEST_UPDATE, "") + } else { + EventBuilder::new(KIND_PULL_REQUEST, description) + } + .tags( + [ + repo_ref + .maintainers + .iter() + .map(|m| { + Tag::from_standardized(TagStandard::Coordinate { + coordinate: Coordinate { + kind: nostr::Kind::GitRepoAnnouncement, + public_key: *m, + identifier: repo_ref.identifier.to_string(), + }, + relay_url: repo_ref.relays.first().cloned(), + uppercase: false, }) - .collect::>(), - mentions.to_vec(), - vec![ - Tag::from_standardized(TagStandard::Subject(title.clone())), - Tag::from_standardized(TagStandard::Reference(format!("{root_commit}"))), - Tag::custom( - nostr::TagKind::Custom(std::borrow::Cow::Borrowed("c")), - vec![format!("{commit}")], - ), - Tag::custom( - nostr::TagKind::Custom(std::borrow::Cow::Borrowed("clone")), - clone_url_hint - .iter() - .map(|s| s.to_string()) - .collect::>(), - ), - Tag::custom( + }) + .collect::>(), + mentions.to_vec(), + if let Some(root_proposal) = root_proposal { + [ + vec![Tag::custom( nostr::TagKind::Custom(std::borrow::Cow::Borrowed("alt")), - vec![format!("git Pull Request: {}", title.clone())], - ), - ], - if let Some(branch_name_tag) = make_branch_name_tag_from_check_out_branch(git_repo) - { - vec![branch_name_tag] - } else { - vec![] - }, - repo_ref - .maintainers - .iter() - .map(|pk| Tag::public_key(*pk)) - .collect(), - ] - .concat(), - ) - .build(*signing_public_key)) + vec![format!("git Pull Request Update")], + )], + if root_proposal.kind.eq(&KIND_PULL_REQUEST) { + vec![ + Tag::custom( + nostr::TagKind::Custom(std::borrow::Cow::Borrowed("E")), + vec![root_proposal.id], + ), + Tag::custom( + nostr::TagKind::Custom(std::borrow::Cow::Borrowed("P")), + vec![root_proposal.pubkey], + ), + ] + } else { + // root proposal is a Patch - so use e tag per nip34 spec + vec![Tag::custom( + nostr::TagKind::Custom(std::borrow::Cow::Borrowed("e")), + vec![root_proposal.id], + )] + }, + ] + .concat() + } else { + [ + vec![ + Tag::from_standardized(TagStandard::Subject(title.clone())), + Tag::custom( + nostr::TagKind::Custom(std::borrow::Cow::Borrowed("alt")), + vec![format!("git Pull Request: {}", title.clone())], + ), + ], + if let Some(branch_name_tag) = + make_branch_name_tag_from_check_out_branch(git_repo) + { + vec![branch_name_tag] + } else { + vec![] + }, + ] + .concat() + }, + vec![ + Tag::from_standardized(TagStandard::Reference(format!("{root_commit}"))), + Tag::custom( + nostr::TagKind::Custom(std::borrow::Cow::Borrowed("c")), + vec![format!("{commit}")], + ), + Tag::custom( + nostr::TagKind::Custom(std::borrow::Cow::Borrowed("clone")), + clone_url_hint + .iter() + .map(|s| s.to_string()) + .collect::>(), + ), + ], + repo_ref + .maintainers + .iter() + .map(|pk| Tag::public_key(*pk)) + .collect(), + ] + .concat(), + ) + .build(*signing_public_key)) } fn make_branch_name_tag_from_check_out_branch(git_repo: &Repo) -> Option { -- cgit v1.2.3 From b4253bb0c543ceef896e0a95482b5403a29a7878 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Wed, 23 Jul 2025 08:55:03 +0100 Subject: fix(status): only use events from author and maintainers instead of status events from any pubkey --- src/bin/git_remote_nostr/utils.rs | 1 + 1 file changed, 1 insertion(+) (limited to 'src/bin/git_remote_nostr') diff --git a/src/bin/git_remote_nostr/utils.rs b/src/bin/git_remote_nostr/utils.rs index d0b4e7e..563c0d8 100644 --- a/src/bin/git_remote_nostr/utils.rs +++ b/src/bin/git_remote_nostr/utils.rs @@ -131,6 +131,7 @@ pub async fn get_open_or_draft_proposals( && e.tags.iter().any(|t| { t.as_slice().len() > 1 && t.as_slice()[1].eq(&proposal.id.to_string()) }) + && (proposal.pubkey.eq(&e.pubkey) || repo_ref.maintainers.contains(&e.pubkey)) }) .collect::>() .first() -- cgit v1.2.3 From f7299cc5fd2276db8d9bb7778c34ddbe5b3a8e48 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Wed, 23 Jul 2025 10:33:20 +0100 Subject: feat(pr): patch upgraded to pr inherit pr status when a patch is upgraded to a pr, eg because new commits would be too large to be additional patches, the patch receives a closed staus and the new pr 'e' tags the original root patch. we therefore need to inherit the new pr's status instead of using the closed status. the closed status was used so that clients don't have to support pr revisions of patches, and still have a good UX. --- src/bin/git_remote_nostr/utils.rs | 43 +++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) (limited to 'src/bin/git_remote_nostr') diff --git a/src/bin/git_remote_nostr/utils.rs b/src/bin/git_remote_nostr/utils.rs index 563c0d8..8433967 100644 --- a/src/bin/git_remote_nostr/utils.rs +++ b/src/bin/git_remote_nostr/utils.rs @@ -18,8 +18,9 @@ use ngit::{ nostr_url::{CloneUrl, NostrUrlDecoded, ServerProtocol}, }, git_events::{ - event_is_revision_root, get_pr_tip_event_or_most_recent_patch_with_ancestors, - is_event_proposal_root_for_branch, status_kinds, + KIND_PULL_REQUEST, event_is_revision_root, + get_pr_tip_event_or_most_recent_patch_with_ancestors, is_event_proposal_root_for_branch, + status_kinds, }, repo_ref::RepoRef, }; @@ -123,8 +124,8 @@ pub async fn get_open_or_draft_proposals( }; let mut open_or_draft_proposals = HashMap::new(); - for proposal in proposals { - let status = if let Some(e) = statuses + let get_status = |proposal: &Event| { + if let Some(e) = statuses .iter() .filter(|e| { status_kinds().contains(&e.kind) @@ -139,8 +140,32 @@ pub async fn get_open_or_draft_proposals( e.kind } else { Kind::GitStatusOpen - }; - if [Kind::GitStatusOpen, Kind::GitStatusDraft].contains(&status) { + } + }; + + let is_proposal_pr_revision_of_patch = |proposal: &Event, patch: &Event| { + proposal.kind.eq(&KIND_PULL_REQUEST) + && proposal.tags.clone().into_iter().any(|t| { + t.as_slice().len() > 1 + && t.as_slice()[0].eq("e") + && t.as_slice()[1].eq(&patch.id.to_string()) + && [Kind::GitStatusOpen, Kind::GitStatusDraft].contains(&get_status(proposal)) + }) + }; + + for proposal in &proposals { + let status = get_status(proposal); + if [Kind::GitStatusOpen, Kind::GitStatusDraft].contains(&status) + || // or patch has been revised as a PR which is open or draft + ( + status.eq(&Kind::GitStatusClosed) && + proposal.kind.eq(&Kind::GitPatch) && + proposals.iter() + .any(|p| { + is_proposal_pr_revision_of_patch(p, proposal) + }) + ) + { if let Ok(commits_events) = get_all_proposal_patch_pr_pr_update_events_from_cache( git_repo_path, repo_ref, @@ -151,8 +176,10 @@ pub async fn get_open_or_draft_proposals( if let Ok(most_recent_proposal_patch_chain) = get_pr_tip_event_or_most_recent_patch_with_ancestors(commits_events.clone()) { - open_or_draft_proposals - .insert(proposal.id, (proposal, most_recent_proposal_patch_chain)); + open_or_draft_proposals.insert( + proposal.id, + (proposal.clone(), most_recent_proposal_patch_chain), + ); } } } -- cgit v1.2.3 From bbdd2093506b31359ab1ff61b709e40de043c879 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Wed, 23 Jul 2025 16:25:49 +0100 Subject: fix(remote): error if pushed proposal is empty erorr if the pushed ref would produce a proposal with no patches, or if the ref is in origin/ --- src/bin/git_remote_nostr/push.rs | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) (limited to 'src/bin/git_remote_nostr') diff --git a/src/bin/git_remote_nostr/push.rs b/src/bin/git_remote_nostr/push.rs index 596cd68..b997ea7 100644 --- a/src/bin/git_remote_nostr/push.rs +++ b/src/bin/git_remote_nostr/push.rs @@ -321,10 +321,15 @@ async fn process_proposal_refspecs( { if refspec.starts_with('+') { // force push - let (_, main_tip) = git_repo.get_main_or_master_branch()?; + let (main_branch_name, main_tip) = git_repo.get_main_or_master_branch()?; let (mut ahead, _) = git_repo.get_commits_ahead_behind(&main_tip, &tip_of_pushed_branch)?; ahead.reverse(); + if ahead.is_empty() { + bail!( + "cannot push '{from}' as proposal as branch isn't ahead of '{main_branch_name}'" + ); + } for patch in generate_patches_or_pr_event_or_pr_updates( git_repo, repo_ref, @@ -356,6 +361,11 @@ async fn process_proposal_refspecs( }; let mut parent_patch = tip_patch.clone(); ahead.reverse(); + if ahead.is_empty() { + bail!( + "cannot push '{from}' as proposal as branch isn't ahead of proposal on nostr" + ); + } if proposal.kind.eq(&KIND_PULL_REQUEST) || are_commits_too_big_for_patches(git_repo, &ahead) { @@ -418,10 +428,15 @@ async fn process_proposal_refspecs( } } else { // TODO new proposal / couldn't find exisiting proposal - let (_, main_tip) = git_repo.get_main_or_master_branch()?; + let (main_branch_name, main_tip) = git_repo.get_main_or_master_branch()?; let (mut ahead, _) = git_repo.get_commits_ahead_behind(&main_tip, &tip_of_pushed_branch)?; ahead.reverse(); + if ahead.is_empty() { + bail!( + "cannot push '{from}' as proposal as branch isn't ahead of '{main_branch_name}'" + ); + } for event in generate_patches_or_pr_event_or_pr_updates( git_repo, repo_ref, &ahead, user_ref, None, signer, term, ) @@ -513,11 +528,11 @@ async fn generate_patches_or_pr_event_or_pr_updates( } } if unsigned_pr_event.is_none() { - // TODO get fallback grasp servers that aren't in repo_grasps cycle - // through until one succeeds TODO create personal-fork - // announcement with grasp servers and push, after a few seconds - // push ref/nostr/eventid. if one success break out of - // for loop and continue + // TODO get grasp_default_set servers that aren't in repo_grasps + // cycle through until one succeeds TODO create + // personal-fork announcement with grasp servers and + // push, after a few seconds push ref/nostr/eventid. if + // one success break out of for loop and continue } if let Some(unsigned_pr_event) = unsigned_pr_event { let pr_event = sign_draft_event( -- cgit v1.2.3 From 3a3f8c877d060f7510b79f32233e2bd2574de4c1 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Wed, 23 Jul 2025 16:27:50 +0100 Subject: fix(remote): dont send pr and patches on upgrade when an upgrade to a pr is needed, dont also try and send patches --- src/bin/git_remote_nostr/push.rs | 45 ++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 22 deletions(-) (limited to 'src/bin/git_remote_nostr') diff --git a/src/bin/git_remote_nostr/push.rs b/src/bin/git_remote_nostr/push.rs index b997ea7..e694e18 100644 --- a/src/bin/git_remote_nostr/push.rs +++ b/src/bin/git_remote_nostr/push.rs @@ -382,28 +382,29 @@ async fn process_proposal_refspecs( { events.push(event); } - } - for (i, commit) in ahead.iter().enumerate() { - let new_patch = generate_patch_event( - git_repo, - &git_repo.get_root_commit()?, - commit, - Some(thread_id), - signer, - repo_ref, - Some(parent_patch.id), - Some(( - (patches.len() + i + 1).try_into().unwrap(), - (patches.len() + ahead.len()).try_into().unwrap(), - )), - None, - &None, - &[], - ) - .await - .context("failed to make patch event from commit")?; - events.push(new_patch.clone()); - parent_patch = new_patch; + } else { + for (i, commit) in ahead.iter().enumerate() { + let new_patch = generate_patch_event( + git_repo, + &git_repo.get_root_commit()?, + commit, + Some(thread_id), + signer, + repo_ref, + Some(parent_patch.id), + Some(( + (patches.len() + i + 1).try_into().unwrap(), + (patches.len() + ahead.len()).try_into().unwrap(), + )), + None, + &None, + &[], + ) + .await + .context("failed to make patch event from commit")?; + events.push(new_patch.clone()); + parent_patch = new_patch; + } } } else { // we shouldn't get here -- cgit v1.2.3 From 802b115ca648011fd311a22ef3650aaa5a1a0acf Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Wed, 23 Jul 2025 17:16:18 +0100 Subject: fix(remote): improve pr error messages as a temporary measure --- src/bin/git_remote_nostr/push.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'src/bin/git_remote_nostr') diff --git a/src/bin/git_remote_nostr/push.rs b/src/bin/git_remote_nostr/push.rs index e694e18..353ad77 100644 --- a/src/bin/git_remote_nostr/push.rs +++ b/src/bin/git_remote_nostr/push.rs @@ -529,6 +529,10 @@ async fn generate_patches_or_pr_event_or_pr_updates( } } if unsigned_pr_event.is_none() { + bail!( + "a commit in your proposal is too big for a nostr patch. The repository doesnt list a grasp server which would otherwise be used to submit your proposal as nostr Pull Request. Soon ngit will support pushing your changes to a different git / grasp git server." + ); + // TODO get grasp_default_set servers that aren't in repo_grasps // cycle through until one succeeds TODO create // personal-fork announcement with grasp servers and @@ -561,7 +565,11 @@ async fn generate_patches_or_pr_event_or_pr_updates( ); } } else { - bail!("could not find a grasp server that accepts the Pull Request refs"); + bail!( + "a commit in your proposal is too big for a nostr patch. tried to use submit as a nostr Pull Request but could not find a grasp server that would accept your changes" + ); + // TODO suggest `ngit send` where user could specify their own clone + // url to push to once that feature is added } } else { for patch in generate_cover_letter_and_patch_events( -- cgit v1.2.3 From 9357b62b9824299be6fc85b09f57d93d3902f79a Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 25 Jul 2025 11:48:22 +0100 Subject: refactor: abstract `get_status` for use by `ngit list` --- src/bin/git_remote_nostr/utils.rs | 58 ++++++++------------------------------- src/bin/ngit/sub_commands/list.rs | 5 +++- src/lib/git_events.rs | 56 ++++++++++++++++++++++++++++++++++++- 3 files changed, 71 insertions(+), 48 deletions(-) (limited to 'src/bin/git_remote_nostr') diff --git a/src/bin/git_remote_nostr/utils.rs b/src/bin/git_remote_nostr/utils.rs index 8433967..2cb85bf 100644 --- a/src/bin/git_remote_nostr/utils.rs +++ b/src/bin/git_remote_nostr/utils.rs @@ -18,9 +18,8 @@ use ngit::{ nostr_url::{CloneUrl, NostrUrlDecoded, ServerProtocol}, }, git_events::{ - KIND_PULL_REQUEST, event_is_revision_root, - get_pr_tip_event_or_most_recent_patch_with_ancestors, is_event_proposal_root_for_branch, - status_kinds, + event_is_revision_root, get_pr_tip_event_or_most_recent_patch_with_ancestors, get_status, + is_event_proposal_root_for_branch, status_kinds, }, repo_ref::RepoRef, }; @@ -104,7 +103,10 @@ pub async fn get_open_or_draft_proposals( get_proposals_and_revisions_from_cache(git_repo_path, repo_ref.coordinates()) .await? .iter() - .filter(|e| !event_is_revision_root(e)) + .filter(|e| + // If we wanted to treat to list Pull Requests that revise a Patch we would do this: + // e.kind.eq(&KIND_PULL_REQUEST) || + !event_is_revision_root(e)) .cloned() .collect(); @@ -124,48 +126,9 @@ pub async fn get_open_or_draft_proposals( }; let mut open_or_draft_proposals = HashMap::new(); - let get_status = |proposal: &Event| { - if let Some(e) = statuses - .iter() - .filter(|e| { - status_kinds().contains(&e.kind) - && e.tags.iter().any(|t| { - t.as_slice().len() > 1 && t.as_slice()[1].eq(&proposal.id.to_string()) - }) - && (proposal.pubkey.eq(&e.pubkey) || repo_ref.maintainers.contains(&e.pubkey)) - }) - .collect::>() - .first() - { - e.kind - } else { - Kind::GitStatusOpen - } - }; - - let is_proposal_pr_revision_of_patch = |proposal: &Event, patch: &Event| { - proposal.kind.eq(&KIND_PULL_REQUEST) - && proposal.tags.clone().into_iter().any(|t| { - t.as_slice().len() > 1 - && t.as_slice()[0].eq("e") - && t.as_slice()[1].eq(&patch.id.to_string()) - && [Kind::GitStatusOpen, Kind::GitStatusDraft].contains(&get_status(proposal)) - }) - }; - for proposal in &proposals { - let status = get_status(proposal); - if [Kind::GitStatusOpen, Kind::GitStatusDraft].contains(&status) - || // or patch has been revised as a PR which is open or draft - ( - status.eq(&Kind::GitStatusClosed) && - proposal.kind.eq(&Kind::GitPatch) && - proposals.iter() - .any(|p| { - is_proposal_pr_revision_of_patch(p, proposal) - }) - ) - { + let status = get_status(proposal, repo_ref, &statuses, &proposals); + if [Kind::GitStatusOpen, Kind::GitStatusDraft].contains(&status) { if let Ok(commits_events) = get_all_proposal_patch_pr_pr_update_events_from_cache( git_repo_path, repo_ref, @@ -196,7 +159,10 @@ pub async fn get_all_proposals( get_proposals_and_revisions_from_cache(git_repo_path, repo_ref.coordinates()) .await? .iter() - .filter(|e| !event_is_revision_root(e)) + .filter(|e| + // If we wanted to treat to list Pull Requests that revise a Patch we would do this: + // e.kind.eq(&KIND_PULL_REQUEST) || + !event_is_revision_root(e)) .cloned() .collect(); diff --git a/src/bin/ngit/sub_commands/list.rs b/src/bin/ngit/sub_commands/list.rs index 9e35b33..95e17f3 100644 --- a/src/bin/ngit/sub_commands/list.rs +++ b/src/bin/ngit/sub_commands/list.rs @@ -72,7 +72,10 @@ pub async fn launch() -> Result<()> { let proposals: Vec = proposals_and_revisions .iter() - .filter(|e| !event_is_revision_root(e)) + .filter(|e| + // If we wanted to treat to list Pull Requests that revise a Patch we would do this: + // e.kind.eq(&KIND_PULL_REQUEST) || + !event_is_revision_root(e)) .cloned() .collect(); diff --git a/src/lib/git_events.rs b/src/lib/git_events.rs index 7cd64ab..2e1f215 100644 --- a/src/lib/git_events.rs +++ b/src/lib/git_events.rs @@ -835,7 +835,61 @@ pub fn is_event_proposal_root_for_branch( || cl .get_branch_name_with_pr_prefix_and_shorthand_id() .is_ok_and(|s| s.eq(&branch_name)) - }) && !event_is_revision_root(e)) + }) && ( + // If we wanted to treat to list Pull Requests that revise a Patch we would do this: + // Note: whilst this the the case elsewhere event_is_revision_root is used, there is more to + // think about here? + // e.kind.eq(&KIND_PULL_REQUEST) || + !event_is_revision_root(e) + )) +} + +pub fn get_status( + proposal: &Event, + repo_ref: &RepoRef, + all_status_in_repo: &[Event], + all_pr_roots_in_repo: &[Event], +) -> Kind { + let get_direct_status = |proposal: &Event| { + if let Some(e) = all_status_in_repo + .iter() + .filter(|e| { + status_kinds().contains(&e.kind) + && e.tags.iter().any(|t| { + t.as_slice().len() > 1 && t.as_slice()[1].eq(&proposal.id.to_string()) + }) + && (proposal.pubkey.eq(&e.pubkey) || repo_ref.maintainers.contains(&e.pubkey)) + }) + .collect::>() + .first() + { + e.kind + } else { + Kind::GitStatusOpen + } + }; + let is_proposal_pr_revision_of_patch = |proposal: &Event, patch: &Event| { + proposal.kind.eq(&KIND_PULL_REQUEST) + && proposal.tags.clone().into_iter().any(|t| { + t.as_slice().len() > 1 + && t.as_slice()[0].eq("e") + && t.as_slice()[1].eq(&patch.id.to_string()) + }) + }; + + let direct_status = get_direct_status(proposal); + if direct_status.eq(&Kind::GitStatusClosed) && proposal.kind.eq(&Kind::GitPatch) { + if let Some(pr_revision) = all_pr_roots_in_repo + .iter() + .find(|p| is_proposal_pr_revision_of_patch(p, proposal)) + { + get_direct_status(pr_revision) + } else { + direct_status + } + } else { + direct_status + } } #[cfg(test)] -- cgit v1.2.3 From 4fc659074ec5ced3cc0727cf1f3e6af082a189cc Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 25 Jul 2025 12:23:23 +0100 Subject: feat(pr): add pr and pr update merge support as these events use `c` instead of `commit` --- src/bin/git_remote_nostr/push.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'src/bin/git_remote_nostr') diff --git a/src/bin/git_remote_nostr/push.rs b/src/bin/git_remote_nostr/push.rs index 353ad77..909a0ab 100644 --- a/src/bin/git_remote_nostr/push.rs +++ b/src/bin/git_remote_nostr/push.rs @@ -1246,7 +1246,7 @@ type MergedProposalsInfo = async fn get_merged_proposals_info( git_repo: &Repo, ahead: &Vec, - available_patches: &[Event], + available_patches_pr_pr_update: &[Event], ) -> Result { let mut proposals: MergedProposalsInfo = HashMap::new(); @@ -1256,19 +1256,19 @@ async fn get_merged_proposals_info( // are in ahead if commit.parent_count() > 1 { for parent in commit.parents() { - for patch_event in available_patches + for event in available_patches_pr_pr_update .iter() .filter(|e| { e.tags.iter().any(|t| { t.as_slice().len() > 1 - && t.as_slice()[0].eq("commit") + && (t.as_slice()[0].eq("commit") || t.as_slice()[0].eq("c")) && t.as_slice()[1].eq(&parent.id().to_string()) }) }) .collect::>() { if let Ok((proposal_id, revision_id)) = - get_proposal_and_revision_root_from_patch(git_repo, patch_event).await + get_proposal_and_revision_root_from_patch(git_repo, event).await { let (entry_revision_id, merged_patches) = proposals.entry(proposal_id).or_default(); @@ -1281,12 +1281,12 @@ async fn get_merged_proposals_info( } else { // three way merge or fast forward merge commits // note: ahead included commits of three-way merged branches - let mut matching_patches = available_patches + let mut matching_patches = available_patches_pr_pr_update .iter() .filter(|e| { e.tags.iter().any(|t| { t.as_slice().len() > 1 - && t.as_slice()[0].eq("commit") + && (t.as_slice()[0].eq("commit") || t.as_slice()[0].eq("c")) && t.as_slice()[1].eq(&commit_hash.to_string()) }) }) @@ -1311,7 +1311,7 @@ async fn get_merged_proposals_info( // applied commits - this is done after so that merged revisions take priority if matching_patches.is_empty() { let author = git_repo.get_commit_author(commit_hash)?; - matching_patches = available_patches + matching_patches = available_patches_pr_pr_update .iter() .filter(|e| { if let Ok(patch_author) = get_patch_author(e) { -- cgit v1.2.3