From e325fa8621210b22a4937f52ae39b141d45eed1d Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Wed, 21 Jun 2023 13:53:00 +0200 Subject: [PATCH] cli: Turn `patch update` into a plumbing command Remove most of the user-friendly functionality and simplify this command so that it is only used for scripting. `git push rad` is now the proper way of updating patches. --- radicle-cli/examples/rad-patch-update.md | 72 ++++++++++++++++++++ radicle-cli/examples/rad-patch.md | 10 +-- radicle-cli/src/commands/patch.rs | 38 ++++------- radicle-cli/src/commands/patch/common.rs | 75 +-------------------- radicle-cli/src/commands/patch/update.rs | 83 ++---------------------- radicle-cli/tests/commands.rs | 20 ++++++ 6 files changed, 115 insertions(+), 183 deletions(-) create mode 100644 radicle-cli/examples/rad-patch-update.md diff --git a/radicle-cli/examples/rad-patch-update.md b/radicle-cli/examples/rad-patch-update.md new file mode 100644 index 00000000..370ea7f4 --- /dev/null +++ b/radicle-cli/examples/rad-patch-update.md @@ -0,0 +1,72 @@ +Let's explore the `rad patch update` plumbing command. First we create a patch: + +``` (stderr) +$ git checkout -q -b feature/1 +$ git commit -q -m "Not a real change" --allow-empty +``` +``` (stderr) +$ git push rad HEAD:refs/patches +✓ Patch 51e0d0bc168ccdc541b7b1aeab2eb9e048c2fcdd opened +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + * [new reference] HEAD -> refs/patches +``` + +``` +$ rad patch show 51e0d0bc168ccdc541b7b1aeab2eb9e048c2fcdd +╭────────────────────────────────────────────────────────────────────╮ +│ Title Not a real change │ +│ Patch 51e0d0bc168ccdc541b7b1aeab2eb9e048c2fcdd │ +│ Author did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi │ +│ Head 51b2f0f77b9849bfaa3e9d3ff68ee2f57771d20c │ +│ Branches feature/1 │ +│ Commits ahead 1, behind 0 │ +│ Status open │ +├────────────────────────────────────────────────────────────────────┤ +│ 51b2f0f Not a real change │ +├────────────────────────────────────────────────────────────────────┤ +│ ● opened by (you) (z6MknSL…StBU8Vi) [ ... ] │ +╰────────────────────────────────────────────────────────────────────╯ +``` + +We can make some changes to the repository: + +``` +$ git mv README README.md +$ git commit -q -m "Rename readme file" +``` + +Let's push the changes, but not to the magic ref, that way the push doesn't +update our patch: + +``` +$ git push rad HEAD:refs/heads/feature/1 +``` + +Now, instead of using `git push` to update the patch, as we normally would, +we run: + +``` +$ rad patch update 51e0d0bc168ccdc541b7b1aeab2eb9e048c2fcdd -m "Updated patch" +c10012c2cb9c0c9bfeba7ef28cae10e4b8db3469 +``` + +The command outputs the new Revision ID, which we can now see here: + +``` +$ rad patch show 51e0d0bc168ccdc541b7b1aeab2eb9e048c2fcdd +╭──────────────────────────────────────────────────────────────────────────────╮ +│ Title Not a real change │ +│ Patch 51e0d0bc168ccdc541b7b1aeab2eb9e048c2fcdd │ +│ Author did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi │ +│ Head 4d272148458a17620541555b1f0905c01658aa9f │ +│ Branches feature/1 │ +│ Commits ahead 2, behind 0 │ +│ Status open │ +├──────────────────────────────────────────────────────────────────────────────┤ +│ 4d27214 Rename readme file │ +│ 51b2f0f Not a real change │ +├──────────────────────────────────────────────────────────────────────────────┤ +│ ● opened by (you) (z6MknSL…StBU8Vi) [ ... ] │ +│ ↑ updated to c10012c2cb9c0c9bfeba7ef28cae10e4b8db3469 (4d27214) [ ... ] │ +╰──────────────────────────────────────────────────────────────────────────────╯ +``` diff --git a/radicle-cli/examples/rad-patch.md b/radicle-cli/examples/rad-patch.md index 9d7a41d1..ccc3e551 100644 --- a/radicle-cli/examples/rad-patch.md +++ b/radicle-cli/examples/rad-patch.md @@ -87,10 +87,12 @@ $ git commit --message "Add README, just for the fun" [flux-capacitor-power 27857ec] Add README, just for the fun 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 README.md -$ rad patch update --message "Add README, just for the fun" 077e4bbe9a6e5546f400ef5951768c37a76f13a4 -Updating 3e674d1 -> 27857ec -1 commit(s) ahead, 0 commit(s) behind -✓ Patch updated to revision 5cdcd2e14411e2bfec7b11bcf4667e2e0fc4d417 +``` +``` (stderr) +$ git push rad -o patch.message="Add README, just for the fun" +✓ Patch 077e4bb updated to 5cdcd2e14411e2bfec7b11bcf4667e2e0fc4d417 +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + 3e674d1..27857ec flux-capacitor-power -> patches/077e4bbe9a6e5546f400ef5951768c37a76f13a4 ``` And let's leave a quick comment for our team: diff --git a/radicle-cli/src/commands/patch.rs b/radicle-cli/src/commands/patch.rs index 1cad6b37..fae468ec 100644 --- a/radicle-cli/src/commands/patch.rs +++ b/radicle-cli/src/commands/patch.rs @@ -63,9 +63,6 @@ Edit options Update options - -q, --quiet Supress most output, only print the revision id - --[no-]announce Announce patch to network (default: false) - --[no-]push Push patch head to storage (default: true) -m, --message [] Provide a comment message to the patch or revision (default: prompt) --no-message Leave the patch or revision comment message blank @@ -130,9 +127,8 @@ pub enum Operation { verbose: bool, }, Update { - patch_id: Option, + patch_id: Rev, message: Message, - quiet: bool, }, Archive { patch_id: Rev, @@ -184,7 +180,6 @@ impl Args for Options { let mut filter = Filter::default(); let mut diff = false; let mut undo = false; - let mut quiet = false; while let Some(arg) = parser.next()? { match arg { @@ -218,11 +213,6 @@ impl Args for Options { push = false; } - // Update options. - Long("quiet") | Short('q') if op == Some(OperationName::Update) => { - quiet = true; - } - // Show options. Long("patch") | Short('p') if op == Some(OperationName::Show) => { diff = true; @@ -233,6 +223,13 @@ impl Args for Options { undo = true; } + // Update options + Long("revision") if op == Some(OperationName::Update) => { + let val = parser.value()?; + let val = string(&val); + revision_id = Some(Rev::from(val)); + } + // List options. Long("all") => { filter = Filter::all(); @@ -305,9 +302,8 @@ impl Args for Options { patch_id: patch_id.ok_or_else(|| anyhow!("a patch must be provided"))?, }, OperationName::Update => Operation::Update { - patch_id, + patch_id: patch_id.ok_or_else(|| anyhow!("a patch must be provided"))?, message, - quiet, }, OperationName::Archive => Operation::Archive { patch_id: patch_id.ok_or_else(|| anyhow!("a patch id must be provided"))?, @@ -369,21 +365,9 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { Operation::Update { ref patch_id, ref message, - quiet, } => { - let patch_id = patch_id - .as_ref() - .map(|id| id.resolve(&repository.backend)) - .transpose()?; - update::run( - patch_id, - message.clone(), - quiet, - &options, - &profile, - &repository, - &workdir, - )?; + let patch_id = patch_id.resolve(&repository.backend)?; + update::run(patch_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/common.rs b/radicle-cli/src/commands/patch/common.rs index 9a7738c4..f64ef80c 100644 --- a/radicle-cli/src/commands/patch/common.rs +++ b/radicle-cli/src/commands/patch/common.rs @@ -1,23 +1,11 @@ -use anyhow::{anyhow, Context}; +use anyhow::anyhow; -use radicle::cob::patch::{Clock, Patch, PatchId, Patches}; use radicle::git; use radicle::git::raw::Oid; use radicle::prelude::*; use radicle::storage::git::Repository; use crate::terminal as term; -use crate::terminal::args::Error; - -use super::Options; - -/// Give the name of the branch or an appropriate error. -#[inline] -pub fn branch_name<'a>(branch: &'a git::raw::Branch) -> anyhow::Result<&'a str> { - branch - .name()? - .ok_or(anyhow!("head branch must be valid UTF-8")) -} /// Give the oid of the branch or an appropriate error. #[inline] @@ -114,64 +102,3 @@ pub fn try_branch(reference: git::raw::Reference<'_>) -> anyhow::Result anyhow::Result { - let head_oid = branch_oid(head_branch)?; - let branch = branch_name(head_branch)?.try_into()?; - let branch = radicle::git::refs::workdir::branch(branch); - - if storage.commit(head_oid).is_err() { - if !options.push { - term::blank(); - - return Err(Error::WithHint { - err: anyhow!("Current branch head was not found in storage"), - hint: "hint: run `git push rad` and try again", - } - .into()); - } - - let (mut remote, _) = radicle::rad::remote(working)?; - - remote - .push::<&str>(&[&format!("+{branch}:{branch}")], None) - .context("failed to push to storage")?; - } - Ok(branch) -} - -/// Find patches with a merge base equal to the one provided. -pub fn find_unmerged_with_base( - patch_head: Oid, - target_head: Oid, - merge_base: Oid, - patches: &Patches, - storage: &Repository, - whoami: &Did, -) -> anyhow::Result> { - // My patches. - let proposed: Vec<_> = patches.proposed_by(whoami)?.collect(); - let mut matches = Vec::new(); - - for (id, patch, clock) in proposed { - if patch.merges().count() != 0 { - continue; - } - if **patch.head() == patch_head { - continue; - } - // Merge-base between the two patches. - if storage.backend.merge_base(**patch.head(), target_head)? == merge_base { - matches.push((id, patch, clock)); - } - } - Ok(matches) -} diff --git a/radicle-cli/src/commands/patch/update.rs b/radicle-cli/src/commands/patch/update.rs index b0eea912..1b35416b 100644 --- a/radicle-cli/src/commands/patch/update.rs +++ b/radicle-cli/src/commands/patch/update.rs @@ -4,60 +4,12 @@ use radicle::prelude::*; use radicle::storage::git::Repository; use super::common::*; -use super::Options; use crate::terminal as term; -fn select_patch( - patches: &patch::Patches, - storage: &Repository, - head_branch: &git::raw::Branch, - target_oid: git::Oid, - whoami: &Did, -) -> anyhow::Result { - let head_oid = branch_oid(head_branch)?; - let base_oid = storage.backend.merge_base(*target_oid, *head_oid)?; - - let mut result = - find_unmerged_with_base(*head_oid, *target_oid, base_oid, patches, storage, whoami)?; - - let Some((id, _, _)) = result.pop() else { - anyhow::bail!("No patches found to update, please specify a patch id"); - }; - - if !result.is_empty() { - anyhow::bail!("More than one patch available to update, please specify a patch id"); - } - term::blank(); - - Ok(id) -} - -fn show_update_commit_info( - storage: &Repository, - current_revision: &patch::Revision, - head_branch: &git::raw::Branch, -) -> anyhow::Result<()> { - let head_oid = branch_oid(head_branch)?; - - term::info!( - "Updating {} -> {}", - term::format::secondary(term::format::oid(current_revision.head())), - term::format::secondary(term::format::oid(head_oid)), - ); - - // Difference between the two revisions. - let head_oid = branch_oid(head_branch)?; - term::patch::print_commits_ahead_behind(&storage.backend, *head_oid, *current_revision.head())?; - - Ok(()) -} - /// Run patch update. pub fn run( - patch_id: Option, + patch_id: patch::PatchId, message: term::patch::Message, - quiet: bool, - options: &Options, profile: &Profile, storage: &Repository, workdir: &git::raw::Repository, @@ -65,49 +17,24 @@ pub fn run( // `HEAD`; This is what we are proposing as a patch. let head_branch = try_branch(workdir.head()?)?; - push_to_storage(workdir, storage, &head_branch, options)?; - let (_, target_oid) = get_merge_target(storage, &head_branch)?; let mut patches = patch::Patches::open(storage)?; - - let patch_id = match patch_id { - Some(patch_id) => patch_id, - None => select_patch(&patches, storage, &head_branch, target_oid, &profile.did())?, - }; let Ok(mut patch) = patches.get_mut(&patch_id) else { anyhow::bail!("Patch `{patch_id}` not found"); }; - let (_, current_revision) = patch.latest(); - if current_revision.head() == branch_oid(&head_branch)? { - if !quiet { - term::info!("Nothing to do, patch is already up to date."); - } + let (_, revision) = patch.latest(); + if revision.head() == branch_oid(&head_branch)? { return Ok(()); } - if !quiet { - show_update_commit_info(storage, current_revision, &head_branch)?; - } - 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, current_revision, &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)?; - if quiet { - term::print(revision); - } else { - term::success!( - "Patch updated to revision {}", - term::format::tertiary(revision), - ); - } - - if options.announce { - // TODO - } + term::print(revision); Ok(()) } diff --git a/radicle-cli/tests/commands.rs b/radicle-cli/tests/commands.rs index efe856da..4fd84da5 100644 --- a/radicle-cli/tests/commands.rs +++ b/radicle-cli/tests/commands.rs @@ -276,6 +276,26 @@ fn rad_patch() { test("examples/rad-patch.md", working.path(), Some(home), []).unwrap(); } +#[test] +fn rad_patch_update() { + let mut environment = Environment::new(); + let profile = environment.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-update.md", + working.path(), + Some(home), + [], + ) + .unwrap(); +} + #[test] fn rad_patch_ahead_behind() { let mut environment = Environment::new();