cli: Support revision id in `rad merge`

Revision IDs are globally unique.  Users will be able to use the
Revision ID in place of its patch ID.

Make `rad merge` refuse to merge an older revision unless the `--force`
option is given.
This commit is contained in:
Slack Coder 2023-03-18 10:53:44 -05:00 committed by Alexis Sellier
parent 8404b5925d
commit 0b7f169c8b
No known key found for this signature in database
6 changed files with 121 additions and 36 deletions

View File

@ -54,3 +54,36 @@ a07ef77 R1 (27857ec) -> R2 (f6484e0)
✓ Patch a07ef77 updated to 737dc47d9eba730c5db8a8e33c41c7955f9093de ✓ Patch a07ef77 updated to 737dc47d9eba730c5db8a8e33c41c7955f9093de
``` ```
Great, all fixed up, lets merge the code.
```
$ git checkout master
Your branch is up to date with 'rad/master'.
$ rad merge 737dc47d9eba730c5db8a8e33c41c7955f9093de
Merging a07ef77 R2 (f6484e0) by did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk into master (f2de534) via fast-forward...
Running `git merge --ff-only f6484e0f43e48a8983b9b39bf9bd4cd889f1d520`...
Updating f2de534..f6484e0
Fast-forward
README.md | 0
REQUIREMENTS.md | 0
2 files changed, 0 insertions(+), 0 deletions(-)
create mode 100644 README.md
create mode 100644 REQUIREMENTS.md
✓ Updated master f2de534 -> f6484e0 via fast-forward
✓ Patch state updated, use `rad push` to publish
```
The patch is now merged and closed :).
```
$ rad patch
╭───────────────────────────────────────────────────────────────────────────────────────────────╮
│ Define power requirements a07ef77 R2 f6484e0 (flux-capacitor-power, master) ahead 3, behind 0 │
├───────────────────────────────────────────────────────────────────────────────────────────────┤
│ ● opened by did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk 3 months ago │
│ ↑ updated to a07ef7743a32a2e902672ea3526d1db6ee08108a (3e674d1) 3 months ago │
│ ↑ updated to 4f15eb5c994edd0bb6be29cb4801aa74308cc628 (27857ec) 3 months ago │
│ ✓ merged by did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (you) 3 months ago │
╰───────────────────────────────────────────────────────────────────────────────────────────────╯
```

View File

@ -1,14 +1,12 @@
use std::ffi::OsString; use std::ffi::OsString;
use std::fmt; use std::fmt;
use std::fmt::Write; use std::fmt::Write;
use std::str::FromStr;
use anyhow::{anyhow, Context}; use anyhow::{anyhow, Context};
use crate::git::Rev; use crate::git::Rev;
use crate::terminal as term; use crate::terminal as term;
use crate::terminal::args::{string, Args, Error, Help}; use crate::terminal::args::{string, Args, Error, Help};
use radicle::cob::patch::RevisionIx;
use radicle::cob::patch::{Patch, PatchId, Patches}; use radicle::cob::patch::{Patch, PatchId, Patches};
use radicle::git; use radicle::git;
use radicle::prelude::*; use radicle::prelude::*;
@ -21,14 +19,14 @@ pub const HELP: Help = Help {
usage: r#" usage: r#"
Usage Usage
rad merge [<id>] [<option>...] rad merge [<revision-id>] [<option>...]
To specify a patch to merge, use the fully qualified patch id. To specify a patch revision to merge, use the fully qualified revision id.
Options Options
-f, --force Force merging an older patch revision
-i, --interactive Ask for confirmations -i, --interactive Ask for confirmations
-r, --revision <number> Revision number to merge, defaults to the latest
--help Print help --help Print help
"#, "#,
}; };
@ -71,11 +69,12 @@ pub enum State {
Open, Open,
Merged, Merged,
} }
#[derive(Debug)] #[derive(Debug)]
pub struct Options { pub struct Options {
pub id: Rev, pub revision_id: Rev,
pub force: bool,
pub interactive: bool, pub interactive: bool,
pub revision: Option<RevisionIx>,
} }
impl Args for Options { impl Args for Options {
@ -83,8 +82,8 @@ impl Args for Options {
use lexopt::prelude::*; use lexopt::prelude::*;
let mut parser = lexopt::Parser::from_args(args); let mut parser = lexopt::Parser::from_args(args);
let mut id: Option<Rev> = None; let mut force = false;
let mut revision: Option<RevisionIx> = None; let mut revision_id = None;
let mut interactive = false; let mut interactive = false;
while let Some(arg) = parser.next()? { while let Some(arg) = parser.next()? {
@ -92,30 +91,28 @@ impl Args for Options {
Long("help") => { Long("help") => {
return Err(Error::Help.into()); return Err(Error::Help.into());
} }
Long("force") | Short('f') => {
force = true;
}
Long("interactive") | Short('i') => { Long("interactive") | Short('i') => {
interactive = true; interactive = true;
} }
Long("revision") | Short('r') => {
let value = parser.value()?;
let id =
RevisionIx::from_str(value.to_str().unwrap_or_default()).map_err(|_| {
anyhow!("invalid revision number `{}`", value.to_string_lossy())
})?;
revision = Some(id);
}
Value(val) => { Value(val) => {
let val = string(&val); let val = string(&val);
id = Some(Rev::from(val)); revision_id = Some(Rev::from(val));
} }
_ => return Err(anyhow::anyhow!(arg.unexpected())), _ => return Err(anyhow::anyhow!(arg.unexpected())),
} }
} }
let revision_id =
revision_id.ok_or_else(|| anyhow!("a revision id to merge must be provided"))?;
Ok(( Ok((
Options { Options {
id: id.ok_or_else(|| anyhow!("a patch id to merge must be provided"))?, revision_id,
force,
interactive, interactive,
revision,
}, },
vec![], vec![],
)) ))
@ -144,10 +141,27 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
// //
// Get patch information // Get patch information
// //
let patch_id = options.id.resolve(&repository.backend)?; let revision_id = options.revision_id.resolve(&repository.backend)?;
let (patch_id, patch, revision) = patches.find_by_revision(&revision_id)?.ok_or(anyhow!(
"no open patch with revision `{}` could be found",
&revision_id
))?;
if !patch.is_open() {
anyhow::bail!(
"revision's patch must be open for merging, but it is `{}`",
patch.state()
);
}
let (last_revision_id, _) = patch
.latest()
.ok_or(anyhow!("patch must have atleast one unredacted revision"))?;
if !options.force && revision_id != *last_revision_id {
anyhow::bail!("refusing to merge old patch revision");
}
let mut patch = patches let mut patch = patches
.get_mut(&patch_id) .get_mut(&patch_id)
.map_err(|e| anyhow!("couldn't find patch {} locally: {e}", &options.id.clone()))?; .map_err(|e| anyhow!("couldn't find patch {} locally: {e}", &id))?;
let head = repo.head()?; let head = repo.head()?;
let branch = head let branch = head
@ -156,11 +170,6 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
let head_oid = head let head_oid = head
.target() .target()
.ok_or_else(|| anyhow!("cannot merge into detatched head; aborting"))?; .ok_or_else(|| anyhow!("cannot merge into detatched head; aborting"))?;
let revision_ix = options.revision.unwrap_or_else(|| patch.version());
let (revision_id, revision) = patch
.revisions()
.nth(revision_ix)
.ok_or_else(|| anyhow!("revision R{} does not exist", revision_ix))?;
// //
// Analyze merge // Analyze merge
@ -221,6 +230,11 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
.to_string(), .to_string(),
}; };
// SAFETY: The patch has already been fetched by its revision_id.
let revision_ix = patch
.revisions()
.position(|(id_, _)| id_ == &revision_id)
.unwrap();
term::info!( term::info!(
"{} {} {} {} by {} into {} {} via {}...", "{} {} {} {} by {} into {} {} via {}...",
term::format::bold("Merging"), term::format::bold("Merging"),
@ -261,7 +275,7 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
// Update patch COB // Update patch COB
// //
// TODO: Don't allow merging the same revision twice? // TODO: Don't allow merging the same revision twice?
patch.merge(*revision_id, head_oid.into(), &signer)?; patch.merge(revision_id, head_oid.into(), &signer)?;
term::success!( term::success!(
"Patch state updated, use {} to publish", "Patch state updated, use {} to publish",

View File

@ -11,7 +11,6 @@ use std::str::FromStr;
use anyhow::anyhow; use anyhow::anyhow;
use anyhow::Context as _; use anyhow::Context as _;
use radicle::cob::ObjectId;
use radicle::crypto::ssh; use radicle::crypto::ssh;
use radicle::git; use radicle::git;
use radicle::git::raw as git2; use radicle::git::raw as git2;
@ -39,10 +38,13 @@ impl Rev {
&self.0 &self.0
} }
/// Resolve the revision to an [`ObjectId`]. /// Resolve the revision to an [`From<git2::Oid>`].
pub fn resolve(&self, repo: &git2::Repository) -> Result<ObjectId, git2::Error> { pub fn resolve<T>(&self, repo: &git2::Repository) -> Result<T, git2::Error>
where
T: From<git2::Oid>,
{
let object = repo.revparse_single(self.as_str())?; let object = repo.revparse_single(self.as_str())?;
Ok(ObjectId::from(object.id())) Ok(object.id().into())
} }
} }

View File

@ -10,6 +10,7 @@ use nonempty::NonEmpty;
use radicle_crypto::PublicKey; use radicle_crypto::PublicKey;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use crate::object;
use crate::pruning_fold; use crate::pruning_fold;
/// Entry contents. /// Entry contents.
@ -60,6 +61,12 @@ impl From<EntryId> for Oid {
} }
} }
impl From<&EntryId> for object::ObjectId {
fn from(EntryId(id): &EntryId) -> Self {
id.into()
}
}
/// One entry in the dependency graph for a change /// One entry in the dependency graph for a change
#[derive(Clone, Debug, PartialEq, Eq, Hash)] #[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub struct Entry { pub struct Entry {

View File

@ -124,7 +124,7 @@ pub fn indented(msg: impl fmt::Display) {
} }
pub fn subcommand(msg: impl fmt::Display) { pub fn subcommand(msg: impl fmt::Display) {
println!("{} {}", style("$").dim(), style(msg).dim()); println!("{}", style(format!("Running `{msg}`...")).dim());
} }
pub fn warning(warning: &str) { pub fn warning(warning: &str) {

View File

@ -216,6 +216,13 @@ impl Patch {
.author .author
} }
/// Get the `Revision` by its `RevisionId`.
///
/// None is returned if the `Revision` has been redacted (deleted).
pub fn revision(&self, id: &RevisionId) -> Option<&Revision> {
self.revisions.get(id).and_then(Redactable::get)
}
/// List of patch revisions. The initial changeset is part of the /// List of patch revisions. The initial changeset is part of the
/// first revision. /// first revision.
pub fn revisions(&self) -> impl DoubleEndedIterator<Item = (&RevisionId, &Revision)> { pub fn revisions(&self) -> impl DoubleEndedIterator<Item = (&RevisionId, &Revision)> {
@ -1010,6 +1017,25 @@ impl<'a> Patches<'a> {
Ok(state_groups) Ok(state_groups)
} }
/// Find the `Patch` containing the given `Revision`.
pub fn find_by_revision(
&self,
id: &RevisionId,
) -> Result<Option<(PatchId, Patch, Revision)>, Error> {
// Revision may be the patch's first, making it have the same ID.
let p_id = ObjectId::from(id);
if let Some(p) = self.get(&p_id)? {
return Ok(p.revision(id).map(|r| (p_id, p.clone(), r.clone())));
}
let result = self
.all()?
.into_iter()
.filter_map(|result| result.ok())
.find_map(|(p_id, p, _)| p.revision(id).map(|r| (p_id, p.clone(), r.clone())));
Ok(result)
}
/// Get a patch. /// Get a patch.
pub fn get(&self, id: &ObjectId) -> Result<Option<Patch>, store::Error> { pub fn get(&self, id: &ObjectId) -> Result<Option<Patch>, store::Error> {
self.raw.get(id).map(|r| r.map(|(p, _)| p)) self.raw.get(id).map(|r| r.map(|(p, _)| p))
@ -1253,8 +1279,8 @@ mod test {
assert_eq!(patch.clock.get(), 1); assert_eq!(patch.clock.get(), 1);
let id = patch.id; let patch_id = patch.id;
let patch = patches.get(&id).unwrap().unwrap(); let patch = patches.get(&patch_id).unwrap().unwrap();
assert_eq!(patch.title(), "My first patch"); assert_eq!(patch.title(), "My first patch");
assert_eq!(patch.description(), "Blah blah blah."); assert_eq!(patch.description(), "Blah blah blah.");
@ -1263,13 +1289,16 @@ mod test {
assert_eq!(patch.target(), target); assert_eq!(patch.target(), target);
assert_eq!(patch.version(), 0); assert_eq!(patch.version(), 0);
let (_, revision) = patch.latest().unwrap(); let (rev_id, revision) = patch.latest().unwrap();
assert_eq!(revision.author.id(), &author); assert_eq!(revision.author.id(), &author);
assert_eq!(revision.description(), ""); assert_eq!(revision.description(), "");
assert_eq!(revision.discussion.len(), 0); assert_eq!(revision.discussion.len(), 0);
assert_eq!(revision.oid, oid); assert_eq!(revision.oid, oid);
assert_eq!(revision.base, base); assert_eq!(revision.base, base);
let (id, _, _) = patches.find_by_revision(rev_id).unwrap().unwrap();
assert_eq!(id, patch_id);
} }
#[test] #[test]