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.
This commit is contained in:
Alexis Sellier 2023-06-21 13:53:00 +02:00
parent b25c2e452b
commit e325fa8621
No known key found for this signature in database
6 changed files with 115 additions and 183 deletions

View File

@ -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) [ ... ] │
╰──────────────────────────────────────────────────────────────────────────────╯
```

View File

@ -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:

View File

@ -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 [<string>] 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<Rev>,
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::<PatchId>(&repository.backend)?;

View File

@ -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<git::raw
};
Ok(branch)
}
/// Push branch to the local storage.
///
/// The branch must be in storage for others to merge the `Patch`.
pub fn push_to_storage(
working: &git::raw::Repository,
storage: &Repository,
head_branch: &git::raw::Branch,
options: &Options,
) -> anyhow::Result<git::RefString> {
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<Vec<(PatchId, Patch, Clock)>> {
// 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)
}

View File

@ -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<patch::PatchId> {
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::PatchId>,
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(())
}

View File

@ -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();