From f76fe63da5f2c2f85215e86c8ecc63eda7c93902 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Mon, 4 Aug 2025 08:50:54 +0100 Subject: refactor: move generate pr event fn into lib for future use in `ngit send` --- src/lib/push.rs | 168 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 167 insertions(+), 1 deletion(-) (limited to 'src/lib/push.rs') diff --git a/src/lib/push.rs b/src/lib/push.rs index 0d0ec93..1c09555 100644 --- a/src/lib/push.rs +++ b/src/lib/push.rs @@ -1,19 +1,31 @@ use std::{ + collections::HashSet, sync::{Arc, Mutex}, time::Instant, }; -use anyhow::{Result, anyhow}; +use anyhow::{Result, anyhow, bail}; use auth_git2::GitAuthenticator; use console::Term; +use nostr::{ + event::{Event, EventBuilder, Kind, Tag, TagStandard, UnsignedEvent}, + hashes::sha1::Hash as Sha1Hash, + key::PublicKey, + nips::nip10::Marker, + signer::NostrSigner, +}; use crate::{ cli_interactor::count_lines_per_msg_vec, + client::{sign_draft_event, sign_event}, git::{ Repo, nostr_url::{CloneUrl, NostrUrlDecoded}, oid_to_shorthand_string, }, + git_events::generate_unsigned_pr_or_update_event, + login::user::UserRef, + repo_ref::{RepoRef, is_grasp_server, normalize_grasp_server_url}, utils::{ Direction, get_short_git_server_name, get_write_protocols_to_try, join_with_and, set_protocol_preference, @@ -308,3 +320,157 @@ impl<'a> PushReporter<'a> { } } } + +pub async fn push_refs_and_generate_pr_or_pr_update_event( + git_repo: &Repo, + repo_ref: &RepoRef, + tip: &Sha1Hash, + user_ref: &UserRef, + root_proposal: Option<&Event>, + signer: &Arc, + term: &Term, +) -> Result> { + let mut events: Vec = vec![]; + 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_or_update_event( + git_repo, + repo_ref, + &user_ref.public_key, + root_proposal, + tip, + &[clone_url], + &[], + )? + }; + + let refspec = format!("{}:refs/nostr/{}", tip, 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() { + bail!( + "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 + // 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, + 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!( + "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 + } + Ok(events) +} + +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 +} -- cgit v1.2.3 From f48677bad3f3dabb80992806e0e4c8ad4d45c716 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Mon, 4 Aug 2025 11:50:39 +0100 Subject: feat(send): support PR and PR update events send as a PR if the commit would make patches that are too big for nostr events. send as a PR update if the proposal is PR. send as a PR, revising a patch root, if patches would be too big. in tests `get_pretend_proposal_root_event` has to be a actual proposal with a tip, rather than just a cover letter, so we have replaced it. --- src/bin/git_remote_nostr/push.rs | 1 + src/bin/ngit/sub_commands/send.rs | 137 +++++++++++++++++++++++++------------- src/lib/git_events.rs | 10 ++- src/lib/push.rs | 3 + src/lib/utils.rs | 36 +++++++++- test_utils/src/lib.rs | 2 +- tests/ngit_send.rs | 84 +++++++++++++++++------ 7 files changed, 202 insertions(+), 71 deletions(-) (limited to 'src/lib/push.rs') diff --git a/src/bin/git_remote_nostr/push.rs b/src/bin/git_remote_nostr/push.rs index e588a5a..3967699 100644 --- a/src/bin/git_remote_nostr/push.rs +++ b/src/bin/git_remote_nostr/push.rs @@ -461,6 +461,7 @@ async fn generate_patches_or_pr_event_or_pr_updates( ahead.first().context("no commits to push")?, user_ref, root_proposal, + &None, signer, term, ) diff --git a/src/bin/ngit/sub_commands/send.rs b/src/bin/ngit/sub_commands/send.rs index 9f1857f..0aefb03 100644 --- a/src/bin/ngit/sub_commands/send.rs +++ b/src/bin/ngit/sub_commands/send.rs @@ -4,9 +4,11 @@ use anyhow::{Context, Result, bail}; use console::Style; use ngit::{ client::{Params, send_events}, - git_events::{EventRefType, generate_cover_letter_and_patch_events}, + git_events::{EventRefType, KIND_PULL_REQUEST, generate_cover_letter_and_patch_events}, + push::push_refs_and_generate_pr_or_pr_update_event, + utils::proposal_tip_is_pr_or_pr_update, }; -use nostr::{ToBech32, nips::nip19::Nip19Event}; +use nostr::{ToBech32, event::Event, nips::nip19::Nip19Event}; use nostr_sdk::hashes::sha1::Hash as Sha1Hash; use crate::{ @@ -60,12 +62,14 @@ pub async fn launch(cli_args: &Cli, args: &SubCommandArgs, no_fetch: bool) -> Re fetching_with_report(git_repo_path, &client, &repo_coordinates).await?; } - let (root_proposal_id, mention_tags) = - get_root_proposal_id_and_mentions_from_in_reply_to(git_repo.get_path()?, &args.in_reply_to) + let repo_ref = get_repo_ref_from_cache(Some(git_repo_path), &repo_coordinates).await?; + + let (root_proposal, mention_tags) = + get_root_proposal_and_mentions_from_in_reply_to(git_repo.get_path()?, &args.in_reply_to) .await?; if let Some(root_ref) = args.in_reply_to.first() { - if root_proposal_id.is_some() { + if root_proposal.is_some() { println!("creating proposal revision for: {root_ref}"); } } @@ -112,7 +116,30 @@ pub async fn launch(cli_args: &Cli, args: &SubCommandArgs, no_fetch: bool) -> Re &main_tip, )?; - let title = if args.no_cover_letter { + let as_pr = { + if let Some(root_proposal) = &root_proposal { + proposal_tip_is_pr_or_pr_update(git_repo_path, &repo_ref, &root_proposal.id).await? + } else { + false + } + } || git_repo.are_commits_too_big_for_patches(&commits); + + let title = if as_pr { + match &args.title { + Some(t) => Some(t.clone()), + None => { + if root_proposal.is_none() { + Some( + Interactor::default() + .input(PromptInputParms::default().with_prompt("title"))? + .clone(), + ) + } else { + None + } + } + } + } else if args.no_cover_letter { None } else { match &args.title { @@ -142,7 +169,7 @@ pub async fn launch(cli_args: &Cli, args: &SubCommandArgs, no_fetch: bool) -> Re t.clone() } else { Interactor::default() - .input(PromptInputParms::default().with_prompt("cover letter description"))? + .input(PromptInputParms::default().with_prompt("description"))? .clone() }, )) @@ -161,42 +188,58 @@ pub async fn launch(cli_args: &Cli, args: &SubCommandArgs, no_fetch: bool) -> Re client.set_signer(signer.clone()).await; - let repo_ref = get_repo_ref_from_cache(Some(git_repo_path), &repo_coordinates).await?; - // oldest first commits.reverse(); - let events = generate_cover_letter_and_patch_events( - cover_letter_title_description.clone(), - &git_repo, - &commits, - &signer, - &repo_ref, - &root_proposal_id, - &mention_tags, - ) - .await?; + let events = if as_pr { + push_refs_and_generate_pr_or_pr_update_event( + &git_repo, + &repo_ref, + commits.last().context("no commits")?, + &user_ref, + root_proposal.as_ref(), + &cover_letter_title_description, + &signer, + &console::Term::stdout(), + ) + .await? - println!( - "posting {} patch{} {} a covering letter...", - if cover_letter_title_description.is_none() { - events.len() - } else { - events.len() - 1 - }, - if cover_letter_title_description.is_none() && events.len().eq(&1) - || cover_letter_title_description.is_some() && events.len().eq(&2) - { - "" - } else { - "es" - }, - if cover_letter_title_description.is_none() { - "without" - } else { - "with" - } - ); + // TODO + // - allow specifying clone url and ref + } else { + let events = generate_cover_letter_and_patch_events( + cover_letter_title_description.clone(), + &git_repo, + &commits, + &signer, + &repo_ref, + &root_proposal.as_ref().map(|e| e.id.to_string()), + &mention_tags, + ) + .await?; + + println!( + "posting {} patch{} {} a covering letter...", + if cover_letter_title_description.is_none() { + events.len() + } else { + events.len() - 1 + }, + if cover_letter_title_description.is_none() && events.len().eq(&1) + || cover_letter_title_description.is_some() && events.len().eq(&2) + { + "" + } else { + "es" + }, + if cover_letter_title_description.is_none() { + "without" + } else { + "with" + } + ); + events + }; send_events( &client, @@ -209,7 +252,7 @@ pub async fn launch(cli_args: &Cli, args: &SubCommandArgs, no_fetch: bool) -> Re ) .await?; - if root_proposal_id.is_none() { + if root_proposal.is_none() { if let Some(event) = events.first() { let event_bech32 = if let Some(relay) = repo_ref.relays.first() { Nip19Event { @@ -376,11 +419,11 @@ fn summarise_commit_for_selection(git_repo: &Repo, commit: &Sha1Hash) -> Result< )) } -async fn get_root_proposal_id_and_mentions_from_in_reply_to( +async fn get_root_proposal_and_mentions_from_in_reply_to( git_repo_path: &Path, in_reply_to: &[String], -) -> Result<(Option, Vec)> { - let root_proposal_id = if let Some(first) = in_reply_to.first() { +) -> Result<(Option, Vec)> { + let root_proposal = if let Some(first) = in_reply_to.first() { match event_tag_from_nip19_or_hex(first, "in-reply-to", EventRefType::Root, true, false)? .as_standardized() { @@ -398,8 +441,8 @@ async fn get_root_proposal_id_and_mentions_from_in_reply_to( .await?; if let Some(first) = events.iter().find(|e| e.id.eq(event_id)) { - if event_is_patch_set_root(first) { - Some(event_id.to_string()) + if event_is_patch_set_root(first) || first.kind.eq(&KIND_PULL_REQUEST) { + Some(first.clone()) } else { None } @@ -415,7 +458,7 @@ async fn get_root_proposal_id_and_mentions_from_in_reply_to( let mut mention_tags = vec![]; for (i, reply_to) in in_reply_to.iter().enumerate() { - if i.ne(&0) || root_proposal_id.is_none() { + if i.ne(&0) || root_proposal.is_none() { mention_tags.push( event_tag_from_nip19_or_hex( reply_to, @@ -431,7 +474,7 @@ async fn get_root_proposal_id_and_mentions_from_in_reply_to( } } - Ok((root_proposal_id, mention_tags)) + Ok((root_proposal, mention_tags)) } // TODO diff --git a/src/lib/git_events.rs b/src/lib/git_events.rs index 79f5772..bbfcbea 100644 --- a/src/lib/git_events.rs +++ b/src/lib/git_events.rs @@ -376,11 +376,13 @@ pub fn event_tag_from_nip19_or_hex( } } +#[allow(clippy::too_many_arguments)] pub fn generate_unsigned_pr_or_update_event( git_repo: &Repo, repo_ref: &RepoRef, signing_public_key: &PublicKey, root_proposal: Option<&Event>, + title_description_overide: &Option<(String, String)>, commit: &Sha1Hash, clone_url_hint: &[&str], mentions: &[nostr::Tag], @@ -395,13 +397,17 @@ pub fn generate_unsigned_pr_or_update_event( None }; - let title = if let Some(cl) = &root_patch_cover_letter { + let title = if let Some((title, _)) = &title_description_overide { + title.clone() + } else if let Some(cl) = &root_patch_cover_letter { cl.title.clone() } else { git_repo.get_commit_message_summary(commit)? }; - let description = if let Some(cl) = &root_patch_cover_letter { + let description = if let Some((_, description)) = &title_description_overide { + description.clone() + } else if let Some(cl) = &root_patch_cover_letter { cl.description.clone() } else { let mut description = git_repo.get_commit_message(commit)?.trim().to_string(); diff --git a/src/lib/push.rs b/src/lib/push.rs index 1c09555..bcd368b 100644 --- a/src/lib/push.rs +++ b/src/lib/push.rs @@ -321,12 +321,14 @@ impl<'a> PushReporter<'a> { } } +#[allow(clippy::too_many_arguments)] pub async fn push_refs_and_generate_pr_or_pr_update_event( git_repo: &Repo, repo_ref: &RepoRef, tip: &Sha1Hash, user_ref: &UserRef, root_proposal: Option<&Event>, + title_description_overide: &Option<(String, String)>, signer: &Arc, term: &Term, ) -> Result> { @@ -348,6 +350,7 @@ pub async fn push_refs_and_generate_pr_or_pr_update_event( repo_ref, &user_ref.public_key, root_proposal, + title_description_overide, tip, &[clone_url], &[], diff --git a/src/lib/utils.rs b/src/lib/utils.rs index 431757f..431a14f 100644 --- a/src/lib/utils.rs +++ b/src/lib/utils.rs @@ -3,11 +3,13 @@ use std::{ collections::HashMap, fmt, io::{self, Stdin}, + path::Path, str::FromStr, }; use anyhow::{Context, Result, bail}; use git2::Repository; +use nostr::nips::nip19::ToBech32; use nostr_sdk::{Event, EventId, Kind, PublicKey, Url}; use crate::{ @@ -20,7 +22,8 @@ use crate::{ nostr_url::{CloneUrl, NostrUrlDecoded, ServerProtocol}, }, git_events::{ - event_is_revision_root, get_pr_tip_event_or_most_recent_patch_with_ancestors, get_status, + KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE, 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, @@ -187,6 +190,37 @@ pub async fn get_all_proposals( Ok(all_proposals) } +pub async fn proposal_tip_is_pr_or_pr_update( + git_repo_path: &Path, + repo_ref: &RepoRef, + proposal_id: &EventId, +) -> Result { + let commits_events = + get_all_proposal_patch_pr_pr_update_events_from_cache(git_repo_path, repo_ref, proposal_id) + .await + .context(format!( + "cannot get existing proposal events for {}", + proposal_id.to_bech32()? + ))?; + let most_recent_proposal_patch_chain = get_pr_tip_event_or_most_recent_patch_with_ancestors( + commits_events.clone(), + ) + .context(format!( + "cannot find tip from proposal events for {}", + proposal_id.to_bech32()?, + ))?; + + Ok([KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE].contains( + &most_recent_proposal_patch_chain + .first() + .context(format!( + "cannot find any proposal events for {}", + proposal_id.to_bech32()? + ))? + .kind, + )) +} + pub fn find_proposal_and_patches_by_branch_name<'a>( refstr: &'a str, proposals: &'a HashMap)>, diff --git a/test_utils/src/lib.rs b/test_utils/src/lib.rs index 3ae004f..12cac76 100644 --- a/test_utils/src/lib.rs +++ b/test_utils/src/lib.rs @@ -210,7 +210,7 @@ pub fn generate_repo_ref_event_with_git_server_with_keys( } /// enough to fool event_is_patch_set_root pub fn get_pretend_proposal_root_event() -> nostr::Event { - serde_json::from_str(r#"{"id":"431e58eb8e1b4e20292d1d5bbe81d5cfb042e1bc165de32eddfdd52245a4cce4","pubkey":"f53e4bcd7a9cdef049cf6467d638a1321958acd3b71eb09823fd6fadb023d768","created_at":1721404213,"kind":1617,"tags":[["a","30617:ba882566eff14f3baa976103998c452d27fe95b65a796a6a9f92628bced76fe5:9ee507fc4357d7ee16a5d8901bedcd103f23c17d-consider-it-random"],["a","30617:f53e4bcd7a9cdef049cf6467d638a1321958acd3b71eb09823fd6fadb023d768:9ee507fc4357d7ee16a5d8901bedcd103f23c17d-consider-it-random"],["r","9ee507fc4357d7ee16a5d8901bedcd103f23c17d"],["t","cover-letter"],["alt","git patch cover letter: exampletitle"],["t","root"],["e","8cb75aa4cda10a3a0f3242dc49d36159d30b3185bf63414cf6ce17f5c14a73b1","","mention"],["branch-name","feature"],["p","ba882566eff14f3baa976103998c452d27fe95b65a796a6a9f92628bced76fe5"],["p","f53e4bcd7a9cdef049cf6467d638a1321958acd3b71eb09823fd6fadb023d768"]],"content":"From fe973a840fba2a8ab37dd505c154854a69a6505c Mon Sep 17 00:00:00 2001\nSubject: [PATCH 0/2] exampletitle\n\nexampledescription","sig":"37d5b2338bf9fd9d598e6494ae88af9a8dbd52330cfe9d025ee55e35e2f3f55e931ba039d9f7fed8e6fc40206e47619a24f730f8eddc2a07ccfb3988a5005170"}"#).unwrap() + serde_json::from_str(r#"{"id":"000c104861e34a453481ab23e7de21a6baf475b394479705363b035936732528","pubkey":"f53e4bcd7a9cdef049cf6467d638a1321958acd3b71eb09823fd6fadb023d768","created_at":1754322009,"kind":1617,"tags":[["a","30617:f53e4bcd7a9cdef049cf6467d638a1321958acd3b71eb09823fd6fadb023d768:9ee507fc4357d7ee16a5d8901bedcd103f23c17d-consider-it-random","ws://localhost:8055"],["a","30617:ba882566eff14f3baa976103998c452d27fe95b65a796a6a9f92628bced76fe5:9ee507fc4357d7ee16a5d8901bedcd103f23c17d-consider-it-random","ws://localhost:8055"],["r","9ee507fc4357d7ee16a5d8901bedcd103f23c17d"],["r","232efb37ebc67692c9e9ff58b83c0d3d63971a0a"],["alt","git patch: add t3.md"],["t","root"],["branch-name","feature"],["p","ba882566eff14f3baa976103998c452d27fe95b65a796a6a9f92628bced76fe5"],["commit","232efb37ebc67692c9e9ff58b83c0d3d63971a0a"],["parent-commit","431b84edc0d2fa118d63faa3c2db9c73d630a5ae"],["commit-pgp-sig",""],["description","add t3.md"],["author","Joe Bloggs","joe.bloggs@pm.me","0","0"],["committer","Joe Bloggs","joe.bloggs@pm.me","0","0"]],"content":"From 232efb37ebc67692c9e9ff58b83c0d3d63971a0a Mon Sep 17 00:00:00 2001\nFrom: Joe Bloggs \nDate: Thu, 1 Jan 1970 00:00:00 +0000\nSubject: [PATCH 1/2] add t3.md\n\n---\n t3.md | 1 +\n 1 file changed, 1 insertion(+)\n create mode 100644 t3.md\n\ndiff --git a/t3.md b/t3.md\nnew file mode 100644\nindex 0000000..f0eec86\n--- /dev/null\n+++ b/t3.md\n@@ -0,0 +1 @@\n+some content\n\\ No newline at end of file\n--\nlibgit2 1.9.1\n\n","sig":"65577fea803ea464bb073273a3fbfbdb5bfdaa64fb3b1d029ee8f3729fde051ad90610d08e441335f365b6c1d6f2270909bc37d12433ca82f0b2928b7a503e31"}"#).unwrap() } /// wrapper for a cli testing tool - currently wraps rexpect and dialoguer diff --git a/tests/ngit_send.rs b/tests/ngit_send.rs index ec72667..e128bd9 100644 --- a/tests/ngit_send.rs +++ b/tests/ngit_send.rs @@ -37,6 +37,25 @@ mod when_commits_behind_ask_to_proceed { Ok(test_repo) } + fn create_relay_51() -> Result> { + Ok(Relay::new( + 8051, + None, + Some(&|relay, client_id, subscription_id, _| -> Result<()> { + relay.respond_events( + client_id, + &subscription_id, + &vec![ + generate_test_key_1_metadata_event("fred"), + generate_test_key_1_relay_list_event(), + generate_repo_ref_event(), + ], + )?; + Ok(()) + }), + )) + } + fn expect_confirm_prompt(p: &mut CliTester) -> Result { p.expect("fetching updates...\r\n")?; p.expect_eventually("\r\n")?; // may be 'no updates' or some updates @@ -49,37 +68,62 @@ mod when_commits_behind_ask_to_proceed { ) } - #[test] - fn asked_with_default_no() -> Result<()> { + #[tokio::test] + #[serial] + async fn asked_with_default_no() -> Result<()> { let test_repo = prep_test_repo()?; + let mut r51 = create_relay_51()?; + // // check relay had the right number of events + let cli_tester_handle = std::thread::spawn(move || -> Result<()> { + let mut p = CliTester::new_from_dir(&test_repo.dir, ["send", "HEAD~2"]); + expect_confirm_prompt(&mut p)?; + p.exit()?; + relay::shutdown_relay(8051)?; + Ok(()) + }); - let mut p = CliTester::new_from_dir(&test_repo.dir, ["send", "HEAD~2"]); - expect_confirm_prompt(&mut p)?; - p.exit()?; + // launch relay + r51.listen_until_close().await?; + cli_tester_handle.join().unwrap()?; Ok(()) } - #[test] - fn when_response_is_false_aborts() -> Result<()> { + #[tokio::test] + #[serial] + async fn when_response_is_false_aborts() -> Result<()> { let test_repo = prep_test_repo()?; + let mut r51 = create_relay_51()?; + let cli_tester_handle = std::thread::spawn(move || -> Result<()> { + let mut p = CliTester::new_from_dir(&test_repo.dir, ["send", "HEAD~2"]); + expect_confirm_prompt(&mut p)?.succeeds_with(Some(false))?; + p.expect_end_with("Error: aborting so commits can be rebased\r\n")?; + relay::shutdown_relay(8051)?; + Ok(()) + }); - let mut p = CliTester::new_from_dir(&test_repo.dir, ["send", "HEAD~2"]); - - expect_confirm_prompt(&mut p)?.succeeds_with(Some(false))?; - - p.expect_end_with("Error: aborting so commits can be rebased\r\n")?; - + // launch relay + r51.listen_until_close().await?; + cli_tester_handle.join().unwrap()?; Ok(()) } - #[test] + + #[tokio::test] #[serial] - fn when_response_is_true_proceeds() -> Result<()> { + async fn when_response_is_true_proceeds() -> Result<()> { let test_repo = prep_test_repo()?; + let mut r51 = create_relay_51()?; + let cli_tester_handle = std::thread::spawn(move || -> Result<()> { + let mut p = CliTester::new_from_dir(&test_repo.dir, ["send", "HEAD~2"]); + expect_confirm_prompt(&mut p)?.succeeds_with(Some(true))?; + p.expect("? include cover letter")?; + p.exit()?; + relay::shutdown_relay(8051)?; + Ok(()) + }); - let mut p = CliTester::new_from_dir(&test_repo.dir, ["send", "HEAD~2"]); - expect_confirm_prompt(&mut p)?.succeeds_with(Some(true))?; - p.expect("? include cover letter")?; - p.exit()?; + // launch relay + r51.listen_until_close().await?; + cli_tester_handle.join().unwrap()?; Ok(()) } } @@ -1620,7 +1664,7 @@ mod root_proposal_specified_using_in_reply_to_with_range_of_head_2_and_cover_let .unwrap() .as_slice()[1], // id of state nevent - "431e58eb8e1b4e20292d1d5bbe81d5cfb042e1bc165de32eddfdd52245a4cce4", + "000c104861e34a453481ab23e7de21a6baf475b394479705363b035936732528", ); } Ok(()) -- cgit v1.2.3 From dee39c39116773fde22c4fe30a87d54d1d3658e2 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Tue, 5 Aug 2025 11:08:26 +0100 Subject: feat(send): push PR to custom clone url if the repo doesnt list any grasp servers, or pushing to them fails --- src/bin/git_remote_nostr/push.rs | 44 +++++++++++++++++++-------- src/bin/ngit/sub_commands/send.rs | 64 ++++++++++++++++++++++++++++++--------- src/lib/push.rs | 55 ++++++++++++++------------------- 3 files changed, 103 insertions(+), 60 deletions(-) (limited to 'src/lib/push.rs') diff --git a/src/bin/git_remote_nostr/push.rs b/src/bin/git_remote_nostr/push.rs index 3967699..4552b91 100644 --- a/src/bin/git_remote_nostr/push.rs +++ b/src/bin/git_remote_nostr/push.rs @@ -450,18 +450,38 @@ async fn generate_patches_or_pr_event_or_pr_updates( signer: &Arc, term: &Term, ) -> Result> { - let mut events: Vec = vec![]; let parent_is_pr = root_proposal.is_some_and(|proposal| proposal.kind.eq(&KIND_PULL_REQUEST)); let use_pr = parent_is_pr || git_repo.are_commits_too_big_for_patches(ahead); if use_pr { - for event in push_refs_and_generate_pr_or_pr_update_event( + let repo_grasps = repo_ref.grasp_servers(); + let repo_grasp_clone_urls: Vec = repo_ref + .git_server + .iter() + .filter(|s| is_grasp_server(s, &repo_grasps)) + .cloned() + .collect(); + + if repo_grasp_clone_urls.is_empty() { + // 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 + + bail!( + "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." + ); + } + + if let (Some(events), _) = push_refs_and_generate_pr_or_pr_update_event( git_repo, repo_ref, ahead.first().context("no commits to push")?, user_ref, root_proposal, &None, + &repo_grasp_clone_urls, signer, term, ) @@ -471,12 +491,17 @@ async fn generate_patches_or_pr_event_or_pr_updates( } else { "a commit in your proposal is too big for a nostr patch so we tried to create it as a nostr PR instead. Unfortunately this failed." } - )? - { - events.push(event); + )? { + Ok(events) + } else { + 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( + generate_cover_letter_and_patch_events( None, git_repo, ahead, @@ -485,13 +510,8 @@ async fn generate_patches_or_pr_event_or_pr_updates( &root_proposal.map(|proposal| proposal.id.to_string()), &[], ) - .await? - { - events.push(patch); - } + .await } - - Ok(events) } type HashMapUrlRefspecs = HashMap>; diff --git a/src/bin/ngit/sub_commands/send.rs b/src/bin/ngit/sub_commands/send.rs index 0aefb03..69ad1e6 100644 --- a/src/bin/ngit/sub_commands/send.rs +++ b/src/bin/ngit/sub_commands/send.rs @@ -1,11 +1,13 @@ -use std::path::Path; +use std::{path::Path, str::FromStr}; use anyhow::{Context, Result, bail}; use console::Style; use ngit::{ client::{Params, send_events}, + git::nostr_url::CloneUrl, git_events::{EventRefType, KIND_PULL_REQUEST, generate_cover_letter_and_patch_events}, push::push_refs_and_generate_pr_or_pr_update_event, + repo_ref::is_grasp_server, utils::proposal_tip_is_pr_or_pr_update, }; use nostr::{ToBech32, event::Event, nips::nip19::Nip19Event}; @@ -192,20 +194,52 @@ pub async fn launch(cli_args: &Cli, args: &SubCommandArgs, no_fetch: bool) -> Re commits.reverse(); let events = if as_pr { - push_refs_and_generate_pr_or_pr_update_event( - &git_repo, - &repo_ref, - commits.last().context("no commits")?, - &user_ref, - root_proposal.as_ref(), - &cover_letter_title_description, - &signer, - &console::Term::stdout(), - ) - .await? - - // TODO - // - allow specifying clone url and ref + let repo_grasps = repo_ref.grasp_servers(); + let repo_grasp_clone_urls: Vec = repo_ref + .git_server + .iter() + .filter(|s| is_grasp_server(s, &repo_grasps)) + .cloned() + .collect(); + if repo_grasp_clone_urls.is_empty() { + println!( + "The repository doesn't list a grasp server which would otherwise be used to submit your proposal as nostr Pull Request." + ); + } + let mut to_try = repo_grasp_clone_urls.clone(); + let mut tried = vec![]; + loop { + let (events, _server_responses) = push_refs_and_generate_pr_or_pr_update_event( + &git_repo, + &repo_ref, + commits.last().context("no commits")?, + &user_ref, + root_proposal.as_ref(), + &cover_letter_title_description, + &repo_grasp_clone_urls, + &signer, + &console::Term::stdout(), + ) + .await?; + for url in to_try { + tried.push(url); + } + to_try = vec![]; + if let Some(events) = events { + break events; + } + let clone_url = Interactor::default() + .input( + PromptInputParms::default().with_prompt("git repo url with write permission"), + )? + .clone(); + if CloneUrl::from_str(&clone_url).is_ok() { + to_try.push(clone_url); + // TODO customise ref to push + } else { + println!("invalid clone url"); + } + } } else { let events = generate_cover_letter_and_patch_events( cover_letter_title_description.clone(), diff --git a/src/lib/push.rs b/src/lib/push.rs index bcd368b..4c2d8f1 100644 --- a/src/lib/push.rs +++ b/src/lib/push.rs @@ -4,7 +4,7 @@ use std::{ time::Instant, }; -use anyhow::{Result, anyhow, bail}; +use anyhow::{Result, anyhow}; use auth_git2::GitAuthenticator; use console::Term; use nostr::{ @@ -25,7 +25,7 @@ use crate::{ }, git_events::generate_unsigned_pr_or_update_event, login::user::UserRef, - repo_ref::{RepoRef, is_grasp_server, normalize_grasp_server_url}, + repo_ref::{RepoRef, normalize_grasp_server_url}, utils::{ Direction, get_short_git_server_name, get_write_protocols_to_try, join_with_and, set_protocol_preference, @@ -329,19 +329,14 @@ pub async fn push_refs_and_generate_pr_or_pr_update_event( user_ref: &UserRef, root_proposal: Option<&Event>, title_description_overide: &Option<(String, String)>, + servers: &[String], signer: &Arc, term: &Term, -) -> Result> { - let mut events: Vec = vec![]; - 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)); +) -> Result<(Option>, Vec<(String, Result<()>)>)> { + let mut responses = vec![]; let mut unsigned_pr_event: Option = None; - let mut failed_clone_urls = vec![]; - for clone_url in repo_grasp_clone_urls { + for clone_url in servers { let mut draft_pr_event = if let Some(ref unsigned_pr_event) = unsigned_pr_event { unsigned_pr_event.clone() } else { @@ -360,7 +355,6 @@ pub async fn push_refs_and_generate_pr_or_pr_update_event( let refspec = format!("{}:refs/nostr/{}", tip, 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}", @@ -368,7 +362,9 @@ pub async fn push_refs_and_generate_pr_or_pr_update_event( ) .as_str(), )?; + responses.push((clone_url.clone(), Err(error))); } else { + responses.push((clone_url.clone(), Ok(()))); term.write_line( format!( "push: commit data sent to {}", @@ -379,17 +375,6 @@ pub async fn push_refs_and_generate_pr_or_pr_update_event( unsigned_pr_event = Some(draft_pr_event); } } - if unsigned_pr_event.is_none() { - bail!( - "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 - // 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, @@ -404,21 +389,25 @@ pub async fn push_refs_and_generate_pr_or_pr_update_event( .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()) + Ok(( + Some(vec![ + pr_event, + create_close_status_for_original_patch( + signer, + repo_ref, + root_proposal.unwrap(), + ) .await?, - ); + ]), + responses, + )) + } else { + Ok((Some(vec![pr_event]), responses)) } } else { - 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 + Ok((None, responses)) } - Ok(events) } async fn create_close_status_for_original_patch( -- cgit v1.2.3 From 29f61ffdf155ea88b8d9aec23d28cf70baba577e Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Tue, 5 Aug 2025 11:38:09 +0100 Subject: feat(send): custom ref for PR clone url allow specifying ref for pushing PR to custom clone url --- src/bin/git_remote_nostr/push.rs | 1 + src/bin/ngit/sub_commands/send.rs | 14 +++++++++++++- src/lib/push.rs | 8 +++++++- 3 files changed, 21 insertions(+), 2 deletions(-) (limited to 'src/lib/push.rs') diff --git a/src/bin/git_remote_nostr/push.rs b/src/bin/git_remote_nostr/push.rs index 4552b91..f98e792 100644 --- a/src/bin/git_remote_nostr/push.rs +++ b/src/bin/git_remote_nostr/push.rs @@ -482,6 +482,7 @@ async fn generate_patches_or_pr_event_or_pr_updates( root_proposal, &None, &repo_grasp_clone_urls, + None, signer, term, ) diff --git a/src/bin/ngit/sub_commands/send.rs b/src/bin/ngit/sub_commands/send.rs index 69ad1e6..609812b 100644 --- a/src/bin/ngit/sub_commands/send.rs +++ b/src/bin/ngit/sub_commands/send.rs @@ -208,6 +208,7 @@ pub async fn launch(cli_args: &Cli, args: &SubCommandArgs, no_fetch: bool) -> Re } let mut to_try = repo_grasp_clone_urls.clone(); let mut tried = vec![]; + let mut git_ref = None; loop { let (events, _server_responses) = push_refs_and_generate_pr_or_pr_update_event( &git_repo, @@ -217,6 +218,7 @@ pub async fn launch(cli_args: &Cli, args: &SubCommandArgs, no_fetch: bool) -> Re root_proposal.as_ref(), &cover_letter_title_description, &repo_grasp_clone_urls, + git_ref.clone(), &signer, &console::Term::stdout(), ) @@ -235,7 +237,17 @@ pub async fn launch(cli_args: &Cli, args: &SubCommandArgs, no_fetch: bool) -> Re .clone(); if CloneUrl::from_str(&clone_url).is_ok() { to_try.push(clone_url); - // TODO customise ref to push + let mut git_ref_or_branch_name = Interactor::default() + .input( + PromptInputParms::default() + .with_prompt("ref / branch name") + .with_default(git_ref.unwrap_or("refs/nostr/".to_string())), + )? + .clone(); + if !git_ref_or_branch_name.starts_with("refs/") { + git_ref_or_branch_name = format!("refs/heads/{git_ref_or_branch_name}"); + } + git_ref = Some(git_ref_or_branch_name); } else { println!("invalid clone url"); } diff --git a/src/lib/push.rs b/src/lib/push.rs index 4c2d8f1..c202397 100644 --- a/src/lib/push.rs +++ b/src/lib/push.rs @@ -330,6 +330,7 @@ pub async fn push_refs_and_generate_pr_or_pr_update_event( root_proposal: Option<&Event>, title_description_overide: &Option<(String, String)>, servers: &[String], + git_ref: Option, signer: &Arc, term: &Term, ) -> Result<(Option>, Vec<(String, Result<()>)>)> { @@ -352,7 +353,12 @@ pub async fn push_refs_and_generate_pr_or_pr_update_event( )? }; - let refspec = format!("{}:refs/nostr/{}", tip, draft_pr_event.id()); + let git_ref_used = git_ref + .clone() + .unwrap_or("refs/nostr/".to_string()) + .replace("", &draft_pr_event.id().to_string()); + + let refspec = format!("{tip}:{git_ref_used}"); if let Err(error) = push_to_remote_url(git_repo, clone_url, &[refspec], term) { term.write_line( -- cgit v1.2.3 From b61ce3469057be9e081ceb01d3a42337ace6a7c4 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Wed, 6 Aug 2025 17:21:04 +0100 Subject: fix(push): capture more error updating refs previously server might respond with errors updating refs but we were not treating this as a failure to push those refs. --- src/lib/push.rs | 92 ++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 58 insertions(+), 34 deletions(-) (limited to 'src/lib/push.rs') diff --git a/src/lib/push.rs b/src/lib/push.rs index c202397..2aafc1d 100644 --- a/src/lib/push.rs +++ b/src/lib/push.rs @@ -1,5 +1,5 @@ use std::{ - collections::HashSet, + collections::{HashMap, HashSet}, sync::{Arc, Mutex}, time::Instant, }; @@ -52,18 +52,33 @@ pub fn push_to_remote( let formatted_url = server_url.format_as(protocol, &decoded_nostr_url.user)?; - if let Err(error) = push_to_remote_url(git_repo, &formatted_url, remote_refspecs, term) { - term.write_line( - format!("push: {formatted_url} failed over {protocol}: {error}").as_str(), - )?; - failed_protocols.push(protocol); - } else { - success = true; - if !failed_protocols.is_empty() { - term.write_line(format!("push: succeeded over {protocol}").as_str())?; - let _ = set_protocol_preference(git_repo, protocol, &server_url, &Direction::Push); + match push_to_remote_url(git_repo, &formatted_url, remote_refspecs, term) { + Err(error) => { + term.write_line( + format!("push: {formatted_url} failed over {protocol}: {error}").as_str(), + )?; + failed_protocols.push(protocol); + } + Ok(failed_refs) => { + if let Some((_, error)) = failed_refs.iter().next() { + term.write_line( + format!("push: {formatted_url} failed over {protocol}: {error}").as_str(), + )?; + failed_protocols.push(protocol); + } else { + success = true; + if !failed_protocols.is_empty() { + term.write_line(format!("push: succeeded over {protocol}").as_str())?; + let _ = set_protocol_preference( + git_repo, + protocol, + &server_url, + &Direction::Push, + ); + } + break; + } } - break; } } if success { @@ -84,12 +99,13 @@ pub fn push_to_remote( } } +// returns failed refs as a HashMaps of failed refspec and their error pub fn push_to_remote_url( git_repo: &Repo, git_server_url: &str, remote_refspecs: &[String], term: &Term, -) -> Result<()> { +) -> Result> { let git_config = git_repo.git_repo.config()?; let mut git_server_remote = git_repo.git_repo.remote_anonymous(git_server_url)?; let auth = GitAuthenticator::default(); @@ -105,6 +121,9 @@ pub fn push_to_remote_url( let mut reporter = push_reporter.lock().unwrap(); if let Some(error) = error { let existing_lines = reporter.count_all_existing_lines(); + reporter + .failed_refs + .insert(name.to_string(), error.to_string()); reporter.update_reference_errors.push(format!( "WARNING: {} failed to push {name} error: {error}", get_short_git_server_name(git_repo, git_server_url), @@ -186,7 +205,8 @@ pub fn push_to_remote_url( push_options.remote_callbacks(remote_callbacks); git_server_remote.push(remote_refspecs, Some(&mut push_options))?; let _ = git_server_remote.disconnect(); - Ok(()) + let reporter = push_reporter.lock().unwrap(); + Ok(reporter.failed_refs.clone()) } #[allow(clippy::cast_precision_loss)] @@ -235,6 +255,7 @@ pub struct PushReporter<'a> { negotiation: Vec, transfer_progress_msgs: Vec, update_reference_errors: Vec, + failed_refs: HashMap, term: &'a console::Term, start_time: Option, end_time: Option, @@ -246,6 +267,7 @@ impl<'a> PushReporter<'a> { negotiation: vec![], transfer_progress_msgs: vec![], update_reference_errors: vec![], + failed_refs: HashMap::new(), term, start_time: None, end_time: None, @@ -334,7 +356,7 @@ pub async fn push_refs_and_generate_pr_or_pr_update_event( signer: &Arc, term: &Term, ) -> Result<(Option>, Vec<(String, Result<()>)>)> { - let mut responses = vec![]; + let mut responses: Vec<(String, Result<()>)> = vec![]; let mut unsigned_pr_event: Option = None; for clone_url in servers { @@ -360,25 +382,27 @@ pub async fn push_refs_and_generate_pr_or_pr_update_event( let refspec = format!("{tip}:{git_ref_used}"); - if let Err(error) = push_to_remote_url(git_repo, clone_url, &[refspec], term) { - term.write_line( - format!( - "push: error sending commit data to {}: {error}", - normalize_grasp_server_url(clone_url)? - ) - .as_str(), - )?; - responses.push((clone_url.clone(), Err(error))); - } else { - responses.push((clone_url.clone(), Ok(()))); - term.write_line( - format!( - "push: commit data sent to {}", - normalize_grasp_server_url(clone_url)? - ) - .as_str(), - )?; - unsigned_pr_event = Some(draft_pr_event); + match push_to_remote_url(git_repo, clone_url, &[refspec], term) { + Err(error) => { + let normalized_url = normalize_grasp_server_url(clone_url)?; + term.write_line(&format!( + "push: error sending commit data to {normalized_url}: {error}" + ))?; + responses.push((clone_url.clone(), Err(anyhow!(error)))); + } + Ok(failed_refs) => { + let normalized_url = normalize_grasp_server_url(clone_url)?; + if let Some((_, error)) = failed_refs.iter().next() { + term.write_line(&format!( + "push: error sending commit data to {normalized_url}: {error}" + ))?; + responses.push((clone_url.clone(), Err(anyhow!(error.clone())))); + } else { + responses.push((clone_url.clone(), Ok(()))); + term.write_line(&format!("push: commit data sent to {normalized_url}"))?; + unsigned_pr_event = Some(draft_pr_event); + } + } } } if let Some(unsigned_pr_event) = unsigned_pr_event { -- cgit v1.2.3 From 646d05e44946c5a248cb8c5b7d852ed316b9592e Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Thu, 7 Aug 2025 10:21:28 +0100 Subject: fix(send): push PR refs to non-grasp servers attempt to use a range of protocols instead of unath http --- src/bin/ngit/sub_commands/sync.rs | 66 ++++++++++++++++++++++++--------------- src/lib/push.rs | 64 ++++++++++++++++++++++++++++--------- 2 files changed, 90 insertions(+), 40 deletions(-) (limited to 'src/lib/push.rs') diff --git a/src/bin/ngit/sub_commands/sync.rs b/src/bin/ngit/sub_commands/sync.rs index c1a3484..e7033b2 100644 --- a/src/bin/ngit/sub_commands/sync.rs +++ b/src/bin/ngit/sub_commands/sync.rs @@ -127,33 +127,47 @@ pub async fn launch(args: &SubCommandArgs) -> Result<()> { term.write_line(&format!("{remote_name} already in sync"))?; } // report already in sync - } else if let Err(error) = push_to_remote( - &git_repo, - url, - &decoded_nostr_url, - &refspecs, - &term, - *is_grasp_server, - ) { - term.write_line(&format!( - "error pushing updates to {remote_name}: error: {error}" - ))?; - } else if *is_grasp_server || args.force { - term.write_line(&format!("{remote_name} sync completed"))?; - // TODO we only know if there was an error but not if it - // rejected any updates } else { - // we should report on refs not force pushed - term.write_line(&format!("{remote_name} sync completed"))?; - } - for name in ¬_deleted { - term.write_line(&format!(" - {name} not deleted"))?; - } - for name in ¬_updated { - term.write_line(&format!(" - {name} not updated due to conflicts"))?; - } - if !not_updated.is_empty() || !not_deleted.is_empty() { - term.write_line("run `ngit sync --force` to delete refs or overwrite conflicts and potentially lose work")?; + match push_to_remote( + &git_repo, + url, + &decoded_nostr_url, + &refspecs, + &term, + *is_grasp_server, + ) { + Err(error) => { + term.write_line(&format!( + "error pushing updates to {remote_name}: error: {error}" + ))?; + } + Ok(failed_refs) => { + if failed_refs.is_empty() { + if *is_grasp_server || args.force { + term.write_line(&format!("{remote_name} sync completed"))?; + // TODO we only know if there was an error but not + // if it rejected any + // updates + } else { + // we should report on refs not force pushed + term.write_line(&format!("{remote_name} sync completed"))?; + } + } else { + term.write_line(&format!( + "{remote_name} sync completed but not all changes were accepted" + ))?; + } + for name in ¬_deleted { + term.write_line(&format!(" - {name} not deleted"))?; + } + for name in ¬_updated { + term.write_line(&format!(" - {name} not updated due to conflicts"))?; + } + if !not_updated.is_empty() || !not_deleted.is_empty() { + term.write_line("run `ngit sync --force` to delete refs or overwrite conflicts and potentially lose work")?; + } + } + } } } diff --git a/src/lib/push.rs b/src/lib/push.rs index 2aafc1d..a5a29a2 100644 --- a/src/lib/push.rs +++ b/src/lib/push.rs @@ -4,7 +4,7 @@ use std::{ time::Instant, }; -use anyhow::{Result, anyhow}; +use anyhow::{Context, Result, anyhow}; use auth_git2::GitAuthenticator; use console::Term; use nostr::{ @@ -19,19 +19,20 @@ use crate::{ cli_interactor::count_lines_per_msg_vec, client::{sign_draft_event, sign_event}, git::{ - Repo, + Repo, RepoActions, nostr_url::{CloneUrl, NostrUrlDecoded}, oid_to_shorthand_string, }, git_events::generate_unsigned_pr_or_update_event, login::user::UserRef, - repo_ref::{RepoRef, normalize_grasp_server_url}, + repo_ref::{RepoRef, is_grasp_server_clone_url, normalize_grasp_server_url}, utils::{ Direction, get_short_git_server_name, get_write_protocols_to_try, join_with_and, set_protocol_preference, }, }; +// returns failed refs as a HashMaps of failed refspec and their error pub fn push_to_remote( git_repo: &Repo, git_server_url: &str, @@ -39,13 +40,14 @@ pub fn push_to_remote( remote_refspecs: &[String], term: &Term, is_grasp_server: bool, -) -> Result<()> { +) -> Result> { let server_url = git_server_url.parse::()?; let protocols_to_attempt = get_write_protocols_to_try(git_repo, &server_url, decoded_nostr_url, is_grasp_server); let mut failed_protocols = vec![]; let mut success = false; + let mut failed_refs = HashMap::new(); for protocol in &protocols_to_attempt { term.write_line(format!("push: {} over {protocol}...", server_url.short_name(),).as_str())?; @@ -59,14 +61,9 @@ pub fn push_to_remote( )?; failed_protocols.push(protocol); } - Ok(failed_refs) => { - if let Some((_, error)) = failed_refs.iter().next() { - term.write_line( - format!("push: {formatted_url} failed over {protocol}: {error}").as_str(), - )?; - failed_protocols.push(protocol); - } else { - success = true; + Ok(failed_refs_on_protocol) => { + success = true; + if failed_refs_on_protocol.is_empty() { if !failed_protocols.is_empty() { term.write_line(format!("push: succeeded over {protocol}").as_str())?; let _ = set_protocol_preference( @@ -77,12 +74,25 @@ pub fn push_to_remote( ); } break; + } else { + term.write_line( + format!( + "push: {formatted_url} with {protocol} complete but {}ref{} not accepted:", + if remote_refspecs.len() != failed_protocols.len() { "some " } else {""}, + if remote_refspecs.len() == 1 { "s"} else {""}, + ).as_str(), + )?; + for (git_ref, error) in &failed_refs_on_protocol { + term.write_line(format!("push: - {git_ref}: {error}").as_str())?; + } + failed_refs = failed_refs_on_protocol; } + break; } } } if success { - Ok(()) + Ok(failed_refs) } else { let error = anyhow!( "{} failed over {}{}", @@ -382,7 +392,33 @@ pub async fn push_refs_and_generate_pr_or_pr_update_event( let refspec = format!("{tip}:{git_ref_used}"); - match push_to_remote_url(git_repo, clone_url, &[refspec], term) { + let res = if is_grasp_server_clone_url(clone_url) { + push_to_remote_url(git_repo, clone_url, &[refspec], term) + } else { + // anticipated only when pushing to user's own repo or a personal-fork with + // non-grasp git servers. this is used to extract prefered protocols / ssh + // details from nostr url + let decoded_nostr_url = { + if let Ok(Some((_, decoded_nostr_url))) = git_repo + .get_first_nostr_remote_when_in_ngit_binary() + .await.context("failed to list git remotes") + .context("no `nostr://` remote detected. `ngit sync` must be run from a repo with a nostr remote") { + decoded_nostr_url + } else { + repo_ref.to_nostr_git_url(&Some(git_repo)) + } + }; + push_to_remote( + git_repo, + clone_url, + &decoded_nostr_url, + &[refspec], + term, + false, + ) + }; + + match res { Err(error) => { let normalized_url = normalize_grasp_server_url(clone_url)?; term.write_line(&format!( -- cgit v1.2.3 From 3b5c48f5a2a4b9be5d14baa8f5e801fefd5c1166 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Thu, 7 Aug 2025 12:55:14 +0100 Subject: fix(send):server not confirming acceptance don't treat this as accepted --- src/lib/push.rs | 61 +++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 21 deletions(-) (limited to 'src/lib/push.rs') diff --git a/src/lib/push.rs b/src/lib/push.rs index a5a29a2..0ee66cf 100644 --- a/src/lib/push.rs +++ b/src/lib/push.rs @@ -32,7 +32,7 @@ use crate::{ }, }; -// returns failed refs as a HashMaps of failed refspec and their error +// returns a HashMap of refs responded to and any related cancellation reasons pub fn push_to_remote( git_repo: &Repo, git_server_url: &str, @@ -40,14 +40,14 @@ pub fn push_to_remote( remote_refspecs: &[String], term: &Term, is_grasp_server: bool, -) -> Result> { +) -> Result>> { let server_url = git_server_url.parse::()?; let protocols_to_attempt = get_write_protocols_to_try(git_repo, &server_url, decoded_nostr_url, is_grasp_server); let mut failed_protocols = vec![]; let mut success = false; - let mut failed_refs = HashMap::new(); + let mut ref_updates = HashMap::new(); for protocol in &protocols_to_attempt { term.write_line(format!("push: {} over {protocol}...", server_url.short_name(),).as_str())?; @@ -61,9 +61,13 @@ pub fn push_to_remote( )?; failed_protocols.push(protocol); } - Ok(failed_refs_on_protocol) => { + Ok(ref_updates_on_protocol) => { success = true; - if failed_refs_on_protocol.is_empty() { + if remote_refspecs.len() == ref_updates_on_protocol.len() + && ref_updates_on_protocol + .values() + .all(|error| error.is_none()) + { if !failed_protocols.is_empty() { term.write_line(format!("push: succeeded over {protocol}").as_str())?; let _ = set_protocol_preference( @@ -80,19 +84,22 @@ pub fn push_to_remote( "push: {formatted_url} with {protocol} complete but {}ref{} not accepted:", if remote_refspecs.len() != failed_protocols.len() { "some " } else {""}, if remote_refspecs.len() == 1 { "s"} else {""}, - ).as_str(), + ).as_str(), )?; - for (git_ref, error) in &failed_refs_on_protocol { - term.write_line(format!("push: - {git_ref}: {error}").as_str())?; + for (git_ref, error) in &ref_updates_on_protocol { + if let Some(error) = error { + term.write_line(format!("push: - {git_ref}: {error}").as_str())?; + } } - failed_refs = failed_refs_on_protocol; + // TODO do we want to report on the refs that weren't responded to? + ref_updates = ref_updates_on_protocol; } break; } } } if success { - Ok(failed_refs) + Ok(ref_updates) } else { let error = anyhow!( "{} failed over {}{}", @@ -109,13 +116,13 @@ pub fn push_to_remote( } } -// returns failed refs as a HashMaps of failed refspec and their error +// returns HashMaps of refspecs responded to and any failure message pub fn push_to_remote_url( git_repo: &Repo, git_server_url: &str, remote_refspecs: &[String], term: &Term, -) -> Result> { +) -> Result>> { let git_config = git_repo.git_repo.config()?; let mut git_server_remote = git_repo.git_repo.remote_anonymous(git_server_url)?; let auth = GitAuthenticator::default(); @@ -129,11 +136,11 @@ pub fn push_to_remote_url( let push_reporter = Arc::clone(&push_reporter); move |name, error| { let mut reporter = push_reporter.lock().unwrap(); + reporter + .ref_updates + .insert(name.to_string(), error.map(|s| s.to_string())); if let Some(error) = error { let existing_lines = reporter.count_all_existing_lines(); - reporter - .failed_refs - .insert(name.to_string(), error.to_string()); reporter.update_reference_errors.push(format!( "WARNING: {} failed to push {name} error: {error}", get_short_git_server_name(git_repo, git_server_url), @@ -156,7 +163,11 @@ pub fn push_to_remote_url( .unwrap_or("") .replace("refs/heads/", "") .replace("refs/tags/", "tags/"); - let msg = if update.dst().is_zero() { + let msg = if let Some(Some(_)) = + reporter.ref_updates.get(update.dst_refname().unwrap_or("")) + { + format!("push: - [failed] {dst_refname}") + } else if update.dst().is_zero() { format!("push: - [delete] {dst_refname}") } else if update.src().is_zero() { if update.dst_refname().unwrap_or("").contains("refs/tags") { @@ -216,7 +227,7 @@ pub fn push_to_remote_url( git_server_remote.push(remote_refspecs, Some(&mut push_options))?; let _ = git_server_remote.disconnect(); let reporter = push_reporter.lock().unwrap(); - Ok(reporter.failed_refs.clone()) + Ok(reporter.ref_updates.clone()) } #[allow(clippy::cast_precision_loss)] @@ -265,7 +276,7 @@ pub struct PushReporter<'a> { negotiation: Vec, transfer_progress_msgs: Vec, update_reference_errors: Vec, - failed_refs: HashMap, + ref_updates: HashMap>, term: &'a console::Term, start_time: Option, end_time: Option, @@ -277,7 +288,7 @@ impl<'a> PushReporter<'a> { negotiation: vec![], transfer_progress_msgs: vec![], update_reference_errors: vec![], - failed_refs: HashMap::new(), + ref_updates: HashMap::new(), term, start_time: None, end_time: None, @@ -426,13 +437,21 @@ pub async fn push_refs_and_generate_pr_or_pr_update_event( ))?; responses.push((clone_url.clone(), Err(anyhow!(error)))); } - Ok(failed_refs) => { + Ok(ref_updates) => { let normalized_url = normalize_grasp_server_url(clone_url)?; - if let Some((_, error)) = failed_refs.iter().next() { + if let Some((_, Some(error))) = ref_updates.iter().next() { term.write_line(&format!( "push: error sending commit data to {normalized_url}: {error}" ))?; responses.push((clone_url.clone(), Err(anyhow!(error.clone())))); + } else if ref_updates.is_empty() { + term.write_line(&format!( + "push: error sending commit data to {normalized_url}: server didn't confirm acceptance" + ))?; + responses.push(( + clone_url.clone(), + Err(anyhow!("server didn't confirm acceptance")), + )); } else { responses.push((clone_url.clone(), Ok(()))); term.write_line(&format!("push: commit data sent to {normalized_url}"))?; -- cgit v1.2.3 From fa7adf840ac2d78defee398a61b60888f615622a Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Thu, 7 Aug 2025 17:49:02 +0100 Subject: fix(send): refs not confirmed are usually accepted having implemented 3b5c48f5a2a4b9be5d14baa8f5e801fefd5c1166, a ref pushed to refs/nostr/ on a github repo was accepted but was not confirmed --- src/bin/ngit/sub_commands/sync.rs | 4 ++-- src/lib/push.rs | 15 +++------------ 2 files changed, 5 insertions(+), 14 deletions(-) (limited to 'src/lib/push.rs') diff --git a/src/bin/ngit/sub_commands/sync.rs b/src/bin/ngit/sub_commands/sync.rs index e7033b2..00dfe75 100644 --- a/src/bin/ngit/sub_commands/sync.rs +++ b/src/bin/ngit/sub_commands/sync.rs @@ -141,8 +141,8 @@ pub async fn launch(args: &SubCommandArgs) -> Result<()> { "error pushing updates to {remote_name}: error: {error}" ))?; } - Ok(failed_refs) => { - if failed_refs.is_empty() { + Ok(updated_refs) => { + if updated_refs.values().all(std::option::Option::is_none) { if *is_grasp_server || args.force { term.write_line(&format!("{remote_name} sync completed"))?; // TODO we only know if there was an error but not diff --git a/src/lib/push.rs b/src/lib/push.rs index 0ee66cf..8cb0212 100644 --- a/src/lib/push.rs +++ b/src/lib/push.rs @@ -63,10 +63,9 @@ pub fn push_to_remote( } Ok(ref_updates_on_protocol) => { success = true; - if remote_refspecs.len() == ref_updates_on_protocol.len() - && ref_updates_on_protocol - .values() - .all(|error| error.is_none()) + if ref_updates_on_protocol + .values() + .all(|error| error.is_none()) { if !failed_protocols.is_empty() { term.write_line(format!("push: succeeded over {protocol}").as_str())?; @@ -444,14 +443,6 @@ pub async fn push_refs_and_generate_pr_or_pr_update_event( "push: error sending commit data to {normalized_url}: {error}" ))?; responses.push((clone_url.clone(), Err(anyhow!(error.clone())))); - } else if ref_updates.is_empty() { - term.write_line(&format!( - "push: error sending commit data to {normalized_url}: server didn't confirm acceptance" - ))?; - responses.push(( - clone_url.clone(), - Err(anyhow!("server didn't confirm acceptance")), - )); } else { responses.push((clone_url.clone(), Ok(()))); term.write_line(&format!("push: commit data sent to {normalized_url}"))?; -- cgit v1.2.3