From f0d0e1ba1cba11d3a98a5ab0c7f1dc72b6bc4e17 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 6 Dec 2024 22:16:44 +0000 Subject: feat(push): send fast-forward merge status event when a proposal was merged using fast-forward status rather than by creating a three way merge commit. if there are multiple revisions, the first one that contains merged proposal tip will be referenced. if there are multiple proposals that contain one of the commits, they will all be marked as merged. the nip needs to be updated as there is no single `merge-commit-id` so that tag needs to support multiple values --- src/bin/git_remote_nostr/push.rs | 249 +++++++++++++++++++++++++++++---------- tests/git_remote_nostr/push.rs | 193 ++++++++++++++++++++++++++++-- 2 files changed, 369 insertions(+), 73 deletions(-) diff --git a/src/bin/git_remote_nostr/push.rs b/src/bin/git_remote_nostr/push.rs index dde4ab0..40e9584 100644 --- a/src/bin/git_remote_nostr/push.rs +++ b/src/bin/git_remote_nostr/push.rs @@ -25,7 +25,7 @@ use ngit::{ nostr_url::{CloneUrl, NostrUrlDecoded}, oid_to_shorthand_string, }, - git_events::{self, get_event_root}, + git_events::{self, event_to_cover_letter, get_event_root}, login::{self, get_curent_user}, repo_ref::{self, get_repo_config_from_yaml}, repo_state, @@ -965,72 +965,181 @@ async fn get_merged_status_events( }; let (ahead, _) = git_repo.get_commits_ahead_behind(&tip_of_remote_branch, &tip_of_pushed_branch)?; - for commit_hash in ahead { - let commit = git_repo.git_repo.find_commit(sha1_to_oid(&commit_hash)?)?; - if commit.parent_count() > 1 { - // merge commit - for parent in commit.parents() { - // lookup parent id - let commit_events = get_events_from_local_cache( - git_repo.get_path()?, - vec![ - nostr::Filter::default() - .kind(nostr::Kind::GitPatch) - .reference(parent.id().to_string()), - ], - ) - .await?; - if let Some(commit_event) = commit_events.iter().find(|e| { - e.tags.iter().any(|t| { - t.as_slice()[0].eq("commit") - && t.as_slice()[1].eq(&parent.id().to_string()) - }) - }) { - let (proposal_id, revision_id) = - get_proposal_and_revision_root_from_patch(git_repo, commit_event) - .await?; - term.write_line( - format!( - "merge commit {}: create nostr proposal status event", - &commit.id().to_string()[..7], - ) - .as_str(), - )?; - events.push( - create_merge_status( - signer, - repo_ref, - &get_event_from_cache_by_id(git_repo, &proposal_id).await?, - &if let Some(revision_id) = revision_id { - Some( - get_event_from_cache_by_id(git_repo, &revision_id) - .await?, - ) - } else { - None - }, - &commit_hash, - commit_event.id, - ) - .await?, - ); + let commit_events = get_events_from_local_cache( + git_repo.get_path()?, + vec![ + nostr::Filter::default().kind(nostr::Kind::GitPatch), + // TODO: limit by repo_ref + ], + ) + .await?; + + let merged_proposals_info = + get_merged_proposals_info(git_repo, &ahead, &commit_events).await?; + + for event in + create_merge_events(term, git_repo, repo_ref, signer, &merged_proposals_info) + .await? + { + events.push(event); + } + } + } + Ok(events) +} + +/// (`proposal_id`, `revision_id`) +type MergedProposalsInfo = + HashMap, HashMap)>; + +async fn get_merged_proposals_info( + git_repo: &Repo, + ahead: &Vec, + available_patches: &[Event], +) -> Result { + let mut proposals: MergedProposalsInfo = HashMap::new(); + + for commit_hash in ahead { + let commit = git_repo.git_repo.find_commit(sha1_to_oid(commit_hash)?)?; + // three-way merge - just to set merge commit id as the merged branch commits + // are in ahead + if commit.parent_count() > 1 { + for parent in commit.parents() { + for patch_event in available_patches + .iter() + .filter(|e| { + e.tags.iter().any(|t| { + t.as_slice()[0].eq("commit") + && 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 + { + let (entry_revision_id, merged_patches) = + proposals.entry(proposal_id).or_default(); + if entry_revision_id == &revision_id { + merged_patches.insert(*commit_hash, MergedPRCommitType::MergeCommit); } } } } + } else { + // three way merge or fast forward merge commits + // note: ahead included commits of three-way merged branches + for patch_event in available_patches + .iter() + .filter(|e| { + e.tags.iter().any(|t| { + t.as_slice()[0].eq("commit") && t.as_slice()[1].eq(&commit_hash.to_string()) + }) + }) + .collect::>() + { + if let Ok((proposal_id, revision_id)) = + get_proposal_and_revision_root_from_patch(git_repo, patch_event).await + { + let (entry_revision_id, merged_patches) = + proposals.entry(proposal_id).or_default(); + // ignore revisions without all the merged commits + if entry_revision_id == &revision_id { + merged_patches.insert( + *commit_hash, + MergedPRCommitType::PatchCommit { + event_id: patch_event.id, + }, + ); + } + } + } + } + } + Ok(proposals) +} + +async fn create_merge_events( + term: &console::Term, + git_repo: &Repo, + repo_ref: &RepoRef, + signer: &Arc, + merged_proposals_info: &MergedProposalsInfo, +) -> Result> { + let mut events = vec![]; + for (proposal_id, (revision_id, merged_patches)) in merged_proposals_info { + let proposal = get_event_from_cache_by_id(git_repo, proposal_id).await?; + + if merged_patches + .values() + .any(|m| *m == MergedPRCommitType::MergeCommit) + { + term.write_line( + format!( + "merge commit {}: create nostr proposal status event", + &merged_patches.keys().next().unwrap().to_string()[..7], + ) + .as_str(), + )?; + } else { + term.write_line( + format!( + "fast-forward merge: create nostr proposal status event for {}", + event_to_cover_letter(&proposal)?.get_branch_name()?, + ) + .as_str(), + )?; } + events.push( + create_merge_status( + signer, + repo_ref, + &proposal, + &if let Some(revision_id) = revision_id { + Some(get_event_from_cache_by_id(git_repo, revision_id).await?) + } else { + None + }, + if merged_patches + .values() + .any(|m| m == &MergedPRCommitType::MergeCommit) + { + vec![*merged_patches.keys().next().unwrap()] + } else { + let mut t: Vec = merged_patches.keys().copied().collect(); + t.reverse(); + t + }, + merged_patches + .values() + .filter_map(|m| match m { + MergedPRCommitType::MergeCommit => None, + MergedPRCommitType::PatchApplied { event_id } + | MergedPRCommitType::PatchCommit { event_id } => Some(*event_id), + }) + .collect(), + ) + .await?, + ); } Ok(events) } +#[derive(PartialEq)] +enum MergedPRCommitType { + MergeCommit, + PatchCommit { event_id: EventId }, + PatchApplied { event_id: EventId }, +} + async fn create_merge_status( signer: &Arc, repo_ref: &RepoRef, proposal: &Event, revision: &Option, - merge_commit: &Sha1Hash, - merged_patch: EventId, + merge_commits: Vec, + merged_patches: Vec, ) -> Result { let mut public_keys = repo_ref .maintainers @@ -1056,14 +1165,20 @@ async fn create_merge_status( public_key: None, uppercase: false, }), - Tag::from_standardized(nostr::TagStandard::Event { - event_id: merged_patch, - relay_url: repo_ref.relays.first().cloned(), - marker: Some(Marker::Mention), - public_key: None, - uppercase: false, - }), ], + // Tags for merged patches + merged_patches + .iter() + .map(|merged_patch| { + Tag::from_standardized(nostr::TagStandard::Event { + event_id: *merged_patch, + relay_url: repo_ref.relays.first().cloned(), + marker: Some(Marker::Mention), + public_key: None, + uppercase: false, + }) + }) + .collect::>(), if let Some(revision) = revision { vec![Tag::from_standardized(nostr::TagStandard::Event { event_id: revision.id, @@ -1085,14 +1200,22 @@ async fn create_merge_status( Tag::from_standardized(nostr::TagStandard::Reference( repo_ref.root_commit.to_string(), )), - Tag::from_standardized(nostr::TagStandard::Reference(format!( - "{merge_commit}" - ))), Tag::custom( nostr::TagKind::Custom(std::borrow::Cow::Borrowed("merge-commit-id")), - vec![format!("{merge_commit}")], + merge_commits + .iter() + .map(|merge_commit| format!("{merge_commit}")) + .collect::>(), ), ], + merge_commits + .iter() + .map(|merge_commit| { + Tag::from_standardized(nostr::TagStandard::Reference(format!( + "{merge_commit}" + ))) + }) + .collect::>(), ] .concat(), ), diff --git a/tests/git_remote_nostr/push.rs b/tests/git_remote_nostr/push.rs index b93475c..4dc2c1d 100644 --- a/tests/git_remote_nostr/push.rs +++ b/tests/git_remote_nostr/push.rs @@ -894,7 +894,8 @@ async fn pushes_to_all_git_servers_listed_and_ok_printed() -> Result<()> { #[tokio::test] #[serial] -async fn proposal_merge_commit_pushed_to_main_leads_to_status_event_issued() -> Result<()> { +async fn proposal_three_way_merge_commit_pushed_to_main_leads_to_status_event_issued() -> Result<()> +{ // let (events, source_git_repo) = prep_source_repo_and_events_including_proposals().await?; let source_path = source_git_repo.dir.to_str().unwrap().to_string(); @@ -958,11 +959,14 @@ async fn proposal_merge_commit_pushed_to_main_leads_to_status_event_issued() -> r57.listen_until_close(), ); - let (output, oid) = cli_tester_handle.join().unwrap()?; + let (output, merge_oid) = cli_tester_handle.join().unwrap()?; assert_eq!( output, - format!(" 431b84e..{} main -> main\r\n", &oid.to_string()[..7]) + format!( + " 431b84e..{} main -> main\r\n", + &merge_oid.to_string()[..7] + ) ); let new_events = r55 @@ -976,7 +980,7 @@ async fn proposal_merge_commit_pushed_to_main_leads_to_status_event_issued() -> assert_eq!(new_events.len(), 2, "{new_events:?}"); - let proposal = r55 + let proposal_cover_letter_event = r55 .events .iter() .find(|e| { @@ -993,14 +997,15 @@ async fn proposal_merge_commit_pushed_to_main_leads_to_status_event_issued() -> .unwrap(); assert_eq!( - oid.to_string(), + vec!["merge-commit-id".to_string(), merge_oid.to_string()], merge_status .tags .iter() .find(|t| t.as_slice()[0].eq("merge-commit-id")) .unwrap() - .as_slice()[1], - "status sets correct merge-commit-id tag" + .clone() + .to_vec(), + "status sets correct merge-commit-id tag {merge_status:?}" ); let proposal_tip = r55 @@ -1009,7 +1014,7 @@ async fn proposal_merge_commit_pushed_to_main_leads_to_status_event_issued() -> .filter(|e| { e.tags .iter() - .any(|t| t.as_slice()[1].eq(&proposal.id.to_string())) + .any(|t| t.as_slice()[1].eq(&proposal_cover_letter_event.id.to_string())) && e.kind.eq(&Kind::GitPatch) }) .last() @@ -1029,7 +1034,175 @@ async fn proposal_merge_commit_pushed_to_main_leads_to_status_event_issued() -> ); assert_eq!( - proposal.id.to_string(), + proposal_cover_letter_event.id.to_string(), + merge_status + .tags + .iter() + .find(|t| t.is_root()) + .unwrap() + .as_slice()[1], + "status tags proposal id as root \r\nmerge status:\r\n{}\r\nproposal:\r\n{}", + merge_status.as_json(), + proposal_cover_letter_event.as_json(), + ); + + Ok(()) +} + +#[tokio::test] +#[serial] +async fn proposal_fast_forward_merge_commits_pushed_to_main_leads_to_status_event_issued() +-> Result<()> { + // + let (events, source_git_repo) = prep_source_repo_and_events_including_proposals().await?; + let source_path = source_git_repo.dir.to_str().unwrap().to_string(); + + let (mut r51, mut r52, mut r53, mut r55, mut r56, mut r57) = ( + Relay::new(8051, None, None), + Relay::new(8052, None, None), + Relay::new(8053, None, None), + Relay::new(8055, None, None), + Relay::new(8056, None, None), + Relay::new(8057, None, None), + ); + r51.events = events.clone(); + r55.events = events.clone(); + + #[allow(clippy::mutable_key_type)] + let before = r55.events.iter().cloned().collect::>(); + + let cli_tester_handle = std::thread::spawn(move || -> Result<(String, Oid)> { + let branch_name = get_proposal_branch_name_from_events(&events, FEATURE_BRANCH_NAME_1)?; + + let git_repo = clone_git_repo_with_nostr_url()?; + git_repo.checkout_remote_branch(&branch_name)?; + git_repo.checkout("refs/heads/main")?; + + CliTester::new_git_with_remote_helper_from_dir( + &git_repo.dir, + ["merge", &branch_name, "-m", "proposal merge commit message"], + ) + .expect_end_eventually_and_print()?; + + let oid = git_repo.get_tip_of_local_branch("main")?; + + let mut p = CliTester::new_git_with_remote_helper_from_dir(&git_repo.dir, ["push"]); + cli_expect_nostr_fetch(&mut p)?; + p.expect(format!("fetching {} ref list over filesystem...\r\n", source_path).as_str())?; + p.expect("list: connecting...\r\n")?; + p.expect_eventually(format!( + "fast-forward merge: create nostr proposal status event for {branch_name}\r\n" + ))?; + // status updates printed here + p.expect_eventually(format!("To {}\r\n", get_nostr_remote_url()?).as_str())?; + let output = p.expect_end_eventually()?; + + for p in [51, 52, 53, 55, 56, 57] { + relay::shutdown_relay(8000 + p)?; + } + + Ok((output, oid)) + }); + // launch relays + let _ = join!( + r51.listen_until_close(), + r52.listen_until_close(), + r53.listen_until_close(), + r55.listen_until_close(), + r56.listen_until_close(), + r57.listen_until_close(), + ); + + let (output, tip_oid) = cli_tester_handle.join().unwrap()?; + + assert_eq!( + output, + format!( + " 431b84e..{} main -> main\r\n", + &tip_oid.to_string()[..7] + ) + ); + + let new_events = r55 + .events + .iter() + .cloned() + .collect::>() + .difference(&before) + .cloned() + .collect::>(); + + assert_eq!(new_events.len(), 2, "{new_events:?}"); + + let proposal_cover_letter_event = r55 + .events + .iter() + .find(|e| { + e.tags + .iter() + .find(|t| t.as_slice()[0].eq("branch-name")) + .is_some_and(|t| t.as_slice()[1].eq(FEATURE_BRANCH_NAME_1)) + }) + .unwrap(); + + let proposal_patches: Vec<&Event> = r55 + .events + .iter() + .filter(|e| { + e.kind == Kind::GitPatch + && e.tags + .iter() + .any(|t| t.as_slice()[1].eq(&proposal_cover_letter_event.id.to_string())) + }) + .collect(); + + let merge_status = new_events + .iter() + .find(|e| e.kind.eq(&Kind::GitStatusApplied)) + .unwrap(); + // println!("{:?}", proposal_cover_letter_event); + // println!("merge status"); + // println!("{:?}", merge_status); + + let patch_commit_ids = proposal_patches + .iter() + .map(|e| { + e.tags + .iter() + .find(|t| t.as_slice()[0].eq("commit")) + .unwrap() + .as_slice()[1] + .to_string() + }) + .collect::>(); + assert_eq!( + [vec!["merge-commit-id".to_string()], patch_commit_ids].concat(), + merge_status + .tags + .iter() + .find(|t| t.as_slice()[0].eq("merge-commit-id")) + .unwrap() + .clone() + .to_vec(), + "status sets correct merge-commit-id tag {merge_status:?}" + ); + + for patch_id in proposal_patches + .iter() + .map(|e| e.id.to_string()) + .collect::>() + { + assert!( + merge_status.tags.iter().any(|t| t.as_slice().len().eq(&4) + && t.as_slice()[1] == patch_id + && t.as_slice()[3].eq("mention")), + "merge status doesnt mention proposal patch {patch_id} \r\nmerge status:\r\n{}", + merge_status.as_json(), + ); + } + + assert_eq!( + proposal_cover_letter_event.id.to_string(), merge_status .tags .iter() @@ -1038,7 +1211,7 @@ async fn proposal_merge_commit_pushed_to_main_leads_to_status_event_issued() -> .as_slice()[1], "status tags proposal id as root \r\nmerge status:\r\n{}\r\nproposal:\r\n{}", merge_status.as_json(), - proposal.as_json(), + proposal_cover_letter_event.as_json(), ); Ok(()) -- cgit v1.2.3