From 48aa725beafa327277f0772347f63c4d1cb6b6b3 Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Mon, 11 Dec 2023 14:31:21 +0000 Subject: [PATCH] cli: Add `--base` option to `rad patch update` Allow the CLI user to update a patch's base commit through the `rad patch update` command. There was check to see if the head of the branch was the same as the head of the revision, which would result in the update being skipped. This was adapted to check that the head *and* the base were the same. A test was added in the examples to showcase this use. Signed-off-by: Fintan Halpenny X-Clacks-Overhead: GNU Terry Pratchett --- radicle-cli/examples/rad-patch-change-base.md | 93 +++++++++++++++++++ radicle-cli/src/commands/patch.rs | 22 ++++- radicle-cli/src/commands/patch/update.rs | 12 ++- radicle-cli/tests/commands.rs | 21 +++++ 4 files changed, 142 insertions(+), 6 deletions(-) create mode 100644 radicle-cli/examples/rad-patch-change-base.md diff --git a/radicle-cli/examples/rad-patch-change-base.md b/radicle-cli/examples/rad-patch-change-base.md new file mode 100644 index 00000000..b2e68c1c --- /dev/null +++ b/radicle-cli/examples/rad-patch-change-base.md @@ -0,0 +1,93 @@ +Sometimes we are a bit forgetful and miss a detail when creating a +patch. In this case we'll stack two patches by creating one after the +other. + +First we add a `REQUIREMENTS` file: + +``` +$ git checkout -b flux-capacitor-power +$ touch REQUIREMENTS +$ git add REQUIREMENTS +$ git commit -v -m "Define power requirements" +[flux-capacitor-power 3e674d1] Define power requirements + 1 file changed, 0 insertions(+), 0 deletions(-) + create mode 100644 REQUIREMENTS +``` +``` (stderr) +$ git push rad flux-capacitor-power +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + * [new branch] flux-capacitor-power -> flux-capacitor-power +$ git push rad -o patch.message="Define power requirements" -o patch.message="See details." HEAD:refs/patches +✓ Patch 6ff4f09c1b5a81347981f59b02ef43a31a07cdae opened +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + * [new reference] HEAD -> refs/patches +``` + +And then a `README` file: +``` +$ git checkout -b add-readme +$ touch README.md +$ git add README.md +$ git commit --message "Add README, just for the fun" +[add-readme 27857ec] Add README, just for the fun + 1 file changed, 0 insertions(+), 0 deletions(-) + create mode 100644 README.md +``` +``` (stderr) +$ git push rad -o patch.message="Add README, just for the fun" HEAD:refs/patches +✓ Patch 11188b84720e8ab2625be2be64987311fb046573 opened +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + * [new reference] HEAD -> refs/patches +``` + +Our second patch looks like the following: + +``` +$ rad patch show 11188b8 -v +╭────────────────────────────────────────────────────╮ +│ Title Add README, just for the fun │ +│ Patch 11188b84720e8ab2625be2be64987311fb046573 │ +│ Author z6MknSL…StBU8Vi (you) │ +│ Head 27857ec9eb04c69cacab516e8bf4b5fd36090f66 │ +│ Base f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354 │ +│ Branches add-readme │ +│ Commits ahead 2, behind 0 │ +│ Status open │ +├────────────────────────────────────────────────────┤ +│ 27857ec Add README, just for the fun │ +│ 3e674d1 Define power requirements │ +├────────────────────────────────────────────────────┤ +│ ● opened by z6MknSL…StBU8Vi (you) now │ +╰────────────────────────────────────────────────────╯ +``` + +But wait, we meant to stack them and so we don't want to see the +commit `3e674d1` as part of this patch, so we create a new revision +with a new `base`: + +``` +$ rad patch update 11188b8 -b 3e674d1 -m "Whoops, forgot to set the base" --no-announce +ea90a38b4df0b323149a06b0472ea1cf9a754dca +``` + +Now, if we show the patch we can see the patch's base has changed and +we have a single commit: + +``` +$ rad patch show 11188b8 -v +╭─────────────────────────────────────────────────────────────────────╮ +│ Title Add README, just for the fun │ +│ Patch 11188b84720e8ab2625be2be64987311fb046573 │ +│ Author z6MknSL…StBU8Vi (you) │ +│ Head 27857ec9eb04c69cacab516e8bf4b5fd36090f66 │ +│ Base 3e674d1a1df90807e934f9ae5da2591dd6848a33 │ +│ Branches add-readme │ +│ Commits ahead 2, behind 0 │ +│ Status open │ +├─────────────────────────────────────────────────────────────────────┤ +│ 27857ec Add README, just for the fun │ +├─────────────────────────────────────────────────────────────────────┤ +│ ● opened by z6MknSL…StBU8Vi (you) now │ +│ ↑ updated to ea90a38b4df0b323149a06b0472ea1cf9a754dca (27857ec) now │ +╰─────────────────────────────────────────────────────────────────────╯ +``` diff --git a/radicle-cli/src/commands/patch.rs b/radicle-cli/src/commands/patch.rs index 6a6214f7..65d8d888 100644 --- a/radicle-cli/src/commands/patch.rs +++ b/radicle-cli/src/commands/patch.rs @@ -109,6 +109,7 @@ Label options Update options + -b, --base Provide a Git revision as the base commit -m, --message [] Provide a comment message to the patch or revision (default: prompt) --no-message Leave the patch or revision comment message blank @@ -201,6 +202,7 @@ pub enum Operation { }, Update { patch_id: Rev, + base_id: Option, message: Message, }, Archive { @@ -304,6 +306,7 @@ impl Args for Options { let mut assign_opts = AssignOptions::default(); let mut label_opts = LabelOptions::default(); let mut review_op = review::Operation::default(); + let mut base_id = None; while let Some(arg) = parser.next()? { match arg { @@ -342,11 +345,11 @@ impl Args for Options { } // Update options. - Long("revision") if op == Some(OperationName::Update) => { + Short('b') | Long("base") if op == Some(OperationName::Update) => { let val = parser.value()?; let rev = term::args::rev(&val)?; - revision_id = Some(rev); + base_id = Some(rev); } // Comment options. @@ -555,6 +558,7 @@ impl Args for Options { }, OperationName::Update => Operation::Update { patch_id: patch_id.ok_or_else(|| anyhow!("a patch must be provided"))?, + base_id, message, }, OperationName::Archive => Operation::Archive { @@ -650,10 +654,22 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { } Operation::Update { ref patch_id, + ref base_id, ref message, } => { let patch_id = patch_id.resolve(&repository.backend)?; - update::run(patch_id, message.clone(), &profile, &repository, &workdir)?; + let base_id = base_id + .as_ref() + .map(|base| base.resolve(&repository.backend)) + .transpose()?; + update::run( + patch_id, + base_id, + message.clone(), + &profile, + &repository, + &workdir, + )?; } Operation::Archive { ref patch_id } => { let patch_id = patch_id.resolve::(&repository.backend)?; diff --git a/radicle-cli/src/commands/patch/update.rs b/radicle-cli/src/commands/patch/update.rs index 1b35416b..aa70c733 100644 --- a/radicle-cli/src/commands/patch/update.rs +++ b/radicle-cli/src/commands/patch/update.rs @@ -9,6 +9,7 @@ use crate::terminal as term; /// Run patch update. pub fn run( patch_id: patch::PatchId, + base_id: Option, message: term::patch::Message, profile: &Profile, storage: &Repository, @@ -23,13 +24,18 @@ pub fn run( anyhow::bail!("Patch `{patch_id}` not found"); }; + let head_oid = branch_oid(&head_branch)?; + let base_oid = match base_id { + Some(oid) => oid, + None => storage.backend.merge_base(*target_oid, *head_oid)?, + }; + let (_, revision) = patch.latest(); - if revision.head() == branch_oid(&head_branch)? { + // N.b. we don't update if both the head and base are the same + if revision.head() == head_oid && **revision.base() == base_oid { return Ok(()); } - let head_oid = branch_oid(&head_branch)?; - let base_oid = storage.backend.merge_base(*target_oid, *head_oid)?; let message = term::patch::get_update_message(message, workdir, revision, &head_oid)?; let signer = term::signer(profile)?; let revision = patch.update(message, base_oid, *head_oid, &signer)?; diff --git a/radicle-cli/tests/commands.rs b/radicle-cli/tests/commands.rs index 93f86a0a..98c34bd3 100644 --- a/radicle-cli/tests/commands.rs +++ b/radicle-cli/tests/commands.rs @@ -605,6 +605,27 @@ fn rad_patch_ahead_behind() { .unwrap(); } +#[test] +fn rad_patch_change_base() { + logger::init(log::Level::Debug); + let mut environment = Environment::new(); + let profile = environment.profile(config::profile("alice")); + let working = tempfile::tempdir().unwrap(); + let home = &profile.home; + + // Setup a test repository. + fixtures::repository(working.path()); + + test("examples/rad-init.md", working.path(), Some(home), []).unwrap(); + test( + "examples/rad-patch-change-base.md", + working.path(), + Some(home), + [], + ) + .unwrap(); +} + #[test] fn rad_patch_draft() { let mut environment = Environment::new();