From 84d8f03cf2471d3530f4657055f272474880b6b5 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 23 Feb 2024 08:15:24 +0000 Subject: feat(push): add `--force` to issue revision wrapping `send --in-reply-to` unless branch up-to-date --- src/main.rs | 4 +- src/sub_commands/push.rs | 67 ++++++++++++++++--- src/sub_commands/send.rs | 14 ++-- tests/push.rs | 170 +++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 231 insertions(+), 24 deletions(-) diff --git a/src/main.rs b/src/main.rs index 9b9b660..4c49280 100644 --- a/src/main.rs +++ b/src/main.rs @@ -40,7 +40,7 @@ enum Commands { /// list proposals; optionally apply them as a new branch List(sub_commands::list::SubCommandArgs), /// send new commits as proposal amendments - Push, + Push(sub_commands::push::SubCommandArgs), /// pull latest commits in proposal linked to checked out branch Pull, /// run with --nsec flag to change npub @@ -56,6 +56,6 @@ async fn main() -> Result<()> { Commands::Send(args) => sub_commands::send::launch(&cli, args).await, Commands::List(args) => sub_commands::list::launch(&cli, args).await, Commands::Pull => sub_commands::pull::launch().await, - Commands::Push => sub_commands::push::launch(&cli).await, + Commands::Push(args) => sub_commands::push::launch(&cli, args).await, } } diff --git a/src/sub_commands/push.rs b/src/sub_commands/push.rs index dd32b2c..75bff2d 100644 --- a/src/sub_commands/push.rs +++ b/src/sub_commands/push.rs @@ -11,6 +11,7 @@ use crate::{ login, repo_ref::{self, RepoRef}, sub_commands::{ + self, list::{ find_commits_for_proposal_root_events, find_proposal_events, get_commit_id_from_patch, get_most_recent_patch_with_ancestors, tag_value, @@ -20,7 +21,18 @@ use crate::{ Cli, }; -pub async fn launch(cli_args: &Cli) -> Result<()> { +#[derive(Debug, clap::Args)] +pub struct SubCommandArgs { + #[arg(long, action)] + /// send proposal revision from checked out proposal branch + force: bool, + #[arg(long, action)] + /// dont prompt for cover letter when force pushing + no_cover_letter: bool, +} + +#[allow(clippy::too_many_lines)] +pub async fn launch(cli_args: &Cli, args: &SubCommandArgs) -> Result<()> { let git_repo = Repo::discover().context("cannot find a git repository")?; let (main_or_master_branch_name, _) = git_repo @@ -59,26 +71,55 @@ pub async fn launch(cli_args: &Cli) -> Result<()> { ) .await?; - // TODO: fix these scenarios: - // - local proposal branch is 2 behind and 1 ahead. intructions: ... - // - proposal has been rebased. (against commit in main) instructions: ... - // - proposal has been rebased. (against commit not in repo) instructions: .. - let most_recent_proposal_patch_chain = get_most_recent_patch_with_ancestors(commit_events) .context("cannot get most recent patch for proposal")?; let branch_tip = git_repo.get_tip_of_local_branch(&branch_name)?; let most_recent_patch_commit_id = str_to_sha1( - &get_commit_id_from_patch(&most_recent_proposal_patch_chain[0]) - .context("latest patch event doesnt have a commit tag")?, + &get_commit_id_from_patch( + most_recent_proposal_patch_chain + .first() + .context("no patches found")?, + ) + .context("latest patch event doesnt have a commit tag")?, ) .context("latest patch event commit tag isn't a valid SHA1 hash")?; + let proposal_base_commit_id = str_to_sha1( + &tag_value( + most_recent_proposal_patch_chain + .last() + .context("no patches found")?, + "parent-commit", + ) + .context("patch is incorrectly formatted")?, + ) + .context("latest patch event parent-commit tag isn't a valid SHA1 hash")?; + if most_recent_patch_commit_id.eq(&branch_tip) { bail!("proposal already up-to-date with local branch"); } + if args.force { + println!("preparing to force push proposal revision..."); + sub_commands::send::launch( + cli_args, + &sub_commands::send::SubCommandArgs { + starting_commit: String::new(), + in_reply_to: Some(proposal_root_event.id.to_string()), + title: None, + description: None, + from_branch: None, + to_branch: None, + no_cover_letter: args.no_cover_letter, + }, + ) + .await?; + println!("force pushed proposal revision"); + return Ok(()); + } + if most_recent_proposal_patch_chain.iter().any(|e| { let c = tag_value(e, "parent-commit").unwrap_or_default(); c.eq(&branch_tip.to_string()) @@ -86,9 +127,15 @@ pub async fn launch(cli_args: &Cli) -> Result<()> { bail!("proposal is ahead of local branch"); } - let (ahead, behind) = git_repo + let Ok((ahead, behind)) = git_repo .get_commits_ahead_behind(&most_recent_patch_commit_id, &branch_tip) - .context("the latest patch in proposal doesnt share an ancestor with your branch.")?; + .context("the latest patch in proposal doesnt share an ancestor with your branch.") + else { + if git_repo.ancestor_of(&proposal_base_commit_id, &branch_tip)? { + bail!("local unpublished proposal ammendments. consider force pushing."); + } + bail!("local unpublished proposal has been rebased. consider force pushing"); + }; if !behind.is_empty() { bail!( diff --git a/src/sub_commands/send.rs b/src/sub_commands/send.rs index 1ccb1f4..ebe23b1 100644 --- a/src/sub_commands/send.rs +++ b/src/sub_commands/send.rs @@ -27,26 +27,26 @@ pub struct SubCommandArgs { #[arg(default_value = "")] /// starting commit (commits since in current branch) or commit range, like /// in `git format-patch` - starting_commit: String, + pub(crate) starting_commit: String, #[clap(long)] /// nevent or event id of an existing proposal for which this is a new /// version - in_reply_to: Option, + pub(crate) in_reply_to: Option, /// optional cover letter title #[clap(short, long)] - title: Option, + pub(crate) title: Option, #[clap(short, long)] /// optional cover letter description - description: Option, + pub(crate) description: Option, #[clap(long)] /// branch to get changes from (defaults to head) - from_branch: Option, + pub(crate) from_branch: Option, #[clap(long)] /// destination branch (defaults to main or master) - to_branch: Option, + pub(crate) to_branch: Option, /// don't ask about a cover letter #[arg(long, action)] - no_cover_letter: bool, + pub(crate) no_cover_letter: bool, } #[allow(clippy::too_many_lines)] diff --git a/tests/push.rs b/tests/push.rs index d1ad0e6..81daf0e 100644 --- a/tests/push.rs +++ b/tests/push.rs @@ -243,7 +243,7 @@ mod when_branch_is_checked_out { mod cli_prompts { use super::*; - async fn run_async_cli_show_up_to_date() -> Result<()> { + async fn run_async_cli_shows_proposal_ahead_error() -> Result<()> { let (mut r51, mut r52, mut r53, mut r55, mut r56) = ( Relay::new(8051, None, None), Relay::new(8052, None, None), @@ -294,8 +294,8 @@ mod when_branch_is_checked_out { #[tokio::test] #[serial] - async fn cli_show_up_to_date() -> Result<()> { - let _ = run_async_cli_show_up_to_date().await; + async fn cli_show_proposal_ahead_error() -> Result<()> { + let _ = run_async_cli_shows_proposal_ahead_error().await; Ok(()) } } @@ -476,7 +476,167 @@ mod when_branch_is_checked_out { } mod when_branch_has_been_rebased { - // use super::*; - // TODO + use super::*; + + mod cli_prompts { + use super::*; + async fn run_async_cli_shows_unpublished_rebase_error() -> Result<()> { + let (mut r51, mut r52, mut r53, mut r55, mut r56) = ( + 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), + ); + + r51.events.push(generate_test_key_1_relay_list_event()); + r51.events.push(generate_test_key_1_metadata_event("fred")); + r51.events.push(generate_repo_ref_event()); + + r55.events.push(generate_repo_ref_event()); + r55.events.push(generate_test_key_1_metadata_event("fred")); + r55.events.push(generate_test_key_1_relay_list_event()); + + let cli_tester_handle = std::thread::spawn(move || -> Result<()> { + cli_tester_create_proposals()?; + + let test_repo = GitTestRepo::default(); + test_repo.populate()?; + + // simulate rebase + std::fs::write(test_repo.dir.join("amazing.md"), "some content")?; + test_repo.stage_and_commit("commit for rebasing on top of")?; + create_and_populate_branch(&test_repo, FEATURE_BRANCH_NAME_1, "a", true)?; + + let mut p = CliTester::new_from_dir(&test_repo.dir, ["push"]); + // p.expect_end_eventually_and_print()?; + + p.expect("finding proposal root event...\r\n")?; + p.expect("found proposal root event. finding commits...\r\n")?; + p.expect("Error: local unpublished proposal has been rebased. consider force pushing\r\n")?; + p.expect_end()?; + + for p in [51, 52, 53, 55, 56] { + relay::shutdown_relay(8000 + p)?; + } + Ok(()) + }); + + // launch relay + let _ = join!( + r51.listen_until_close(), + r52.listen_until_close(), + r53.listen_until_close(), + r55.listen_until_close(), + r56.listen_until_close(), + ); + cli_tester_handle.join().unwrap()?; + Ok(()) + } + + #[tokio::test] + #[serial] + async fn cli_shows_unpublished_rebase_error() -> Result<()> { + let _ = run_async_cli_shows_unpublished_rebase_error().await; + Ok(()) + } + } + mod with_force_flag { + use super::*; + + mod cli_prompts { + use super::*; + async fn run_async_cli_shows_revision_sent() -> Result<()> { + let (mut r51, mut r52, mut r53, mut r55, mut r56) = ( + 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), + ); + + r51.events.push(generate_test_key_1_relay_list_event()); + r51.events.push(generate_test_key_1_metadata_event("fred")); + r51.events.push(generate_repo_ref_event()); + + r55.events.push(generate_repo_ref_event()); + r55.events.push(generate_test_key_1_metadata_event("fred")); + r55.events.push(generate_test_key_1_relay_list_event()); + + let cli_tester_handle = std::thread::spawn(move || -> Result<()> { + cli_tester_create_proposals()?; + + let test_repo = GitTestRepo::default(); + test_repo.populate()?; + + // simulate rebase + std::fs::write(test_repo.dir.join("amazing.md"), "some content")?; + test_repo.stage_and_commit("commit for rebasing on top of")?; + create_and_populate_branch(&test_repo, FEATURE_BRANCH_NAME_1, "a", false)?; + let mut p = CliTester::new_from_dir( + &test_repo.dir, + [ + "--nsec", + TEST_KEY_1_NSEC, + "--password", + TEST_PASSWORD, + "--disable-cli-spinners", + "push", + "--force", + "--no-cover-letter", + ], + ); + p.expect("finding proposal root event...\r\n")?; + p.expect("found proposal root event. finding commits...\r\n")?; + p.expect("preparing to force push proposal revision...\r\n")?; + + // standard output from `ngit send` + p.expect("creating patch for 2 commits from 'head' that can be merged into 'main'\r\n")?; + p.expect("searching for profile and relay updates...\r\n")?; + p.expect("\r")?; + p.expect("logged in as fred\r\n")?; + p.expect("posting 2 patches without a covering letter...\r\n")?; + + relay::expect_send_with_progress( + &mut p, + vec![ + (" [my-relay] [repo-relay] ws://localhost:8055", true, ""), + (" [my-relay] ws://localhost:8053", true, ""), + (" [repo-relay] ws://localhost:8056", true, ""), + (" [default] ws://localhost:8051", true, ""), + (" [default] ws://localhost:8052", true, ""), + ], + 2, + )?; + // end standard `ngit send output` + p.expect_after_whitespace("force pushed proposal revision\r\n")?; + p.expect_end()?; + + for p in [51, 52, 53, 55, 56] { + relay::shutdown_relay(8000 + p)?; + } + Ok(()) + }); + + // launch relay + let _ = join!( + r51.listen_until_close(), + r52.listen_until_close(), + r53.listen_until_close(), + r55.listen_until_close(), + r56.listen_until_close(), + ); + cli_tester_handle.join().unwrap()?; + Ok(()) + } + + #[tokio::test] + #[serial] + async fn cli_shows_revision_sent() -> Result<()> { + let _ = run_async_cli_shows_revision_sent().await; + Ok(()) + } + } + } } } -- cgit v1.2.3