From ccdbc337ae16d7c70be44166269c8b2d5b9f5c09 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 2 Feb 2024 06:44:28 +0000 Subject: feat(prs-create)!: use nip34 patch kind and tags - change kind number - remove "r-" prefix from unique commit id r tag - rename tag commit-sig to commit-pgp-sig - a tag for repo identifer and pubkey. this serves as a vote for this pubkey being a maintainer - add relay hints - change format of committer tag - remove r references to parent commit id - tag parent patch event if its part of change request author and commit-message tags still need to be removed but they are required to apply patches with gitlib2. we will need to fallback to running the git client to apply patches. BREAKING CHANGE: change patch/commit event kind and tags to reflect nip34 draft. events with the older kind will no longer be found and will not be in a valid format --- src/sub_commands/prs/create.rs | 143 ++++++++++++++++++++++++++++------------- src/sub_commands/prs/list.rs | 41 +----------- src/sub_commands/pull.rs | 6 +- src/sub_commands/push.rs | 18 ++++-- 4 files changed, 112 insertions(+), 96 deletions(-) (limited to 'src/sub_commands') diff --git a/src/sub_commands/prs/create.rs b/src/sub_commands/prs/create.rs index ad6a61a..0f7cbda 100644 --- a/src/sub_commands/prs/create.rs +++ b/src/sub_commands/prs/create.rs @@ -13,7 +13,9 @@ use crate::{ cli_interactor::{Interactor, InteractorPrompt, PromptConfirmParms, PromptInputParms}, client::Connect, git::{Repo, RepoActions}, - login, repo_ref, Cli, + login, + repo_ref::{self, RepoRef, REPO_REF_KIND}, + Cli, }; #[derive(Debug, clap::Args)] @@ -96,9 +98,6 @@ pub async fn launch( client.set_keys(&keys).await; - let events = - generate_pr_and_patch_events(&title, &description, &to_branch, &git_repo, &ahead, &keys)?; - let repo_ref = repo_ref::fetch( &git_repo, git_repo @@ -110,6 +109,16 @@ pub async fn launch( ) .await?; + let events = generate_pr_and_patch_events( + &title, + &description, + &to_branch, + &git_repo, + &ahead, + &keys, + &repo_ref, + )?; + println!( "posting 1 pull request with {} commits...", events.len() - 1 @@ -299,7 +308,7 @@ mod tests_unique_and_duplicate { } pub static PR_KIND: u64 = 318; -pub static PATCH_KIND: u64 = 317; +pub static PATCH_KIND: u64 = 1617; pub fn generate_pr_and_patch_events( title: &str, @@ -308,6 +317,7 @@ pub fn generate_pr_and_patch_events( git_repo: &Repo, commits: &Vec, keys: &nostr::Keys, + repo_ref: &RepoRef, ) -> Result> { let root_commit = git_repo .get_root_commit(to_branch) @@ -342,8 +352,16 @@ pub fn generate_pr_and_patch_events( let mut events = vec![pr_event]; for commit in commits { events.push( - generate_patch_event(git_repo, &root_commit, commit, pr_event_id, keys) - .context("failed to generate patch event")?, + generate_patch_event( + git_repo, + &root_commit, + commit, + pr_event_id, + keys, + repo_ref, + events.last().map(nostr::Event::id), + ) + .context("failed to generate patch event")?, ); } Ok(events) @@ -353,55 +371,92 @@ pub fn generate_patch_event( git_repo: &Repo, root_commit: &Sha1Hash, commit: &Sha1Hash, - pr_event_id: nostr::EventId, + thread_event_id: nostr::EventId, keys: &nostr::Keys, + repo_ref: &RepoRef, + parent_patch_event_id: Option, ) -> Result { let commit_parent = git_repo .get_commit_parent(commit) .context("failed to get parent commit")?; + let relay_hint = repo_ref.relays.first().map(nostr::UncheckedUrl::from); EventBuilder::new( nostr::event::Kind::Custom(PATCH_KIND), git_repo .make_patch_from_commit(commit) .context(format!("cannot make patch for commit {commit}"))?, [ - Tag::Reference(format!("r-{root_commit}")), - Tag::Reference(commit.to_string()), - Tag::Reference(commit_parent.to_string()), - Tag::Event { - event_id: pr_event_id, - relay_url: None, // TODO: add relay - marker: Some(Marker::Root), + vec![ + Tag::A { + kind: nostr::Kind::Custom(REPO_REF_KIND), + public_key: *repo_ref.maintainers.first() + .context("repo reference should always have at least one maintainer - the issuer of the repo event") + ?, + identifier: repo_ref.identifier.to_string(), + relay_url: relay_hint.clone(), + }, + Tag::Reference(format!("{root_commit}")), + // commit id reference is a trade-off. its now + // unclear which one is the root commit id but it + // enables easier location of code comments againt + // code that makes it into the main branch, assuming + // the commit id is correct + Tag::Reference(commit.to_string()), + + Tag::Event { + event_id: thread_event_id, + relay_url: relay_hint.clone(), + marker: Some(Marker::Root), + }, + ], + if let Some(id) = parent_patch_event_id { + vec![Tag::Event { + event_id: id, + relay_url: relay_hint.clone(), + marker: Some(Marker::Reply), + }] + } else { + vec![] }, - Tag::Generic( - TagKind::Custom("commit".to_string()), - vec![commit.to_string()], - ), - Tag::Generic( - TagKind::Custom("parent-commit".to_string()), - vec![commit_parent.to_string()], - ), - Tag::Generic( - TagKind::Custom("commit-sig".to_string()), - vec![ - git_repo - .extract_commit_pgp_signature(commit) - .unwrap_or_default(), - ], - ), - Tag::Description(git_repo.get_commit_message(commit)?.to_string()), - Tag::Generic( - TagKind::Custom("author".to_string()), - git_repo.get_commit_author(commit)?, - ), - Tag::Generic( - TagKind::Custom("committer".to_string()), - git_repo.get_commit_comitter(commit)?, - ), - // TODO: add Repo event tags - // TODO: people tag maintainers - // TODO: add relay tags - ], + // whilst it is in nip34 draft to tag the maintainers + // I'm not sure it is a good idea because if they are + // interested in all patches then their specialised + // client should subscribe to patches tagged with the + // repo reference. maintainers of large repos will not + // be interested in every patch. + repo_ref.maintainers + .iter() + .map(|pk| Tag::public_key(*pk)) + .collect(), + vec![ + Tag::Generic( + TagKind::Custom("commit".to_string()), + vec![commit.to_string()], + ), + Tag::Generic( + TagKind::Custom("parent-commit".to_string()), + vec![commit_parent.to_string()], + ), + Tag::Generic( + TagKind::Custom("commit-pgp-sig".to_string()), + vec![ + git_repo + .extract_commit_pgp_signature(commit) + .unwrap_or_default(), + ], + ), + Tag::Description(git_repo.get_commit_message(commit)?.to_string()), + Tag::Generic( + TagKind::Custom("author".to_string()), + git_repo.get_commit_author(commit)?, + ), + Tag::Generic( + TagKind::Custom("committer".to_string()), + git_repo.get_commit_comitter(commit)?, + ), + ], + ] + .concat(), ) .to_event(keys) .context("failed to sign event") diff --git a/src/sub_commands/prs/list.rs b/src/sub_commands/prs/list.rs index f0b4ad9..96004d4 100644 --- a/src/sub_commands/prs/list.rs +++ b/src/sub_commands/prs/list.rs @@ -115,8 +115,7 @@ pub async fn launch( vec![ nostr::Filter::default() .kind(nostr::Kind::Custom(PATCH_KIND)) - .event(pr_events[selected_index].id) - .reference(format!("r-{root_commit}")), + .event(pr_events[selected_index].id), ], ) .await? @@ -127,9 +126,6 @@ pub async fn launch( t.as_vec().len() > 2 && t.as_vec()[1].eq(&pr_events[selected_index].id.to_string()) }) - && e.tags - .iter() - .any(|t| t.as_vec().len() > 1 && t.as_vec()[1].eq(&format!("r-{root_commit}"))) }) .map(std::borrow::ToOwned::to_owned) .collect(); @@ -154,41 +150,6 @@ pub async fn launch( ); } - // // TODO: look for mapping of existing branch - - // // if latest_commit_id exists locally - // if local_branch_base == latest_commit_id { - // // TODO: check if its in the main / master branch (already merged) - // // TODO: check if it has any decendants and warn. maybe the user has - // // been working on a updates to be pushed? Suggest checking - // // out that branch. - // // we could search nostr for decendants of the commit as well? - // // perhaps this is overkill - // // TODO: check out the branch which it is the tip of. if the name of the - // // branch is different then ask the user if they would like to - // // use the existing branch or create one with the name of the PR. - // // TODO: if there are no decendants and its not the tip then - // // its an ophan commit so just make a branch from this commit. - // } - // // if commits ahead exist in a branch other than main or master - // // TODO: Identify probable existing branches - check remote tracker? - // // TODO: beind head - // else { - // // TODO: look for existing branch with same name - // // TODO: create remote tracker - // git_repo.create_branch_at_commit(&branch_name, &local_branch_base); - // git_repo.checkout(&branch_name)?; - // ahead.reverse(); - // for event in ahead { - // git_repo.apply_patch(event, branch_name)?; - // } - // println!("applied!") - // } - // // TODO: check if commits in pr exist, if so look for branches with they are - // in // could we suggest pulling updates into that branch? - // // - - // TODO: checkout PR branch Ok(()) } diff --git a/src/sub_commands/pull.rs b/src/sub_commands/pull.rs index a8f9529..342c8bb 100644 --- a/src/sub_commands/pull.rs +++ b/src/sub_commands/pull.rs @@ -78,8 +78,7 @@ pub async fn launch() -> Result<()> { vec![ nostr::Filter::default() .kind(nostr::Kind::Custom(PATCH_KIND)) - .event(pr_event.id) - .reference(format!("r-{root_commit}")), + .event(pr_event.id), ], ) .await? @@ -89,9 +88,6 @@ pub async fn launch() -> Result<()> { && e.tags .iter() .any(|t| t.as_vec().len() > 2 && t.as_vec()[1].eq(&pr_event.id.to_string())) - && e.tags - .iter() - .any(|t| t.as_vec().len() > 1 && t.as_vec()[1].eq(&format!("r-{root_commit}"))) }) .map(std::borrow::ToOwned::to_owned) .collect(); diff --git a/src/sub_commands/push.rs b/src/sub_commands/push.rs index e592cd7..fac746f 100644 --- a/src/sub_commands/push.rs +++ b/src/sub_commands/push.rs @@ -102,8 +102,16 @@ pub async fn launch(cli_args: &Cli) -> Result<()> { let mut patch_events: Vec = vec![]; for commit in &ahead { patch_events.push( - generate_patch_event(&git_repo, &root_commit, commit, pr_event.id, &keys) - .context("cannot make patch event from commit")?, + generate_patch_event( + &git_repo, + &root_commit, + commit, + pr_event.id, + &keys, + &repo_ref, + patch_events.last().map(nostr::Event::id), + ) + .context("cannot make patch event from commit")?, ); } println!("pushing {} commits", ahead.len()); @@ -162,8 +170,7 @@ async fn fetch_pr_and_most_recent_patch_chain( vec![ nostr::Filter::default() .kind(nostr::Kind::Custom(PATCH_KIND)) - .event(pr_event.id) - .reference(format!("r-{root_commit}")), + .event(pr_event.id), ], ) .await? @@ -173,9 +180,6 @@ async fn fetch_pr_and_most_recent_patch_chain( && e.tags .iter() .any(|t| t.as_vec().len() > 2 && t.as_vec()[1].eq(&pr_event.id.to_string())) - && e.tags - .iter() - .any(|t| t.as_vec().len() > 1 && t.as_vec()[1].eq(&format!("r-{root_commit}"))) }) .map(std::borrow::ToOwned::to_owned) .collect(); -- cgit v1.2.3