From 9971f23a184d57600ea9b1962910d32bf6aec185 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Thu, 8 Aug 2024 17:34:40 +0100 Subject: feat(remote): `push` force push proposal will issue a proposal revision --- src/git_remote_helper.rs | 102 +++++++++++++++++++++--------- test_utils/src/git.rs | 5 +- tests/git_remote_helper.rs | 153 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 227 insertions(+), 33 deletions(-) diff --git a/src/git_remote_helper.rs b/src/git_remote_helper.rs index f36b23b..be230b1 100644 --- a/src/git_remote_helper.rs +++ b/src/git_remote_helper.rs @@ -33,7 +33,10 @@ use sub_commands::{ get_most_recent_patch_with_ancestors, get_proposals_and_revisions_from_cache, status_kinds, tag_value, }, - send::{event_is_revision_root, event_to_cover_letter, generate_patch_event, send_events}, + send::{ + event_is_revision_root, event_to_cover_letter, generate_cover_letter_and_patch_events, + generate_patch_event, send_events, + }, }; #[cfg(not(test))] @@ -656,21 +659,42 @@ async fn push( if let Some((_, (proposal, patches))) = find_proposal_and_patches_by_branch_name(to, &open_proposals) { - if to.starts_with('+') { - // TODO do force push - issue as revision - } else { - let tip_patch = patches.first().unwrap(); - let tip_of_proposal = get_commit_id_from_patch(tip_patch)?; - let tip_of_proposal_commit = - git_repo.get_commit_or_tip_of_reference(&tip_of_proposal)?; + if [repo_ref.maintainers.clone(), vec![proposal.author()]] + .concat() + .contains(&user_ref.public_key) + { let tip_of_pushed_branch = git_repo.get_commit_or_tip_of_reference(from)?; - let (mut ahead, behind) = git_repo - .get_commits_ahead_behind(&tip_of_proposal_commit, &tip_of_pushed_branch)?; - if behind.is_empty() { - if [repo_ref.maintainers.clone(), vec![proposal.author()]] - .concat() - .contains(&user_ref.public_key) + if refspec.starts_with('+') { + // force push + let (_, main_tip) = git_repo.get_main_or_master_branch()?; + let (mut ahead, _) = + git_repo.get_commits_ahead_behind(&main_tip, &tip_of_pushed_branch)?; + ahead.reverse(); + for patch in generate_cover_letter_and_patch_events( + None, + git_repo, + &ahead, + &signer, + repo_ref, + &Some(proposal.id().to_string()), + &[], + ) + .await? { + events.push(patch); + } + } else { + // fast forward push + let tip_patch = patches.first().unwrap(); + let tip_of_proposal = get_commit_id_from_patch(tip_patch)?; + let tip_of_proposal_commit = + git_repo.get_commit_or_tip_of_reference(&tip_of_proposal)?; + + let (mut ahead, behind) = git_repo.get_commits_ahead_behind( + &tip_of_proposal_commit, + &tip_of_pushed_branch, + )?; + if behind.is_empty() { let thread_id = if let Ok(root_event_id) = get_event_root(tip_patch) { root_event_id } else { @@ -702,25 +726,25 @@ async fn push( parent_patch = new_patch; } } else { + // we shouldn't get here + term.write_line( + format!( + "WARNING: failed to push {from} as nostr proposal. Try and force push ", + ) + .as_str(), + ) + .unwrap(); println!( - "error {to} permission denied. you are not the proposal author or a repo maintainer" + "error {to} cannot fastforward as newer patches found on proposal" ); rejected_proposal_refspecs.push(refspec.to_string()); } - } else { - // we shouldn't get here - term.write_line( - format!( - "WARNING: failed to push {from} as nostr proposal. Try and force push ", - ) - .as_str(), - ) - .unwrap(); - println!( - "error {to} cannot fastforward as newer patches found on proposal" - ); - rejected_proposal_refspecs.push(refspec.to_string()); } + } else { + println!( + "error {to} permission denied. you are not the proposal author or a repo maintainer" + ); + rejected_proposal_refspecs.push(refspec.to_string()); } } else { // TODO new proposal / proposal no longer open @@ -1046,7 +1070,14 @@ fn refspec_to_from_to(refspec: &str) -> Result<(&str, &str)> { ); } let parts = refspec.split(':').collect::>(); - Ok((parts.first().unwrap(), parts.get(1).unwrap())) + Ok(( + if parts.first().unwrap().starts_with('+') { + &parts.first().unwrap()[1..] + } else { + parts.first().unwrap() + }, + parts.get(1).unwrap(), + )) } fn refspec_remote_ref_name( @@ -1061,7 +1092,8 @@ fn refspec_remote_ref_name( Ok(format!( "refs/remotes/{}/{}", nostr_remote.name().context("remote should have a name")?, - to.replace("refs/heads/", ""), // TODO only replace if it begins with this + to.replace("refs/heads/", ""), /* TODO only replace if it begins with this + * TODO what about tags? */ )) } @@ -1289,4 +1321,14 @@ mod tests { } } } + + mod refspec_to_from_to { + use super::*; + + #[test] + fn trailing_plus_stripped() { + let (from, _) = refspec_to_from_to("+testing:testingb").unwrap(); + assert_eq!(from, "testing"); + } + } } diff --git a/test_utils/src/git.rs b/test_utils/src/git.rs index 5ae7f39..2a3d566 100644 --- a/test_utils/src/git.rs +++ b/test_utils/src/git.rs @@ -274,12 +274,11 @@ impl GitTestRepo { Ok(()) } - pub fn checkout_remote_branch(&self, branch_name: &str) -> Result<()> { + pub fn checkout_remote_branch(&self, branch_name: &str) -> Result { self.checkout(&format!("remotes/origin/{branch_name}"))?; let mut branch = self.create_branch(branch_name)?; branch.set_upstream(Some(&format!("origin/{branch_name}")))?; - self.checkout(branch_name)?; - Ok(()) + self.checkout(branch_name) } } diff --git a/tests/git_remote_helper.rs b/tests/git_remote_helper.rs index dc6d931..379954d 100644 --- a/tests/git_remote_helper.rs +++ b/tests/git_remote_helper.rs @@ -1856,4 +1856,157 @@ mod push { Ok(()) } + + #[tokio::test] + #[serial] + async fn force_push_creates_proposal_revision() -> 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(); + + let before = r55.events.iter().cloned().collect::>(); + + let cli_tester_handle = std::thread::spawn(move || -> Result<(String, String)> { + let branch_name = get_proposal_branch_name_from_events(&events, FEATURE_BRANCH_NAME_1)?; + + let git_repo = clone_git_repo_with_nostr_url()?; + let oid = git_repo.checkout_remote_branch(&branch_name)?; + // remove last commit + git_repo.checkout("main")?; + git_repo.git_repo.branch( + &branch_name, + &git_repo.git_repo.find_commit(oid)?.parent(0)?, + true, + )?; + git_repo.checkout(&branch_name)?; + + std::fs::write(git_repo.dir.join("new.md"), "some content")?; + git_repo.stage_and_commit("new.md")?; + + std::fs::write(git_repo.dir.join("new2.md"), "some content")?; + git_repo.stage_and_commit("new2.md")?; + + let mut p = + CliTester::new_git_with_remote_helper_from_dir(&git_repo.dir, ["push", "--force"]); + cli_expect_nostr_fetch(&mut p)?; + p.expect(format!("fetching refs list: {}...\r\n\r", source_path).as_str())?; + p.expect(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, branch_name)) + }); + // 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, branch_name) = cli_tester_handle.join().unwrap()?; + + assert_eq!( + output, + format!(" + eb5d678...8a296c8 {branch_name} -> {branch_name} (forced update)\r\n") + .as_str(), + ); + + let new_events = r55 + .events + .iter() + .cloned() + .collect::>() + .difference(&before) + .cloned() + .collect::>(); + assert_eq!(new_events.len(), 3); + + let proposal = r55 + .events + .iter() + .find(|e| { + e.iter_tags() + .find(|t| t.as_vec()[0].eq("branch-name")) + .is_some_and(|t| t.as_vec()[1].eq(FEATURE_BRANCH_NAME_1)) + }) + .unwrap(); + + let revision_root_patch = new_events + .iter() + .find(|e| e.iter_tags().any(|t| t.as_vec()[1].eq("revision-root"))) + .unwrap(); + + assert_eq!( + proposal.id().to_string(), + revision_root_patch + .tags + .iter() + .find(|t| t.is_reply()) + .unwrap() + .as_vec()[1], + "revision root patch replies to original proposal" + ); + + assert!( + revision_root_patch.content.contains("[PATCH 1/3]"), + "revision root labeled with [PATCH 1/3]" + ); + + let second_patch = new_events + .iter() + .find(|e| e.content.contains("new.md")) + .unwrap(); + let third_patch = new_events + .iter() + .find(|e| e.content.contains("new2.md")) + .unwrap(); + assert!( + second_patch.content.contains("[PATCH 2/3]"), + "second patch labeled with [PATCH 2/3]" + ); + assert!( + third_patch.content.contains("[PATCH 3/3]"), + "third patch labeled with [PATCH 3/3]" + ); + + assert_eq!( + revision_root_patch.id().to_string(), + second_patch + .tags + .iter() + .find(|t| t.is_root()) + .unwrap() + .as_vec()[1], + "second patch sets revision id as root" + ); + + assert_eq!( + second_patch.id().to_string(), + third_patch + .tags + .iter() + .find(|t| t.is_reply()) + .unwrap() + .as_vec()[1], + "third patch replies to the second new patch" + ); + + Ok(()) + } } -- cgit v1.2.3