Skip to content

Commit

Permalink
feat(prs-create)!: use nip34 patch kind and tags
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
DanConwayDev committed Feb 2, 2024
1 parent 401e60b commit ccdbc33
Show file tree
Hide file tree
Showing 7 changed files with 140 additions and 131 deletions.
45 changes: 22 additions & 23 deletions src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ pub trait RepoActions {
fn get_head_commit(&self) -> Result<Sha1Hash>;
fn get_commit_parent(&self, commit: &Sha1Hash) -> Result<Sha1Hash>;
fn get_commit_message(&self, commit: &Sha1Hash) -> Result<String>;
/// returns vector ["name", "email", "unixtime"]
/// eg ["joe bloggs", "[email protected]", "12176,-300"]
/// returns vector ["name", "email", "unixtime", "offset"]
/// eg ["joe bloggs", "[email protected]", "12176","-300"]
fn get_commit_author(&self, commit: &Sha1Hash) -> Result<Vec<String>>;
/// returns vector ["name", "email", "unixtime"]
/// eg ["joe bloggs", "[email protected]", "12176,-300"]
/// returns vector ["name", "email", "unixtime", "offset"]
/// eg ["joe bloggs", "[email protected]", "12176","-300"]
fn get_commit_comitter(&self, commit: &Sha1Hash) -> Result<Vec<String>>;
fn get_commits_ahead_behind(
&self,
Expand Down Expand Up @@ -442,7 +442,8 @@ fn git_sig_to_tag_vec(sig: &git2::Signature) -> Vec<String> {
vec![
sig.name().unwrap_or("").to_string(),
sig.email().unwrap_or("").to_string(),
format!("{},{}", sig.when().seconds(), sig.when().offset_minutes()),
format!("{}", sig.when().seconds()),
format!("{}", sig.when().offset_minutes()),
]
}

Expand Down Expand Up @@ -477,7 +478,7 @@ fn apply_patch(git_repo: &Repo, patch: &nostr::Event) -> Result<()> {
index.add_all(["."], git2::IndexAddOption::DEFAULT, None)?;
index.write()?;

let pgp_sig = if let Ok(pgp_sig) = tag_value(patch, "commit-sig") {
let pgp_sig = if let Ok(pgp_sig) = tag_value(patch, "commit-pgp-sig") {
if pgp_sig.is_empty() {
None
} else {
Expand Down Expand Up @@ -585,22 +586,15 @@ fn extract_sig_from_patch_tags<'a>(
.find(|t| t.as_vec()[0].eq(tag_name))
.context(format!("tag '{tag_name}' not present in patch"))?
.as_vec();
if v.len() != 4 {
if v.len() != 5 {
bail!("tag '{tag_name}' is incorrectly formatted")
}
let git_time: Vec<&str> = v[3].split(',').collect();
if git_time.len() != 2 {
bail!("tag '{tag_name}' time is incorrectly formatted")
}
git2::Signature::new(
v[1].as_str(),
v[2].as_str(),
&git2::Time::new(
git_time[0]
.parse()
.context("tag time is incorrectly formatted")?,
git_time[1]
.parse()
v[3].parse().context("tag time is incorrectly formatted")?,
v[4].parse()
.context("tag time offset is incorrectly formatted")?,
),
)
Expand All @@ -611,7 +605,7 @@ fn extract_sig_from_patch_tags<'a>(
mod tests {
use std::fs;

use test_utils::git::GitTestRepo;
use test_utils::{generate_repo_ref_event, git::GitTestRepo, TEST_KEY_1_KEYS};

use super::*;

Expand Down Expand Up @@ -706,19 +700,22 @@ mod tests {
#[test]
fn no_offset() -> Result<()> {
let res = prep(&git2::Time::new(5000, 0))?;
assert_eq!("5000,0", res[2]);
assert_eq!("5000", res[2]);
assert_eq!("0", res[3]);
Ok(())
}
#[test]
fn positive_offset() -> Result<()> {
let res = prep(&git2::Time::new(5000, 300))?;
assert_eq!("5000,300", res[2]);
assert_eq!("5000", res[2]);
assert_eq!("300", res[3]);
Ok(())
}
#[test]
fn negative_offset() -> Result<()> {
let res = prep(&git2::Time::new(5000, -300))?;
assert_eq!("5000,-300", res[2]);
assert_eq!("5000", res[2]);
assert_eq!("-300", res[3]);
Ok(())
}
}
Expand Down Expand Up @@ -1205,10 +1202,9 @@ mod tests {
}

mod apply_patch {
use test_utils::TEST_KEY_1_KEYS;

use super::*;
use crate::sub_commands::prs::create::generate_patch_event;
use crate::{repo_ref::RepoRef, sub_commands::prs::create::generate_patch_event};

fn generate_patch_from_head_commit(test_repo: &GitTestRepo) -> Result<nostr::Event> {
let original_oid = test_repo.git_repo.head()?.peel_to_commit()?.id();
Expand All @@ -1219,6 +1215,8 @@ mod tests {
&oid_to_sha1(&original_oid),
nostr::EventId::all_zeros(),
&TEST_KEY_1_KEYS,
&RepoRef::try_from(generate_repo_ref_event()).unwrap(),
None,
)
}
fn test_patch_applies_to_repository(patch_event: nostr::Event) -> Result<()> {
Expand Down Expand Up @@ -1358,7 +1356,7 @@ mod tests {
use test_utils::TEST_KEY_1_KEYS;

use super::*;
use crate::sub_commands::prs::create::generate_pr_and_patch_events;
use crate::{repo_ref::RepoRef, sub_commands::prs::create::generate_pr_and_patch_events};

static BRANCH_NAME: &str = "add-example-feature";
// returns original_repo, pr_event, patch_events
Expand All @@ -1378,6 +1376,7 @@ mod tests {
&git_repo,
&vec![oid_to_sha1(&oid1), oid_to_sha1(&oid2), oid_to_sha1(&oid3)],
&TEST_KEY_1_KEYS,
&RepoRef::try_from(generate_repo_ref_event()).unwrap(),
)?;

events.reverse();
Expand Down
143 changes: 99 additions & 44 deletions src/sub_commands/prs/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -308,6 +317,7 @@ pub fn generate_pr_and_patch_events(
git_repo: &Repo,
commits: &Vec<Sha1Hash>,
keys: &nostr::Keys,
repo_ref: &RepoRef,
) -> Result<Vec<nostr::Event>> {
let root_commit = git_repo
.get_root_commit(to_branch)
Expand Down Expand Up @@ -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)
Expand All @@ -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<nostr::EventId>,
) -> Result<nostr::Event> {
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")
Expand Down
41 changes: 1 addition & 40 deletions src/sub_commands/prs/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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();
Expand All @@ -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(())
}

Expand Down
Loading

0 comments on commit ccdbc33

Please sign in to comment.