diff --git a/.gitignore b/.gitignore index ea8c4bf7..66b36021 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ /target +/radicle-cli/target diff --git a/radicle-cli/examples/rad-patch-ahead-behind.md b/radicle-cli/examples/rad-patch-ahead-behind.md new file mode 100644 index 00000000..8eb496f0 --- /dev/null +++ b/radicle-cli/examples/rad-patch-ahead-behind.md @@ -0,0 +1,118 @@ +In this example, we explore how the `ahead/behind` indicator works, and what is +shown as diffs in the case of divergent branches. + +First we add the `CONTRIBUTORS` file to `master`, which contains one entry: +``` +$ git checkout -q master +$ git add CONTRIBUTORS +$ git commit -a -q -m "Add contributors" +$ git push rad master +$ cat CONTRIBUTORS +Alice Jones +``` + +Then we create a feature branch which adds another entry: +``` +$ git checkout -q -b feature/1 +$ sed -i '$a Alan K' CONTRIBUTORS +$ git commit -a -q -m "Add Alan" +``` + +We go back to master, and add a different second entry, essentially forking +the history: +``` +$ git checkout -q master +$ sed -i '$a Jason Bourne' CONTRIBUTORS +$ git commit -a -q -m "Add Jason" +$ git push rad master +$ git log --graph --decorate --abbrev-commit --pretty=oneline --all +* 5c88a79 (feature/1) Add Alan +| * e101a99 (HEAD -> master, rad/master) Add Jason +|/ [..] +* f64fb2c Add contributors +* f2de534 Second commit +* 08c788d Initial commit +``` + +Then we create a patch from `feature/1`: +``` (stderr) +$ git push rad feature/1:refs/patches +✓ Patch 866f59c001cd4d78a151f444b34265566c83c264 opened +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + * [new reference] feature/1 -> refs/patches +``` + +When listing, we see that it has one addition: +``` +$ rad patch list +╭─────────────────────────────────────────────────────────────────────────────╮ +│ ● ID Title Author Head + - Updated │ +├─────────────────────────────────────────────────────────────────────────────┤ +│ ● 866f59c Add Alan z6MknSL…StBU8Vi (you) 5c88a79 +1 -0 [ ... ] │ +╰─────────────────────────────────────────────────────────────────────────────╯ +``` + +When showing the patch, we see that it is `ahead 1, behind 1`, since master has +diverged by one commit: +``` +$ rad patch show -p 866f59c +╭────────────────────────────────────────────────────────────────────╮ +│ Title Add Alan │ +│ Patch 866f59c001cd4d78a151f444b34265566c83c264 │ +│ Author did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi │ +│ Head 5c88a79d75f5c2b4cc51ee6f163d2db91ee198d7 │ +│ Branches feature/1 │ +│ Commits ahead 1, behind 1 │ +│ Status open │ +├────────────────────────────────────────────────────────────────────┤ +│ 5c88a79 Add Alan │ +├────────────────────────────────────────────────────────────────────┤ +│ ● opened by (you) (z6MknSL…StBU8Vi) [ ... ] │ +╰────────────────────────────────────────────────────────────────────╯ + +commit 5c88a79d75f5c2b4cc51ee6f163d2db91ee198d7 +Author: radicle +Date: Thu Dec 15 17:28:04 2022 +0000 + + Add Alan + +diff --git a/CONTRIBUTORS b/CONTRIBUTORS +index 3f60d25..6829c43 100644 +--- a/CONTRIBUTORS ++++ b/CONTRIBUTORS +@@ -1 +1,2 @@ + Alice Jones ++Alan K + +``` + +Then, we stack another change onto `feature/1`, adding another contributor: +``` (stderr) +$ git checkout -q -b feature/2 feature/1 +$ sed -i '$a Mel Farna' CONTRIBUTORS +$ git commit -a -q -m "Add Mel" +$ git push -o patch.message="Add Mel" rad HEAD:refs/patches +✓ Patch 57cb9b2758518e547de324456ac967fda456c6c1 opened +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + * [new reference] HEAD -> refs/patches +``` + +When we look at the patch, we see that it has both commits, because this new +patch uses the same base as the previous patch: +``` +$ rad patch show 57cb9b2758518e547de324456ac967fda456c6c1 +╭────────────────────────────────────────────────────────────────────╮ +│ Title Add Mel │ +│ Patch 57cb9b2758518e547de324456ac967fda456c6c1 │ +│ Author did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi │ +│ Head 7f63fcbcf23fc39eea784c091ad3d20d7e4bd005 │ +│ Branches feature/2 │ +│ Commits ahead 2, behind 1 │ +│ Status open │ +├────────────────────────────────────────────────────────────────────┤ +│ 7f63fcb Add Mel │ +│ 5c88a79 Add Alan │ +├────────────────────────────────────────────────────────────────────┤ +│ ● opened by (you) (z6MknSL…StBU8Vi) 5 months ago │ +╰────────────────────────────────────────────────────────────────────╯ +``` diff --git a/radicle-cli/src/commands/patch/common.rs b/radicle-cli/src/commands/patch/common.rs index fcd6bcd9..9a7738c4 100644 --- a/radicle-cli/src/commands/patch/common.rs +++ b/radicle-cli/src/commands/patch/common.rs @@ -1,6 +1,6 @@ use anyhow::{anyhow, Context}; -use radicle::cob::patch::{Clock, MergeTarget, Patch, PatchId, Patches}; +use radicle::cob::patch::{Clock, Patch, PatchId, Patches}; use radicle::git; use radicle::git::raw::Oid; use radicle::prelude::*; @@ -52,22 +52,6 @@ pub fn get_merge_target( Ok((get_branch(qualified_ref), (*target_oid).into())) } -/// Return the [`Oid`] of the merge target. -pub fn patch_merge_target_oid(target: MergeTarget, repository: &Repository) -> anyhow::Result { - match target { - MergeTarget::Delegates => { - if let Ok((_, target)) = repository.head() { - Ok(*target) - } else { - anyhow::bail!( - "failed to determine default branch head for project {}", - repository.id, - ); - } - } - } -} - /// Get the diff stats between two commits. pub fn diff_stats( repo: &git::raw::Repository, diff --git a/radicle-cli/src/commands/patch/list.rs b/radicle-cli/src/commands/patch/list.rs index 4cd00818..3014ef56 100644 --- a/radicle-cli/src/commands/patch/list.rs +++ b/radicle-cli/src/commands/patch/list.rs @@ -98,7 +98,8 @@ pub fn row( ) -> anyhow::Result<[term::Line; 9]> { let state = patch.state(); let (_, revision) = patch.latest(); - let stats = common::diff_stats(repository.raw(), revision.base(), &revision.head())?; + let (from, to) = patch.range(repository)?; + let stats = common::diff_stats(repository.raw(), &from, &to)?; let author = patch.author().id; Ok([ diff --git a/radicle-cli/src/commands/patch/show.rs b/radicle-cli/src/commands/patch/show.rs index a6a5602f..11c1b01b 100644 --- a/radicle-cli/src/commands/patch/show.rs +++ b/radicle-cli/src/commands/patch/show.rs @@ -10,17 +10,15 @@ use radicle_term::{ use crate::terminal as term; -use super::common::*; use super::*; -fn show_patch_diff(patch: &patch::Patch, storage: &Repository) -> anyhow::Result<()> { - let target_head = patch_merge_target_oid(patch.target(), storage)?; - let base_oid = storage.raw().merge_base(target_head, **patch.head())?; - let diff = format!("{}..{}", base_oid, patch.head()); +fn show_patch_diff(patch: &patch::Patch, stored: &Repository) -> anyhow::Result<()> { + let (from, to) = patch.range(stored)?; + let range = format!("{}..{}", from, to); process::Command::new("git") - .current_dir(storage.path()) - .args(["log", "--patch", &diff]) + .current_dir(stored.path()) + .args(["log", "--patch", &range]) .stdout(process::Stdio::inherit()) .stderr(process::Stdio::inherit()) .spawn()? @@ -30,7 +28,8 @@ fn show_patch_diff(patch: &patch::Patch, storage: &Repository) -> anyhow::Result } fn patch_commits(patch: &patch::Patch, stored: &Repository) -> anyhow::Result> { - let range = format!("{}..{}", patch.base(), patch.head()); + let (from, to) = patch.range(stored)?; + let range = format!("{}..{}", from, to); let mut revwalk = stored.revwalk(*patch.head())?; let mut lines = Vec::new(); @@ -67,8 +66,11 @@ pub fn run( let (_, revision) = patch.latest(); let state = patch.state(); let branches = common::branches(&revision.head(), workdir)?; - let target_head = common::patch_merge_target_oid(patch.target(), stored)?; - let ahead_behind = common::ahead_behind(stored.raw(), revision.head().into(), target_head)?; + let ahead_behind = common::ahead_behind( + stored.raw(), + *revision.head(), + *patch.target().head(stored)?, + )?; let mut attrs = Table::<2, term::Line>::new(TableOptions { spacing: 2, diff --git a/radicle-cli/tests/commands.rs b/radicle-cli/tests/commands.rs index adbd3051..efe856da 100644 --- a/radicle-cli/tests/commands.rs +++ b/radicle-cli/tests/commands.rs @@ -1,6 +1,6 @@ -use std::env; use std::path::Path; use std::str::FromStr; +use std::{env, fs}; use std::{thread, time}; use radicle::git; @@ -276,6 +276,28 @@ fn rad_patch() { test("examples/rad-patch.md", working.path(), Some(home), []).unwrap(); } +#[test] +fn rad_patch_ahead_behind() { + 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()); + + fs::write(working.path().join("CONTRIBUTORS"), "Alice Jones\n").unwrap(); + + test("examples/rad-init.md", working.path(), Some(home), []).unwrap(); + test( + "examples/rad-patch-ahead-behind.md", + working.path(), + Some(home), + [], + ) + .unwrap(); +} + #[test] fn rad_patch_draft() { let mut environment = Environment::new(); diff --git a/radicle/src/cob/patch.rs b/radicle/src/cob/patch.rs index 0473963d..a12c25aa 100644 --- a/radicle/src/cob/patch.rs +++ b/radicle/src/cob/patch.rs @@ -24,6 +24,7 @@ use crate::cob::thread::Thread; use crate::cob::{store, ActorId, EntryId, ObjectId, TypeName}; use crate::crypto::{PublicKey, Signer}; use crate::git; +use crate::identity; use crate::identity::doc::DocError; use crate::identity::PayloadError; use crate::prelude::*; @@ -155,6 +156,18 @@ pub enum MergeTarget { Delegates, } +impl MergeTarget { + /// Get the head of the target branch. + pub fn head(&self, repo: &R) -> Result { + match self { + MergeTarget::Delegates => { + let (_, target) = repo.head()?; + Ok(target) + } + } + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct Patch { /// Title of the patch. @@ -290,6 +303,28 @@ impl Patch { &self.latest().1.base } + /// Get the merge base of this patch. + pub fn merge_base(&self, repo: &R) -> Result { + let target = self.target().head(repo).map_err(|_| { + git::ext::Error::NotFound(git::ext::NotFound::NoSuchBranch(String::from("HEAD"))) + })?; + let base = repo.merge_base(&target, self.head())?; + + Ok(base) + } + + /// Get the commit range of this patch. + pub fn range( + &self, + repo: &R, + ) -> Result<(git::Oid, git::Oid), git::ext::Error> { + if self.is_merged() { + Ok((*self.base(), *self.head())) + } else { + Ok((self.merge_base(repo)?, *self.head())) + } + } + /// Index of latest revision in the revisions list. pub fn version(&self) -> RevisionIx { self.revisions @@ -320,6 +355,11 @@ impl Patch { self.latest().1.timestamp() } + /// Check if the patch is merged. + pub fn is_merged(&self) -> bool { + matches!(self.state(), State::Merged { .. }) + } + /// Check if the patch is open. pub fn is_open(&self) -> bool { matches!(self.state(), State::Open { .. }) diff --git a/radicle/src/storage.rs b/radicle/src/storage.rs index 1f4a93ad..36f3b2b9 100644 --- a/radicle/src/storage.rs +++ b/radicle/src/storage.rs @@ -437,6 +437,9 @@ pub trait ReadRepository { /// Get the repository's identity document at a specific commit. fn identity_doc_at(&self, head: Oid) -> Result, DocError>; + + /// Get the merge base of two commits. + fn merge_base(&self, left: &Oid, right: &Oid) -> Result; } /// Allows read-write access to a repository. diff --git a/radicle/src/storage/git.rs b/radicle/src/storage/git.rs index 7dfaf881..6a688bd8 100644 --- a/radicle/src/storage/git.rs +++ b/radicle/src/storage/git.rs @@ -595,6 +595,13 @@ impl ReadRepository for Repository { } Ok(longest.into()) } + + fn merge_base(&self, left: &Oid, right: &Oid) -> Result { + self.backend + .merge_base(**left, **right) + .map(Oid::from) + .map_err(ext::Error::from) + } } impl WriteRepository for Repository { diff --git a/radicle/src/test/storage.rs b/radicle/src/test/storage.rs index cb00ebb3..1af656e4 100644 --- a/radicle/src/test/storage.rs +++ b/radicle/src/test/storage.rs @@ -233,6 +233,10 @@ impl ReadRepository for MockRepository { fn canonical_identity_head(&self) -> Result { Ok(Oid::from_str("cccccccccccccccccccccccccccccccccccccccc").unwrap()) } + + fn merge_base(&self, _left: &Oid, _right: &Oid) -> Result { + todo!() + } } impl WriteRepository for MockRepository {