cli: Remove `rad patch open`

Remove this command in favor of the `git push` flow.

We also remove the `rad/patches` remote configuration which wasn't
working properly with `git fetch --all`.
This commit is contained in:
Alexis Sellier 2023-06-07 15:57:11 +02:00
parent 547fdcc03b
commit 3d803b2498
No known key found for this signature in database
12 changed files with 41 additions and 296 deletions

View File

@ -68,13 +68,12 @@ $ git ls-remote rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JU
2647168c23e7c2b2c1936d695443944e143bc3f7 refs/cobs/xyz.radicle.patch/2647168c23e7c2b2c1936d695443944e143bc3f7
```
We can also create patches by pushing to the `rad/patches` remote. It's a bit
simpler:
We can create another patch:
``` (stderr)
$ git checkout -b feature/2 -q master
$ git commit -a -m "Add more things" -q --allow-empty
$ git push rad/patches
$ git push rad HEAD:refs/patches
✓ Patch b8ab1c99c1c8205680a3494f04fb3934ec738ddd opened
To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi
* [new reference] HEAD -> refs/patches
@ -101,13 +100,6 @@ $ rad patch
╰────────────────────────────────────────────────────────────────────────────────────╯
```
Note that we can't fetch from `rad/patches`:
``` (stderr) (fail)
$ git fetch rad/patches
fatal: couldn't find remote ref refs/patches
```
To update our patch, we simply push commits to the upstream branch:
```

View File

@ -24,17 +24,11 @@ $ git commit -v -m "Define power requirements"
Once the code is ready, we open (or create) a patch with our changes for the project.
```
$ rad patch open --message "Define power requirements" --message "See details."
master <- z6MknSLStBU8Vi/flux-capacitor-power (3e674d1)
1 commit(s) ahead, 0 commit(s) behind
3e674d1 Define power requirements
✓ Patch 077e4bbe9a6e5546f400ef5951768c37a76f13a4 created
To publish your patch to the network, run:
git push rad
``` (stderr)
$ git push rad -o patch.message="Define power requirements" -o patch.message="See details." HEAD:refs/patches
✓ Patch 077e4bbe9a6e5546f400ef5951768c37a76f13a4 opened
To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi
* [new reference] HEAD -> refs/patches
```
It will now be listed as one of the project's open patches.
@ -80,7 +74,7 @@ index 0000000..e69de29
We can also see that it set an upstream for our patch branch:
```
$ git branch -vv
* flux-capacitor-power 3e674d1 [rad/flux-capacitor-power] Define power requirements
* flux-capacitor-power 3e674d1 [rad/patches/077e4bbe9a6e5546f400ef5951768c37a76f13a4] Define power requirements
master f2de534 [rad/master] Second commit
```

View File

@ -9,10 +9,9 @@ Now, we can see that there is a new remote in the list of remotes:
```
$ rad remote list
bob z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk (fetch)
rad (canonical upstream) (fetch)
rad z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (push)
rad/patches (patches upstream) (push)
bob z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk (fetch)
rad (canonical upstream) (fetch)
rad z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (push)
```
You can see both `bob` and `rad` as remotes. The `rad` remote is our personal remote of the project.

View File

@ -24,17 +24,11 @@ $ git commit -v -m "Define power requirements"
Once the code is ready, we open a patch with our changes.
```
$ rad patch open --message "Define power requirements" --message "See details."
master <- z6Mkt67v4N1tRk/flux-capacitor-power (3e674d1)
1 commit(s) ahead, 0 commit(s) behind
3e674d1 Define power requirements
✓ Patch 50e29a111972f3b7d2123c5057de5bdf09bc7b1c created
To publish your patch to the network, run:
git push rad
``` (stderr)
$ git push rad -o no-sync -o patch.message="Define power requirements" -o patch.message="See details." HEAD:refs/patches
✓ Patch 50e29a111972f3b7d2123c5057de5bdf09bc7b1c opened
To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk
* [new reference] HEAD -> refs/patches
```
It will now be listed as one of the project's open patches.
@ -67,8 +61,8 @@ $ rad patch show 50e29a111972f3b7d2123c5057de5bdf09bc7b1c
We can also confirm that the patch branch is in storage:
```
$ git ls-remote rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk refs/heads/flux-capacitor-power
3e674d1a1df90807e934f9ae5da2591dd6848a33 refs/heads/flux-capacitor-power
$ git ls-remote rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk refs/heads/patches/*
3e674d1a1df90807e934f9ae5da2591dd6848a33 refs/heads/patches/50e29a111972f3b7d2123c5057de5bdf09bc7b1c
```
Wait, let's add a README too! Just for fun.
@ -80,10 +74,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" 50e29a111972f3b7d2123c5057de5bdf09bc7b1c
Updating 3e674d1 -> 27857ec
1 commit(s) ahead, 0 commit(s) behind
✓ Patch updated to revision 3530243d46a2e7a8e4eac7afcbb17cc7c56b3d29
```
``` (stderr) RAD_SOCKET=/dev/null
$ git push -o patch.message="Add README, just for the fun"
✓ Patch 50e29a1 updated to 3530243d46a2e7a8e4eac7afcbb17cc7c56b3d29
To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk
3e674d1..27857ec flux-capacitor-power -> patches/50e29a111972f3b7d2123c5057de5bdf09bc7b1c
```
And let's leave a quick comment for our team:

View File

@ -21,16 +21,16 @@ $ rad remote add z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk --name bob
``` (stderr)
$ git fetch bob
From rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk
* [new branch] flux-capacitor-power -> bob/flux-capacitor-power
* [new branch] master -> bob/master
* [new branch] master -> bob/master
* [new branch] patches/50e29a111972f3b7d2123c5057de5bdf09bc7b1c -> bob/patches/50e29a111972f3b7d2123c5057de5bdf09bc7b1c
```
The contributor's changes are now visible to us.
```
$ git branch -r
bob/flux-capacitor-power
bob/master
bob/patches/50e29a111972f3b7d2123c5057de5bdf09bc7b1c
rad/master
$ rad patch show 50e29a1
╭──────────────────────────────────────────────────────────────────────────────╮
@ -57,11 +57,11 @@ way will tell others about the corrections we needed before merging the
changes.
```
$ git checkout flux-capacitor-power
branch 'flux-capacitor-power' set up to track 'bob/flux-capacitor-power'.
$ git checkout patches/50e29a111972f3b7d2123c5057de5bdf09bc7b1c
branch 'patches/50e29a111972f3b7d2123c5057de5bdf09bc7b1c' set up to track 'bob/patches/50e29a111972f3b7d2123c5057de5bdf09bc7b1c'.
$ git mv REQUIREMENTS REQUIREMENTS.md
$ git commit --fixup HEAD~
[flux-capacitor-power f6484e0] fixup! Define power requirements
[patches/50e29a111972f3b7d2123c5057de5bdf09bc7b1c f6484e0] fixup! Define power requirements
1 file changed, 0 insertions(+), 0 deletions(-)
rename REQUIREMENTS => REQUIREMENTS.md (100%)
$ rad patch update --message "Define power requirements" --message "See details." 50e29a111972f3b7d2123c5057de5bdf09bc7b1c
@ -75,7 +75,7 @@ Great, all fixed up, lets merge the code.
```
$ git checkout master
Your branch is up to date with 'rad/master'.
$ git merge flux-capacitor-power
$ git merge patches/50e29a111972f3b7d2123c5057de5bdf09bc7b1c
Updating f2de534..f6484e0
Fast-forward
README.md | 0
@ -95,7 +95,7 @@ $ rad patch show 50e29a1
│ Patch 50e29a111972f3b7d2123c5057de5bdf09bc7b1c │
│ Author did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk │
│ Head f6484e0f43e48a8983b9b39bf9bd4cd889f1d520 │
│ Branches flux-capacitor-power, master
│ Branches master, patches/50e29a111972f3b7d2123c5057de5bdf09bc7b1c
│ Commits up to date │
│ Status merged │
│ │

View File

@ -117,7 +117,8 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
let default_branch = proj.default_branch().clone();
let path = working.workdir().unwrap(); // SAFETY: The working copy is not bare.
// Setup tracking for project delegates.
// Configure repository and setup tracking for project delegates.
radicle::git::configure_repository(&working)?;
checkout::setup_remotes(
project::SetupRemote {
project: options.id,

View File

@ -4,8 +4,6 @@ mod archive;
mod checkout;
#[path = "patch/common.rs"]
mod common;
#[path = "patch/create.rs"]
mod create;
#[path = "patch/delete.rs"]
mod delete;
#[path = "patch/edit.rs"]
@ -46,7 +44,6 @@ Usage
rad patch [<option>...]
rad patch list [--all|--merged|--open|--archived|--draft] [<option>...]
rad patch show <patch-id> [<option>...]
rad patch open [--draft] [<option>...]
rad patch archive <patch-id> [<option>...]
rad patch update <patch-id> [<option>...]
rad patch checkout <patch-id> [<option>...]
@ -63,9 +60,8 @@ Edit options
-m, --message [<string>] Provide a comment message to the patch or revision (default: prompt)
Open/Update options
Update options
--draft Open patch in draft mode
-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)
@ -92,7 +88,6 @@ Other options
#[derive(Debug, Default, PartialEq, Eq)]
pub enum OperationName {
Open,
Show,
Update,
Archive,
@ -128,11 +123,6 @@ impl std::fmt::Debug for Filter {
#[derive(Debug)]
pub enum Operation {
Open {
message: Message,
draft: bool,
quiet: bool,
},
Show {
patch_id: Rev,
diff: bool,
@ -191,7 +181,6 @@ impl Args for Options {
let mut push = true;
let mut filter = Filter::default();
let mut diff = false;
let mut draft = false;
let mut undo = false;
let mut quiet = false;
@ -227,13 +216,8 @@ impl Args for Options {
push = false;
}
// Open/update options.
Long("draft") if op == Some(OperationName::Open) => {
draft = true;
}
Long("quiet") | Short('q')
if op == Some(OperationName::Open) || op == Some(OperationName::Update) =>
{
// Update options.
Long("quiet") | Short('q') if op == Some(OperationName::Update) => {
quiet = true;
}
@ -274,7 +258,6 @@ impl Args for Options {
Value(val) if op.is_none() => match val.to_string_lossy().as_ref() {
"l" | "list" => op = Some(OperationName::List),
"o" | "open" => op = Some(OperationName::Open),
"s" | "show" => op = Some(OperationName::Show),
"u" | "update" => op = Some(OperationName::Update),
"d" | "delete" => op = Some(OperationName::Delete),
@ -310,11 +293,6 @@ impl Args for Options {
}
let op = match op.unwrap_or_default() {
OperationName::Open => Operation::Open {
message,
draft,
quiet,
},
OperationName::List => Operation::List { filter },
OperationName::Show => Operation::Show {
patch_id: patch_id.ok_or_else(|| anyhow!("a patch must be provided"))?,
@ -374,21 +352,6 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
}
match options.op {
Operation::Open {
ref message,
draft,
quiet,
} => {
create::run(
message.clone(),
draft,
quiet,
options,
&profile,
&repository,
&workdir,
)?;
}
Operation::List { filter: Filter(f) } => {
list::run(f, &repository, &profile)?;
}

View File

@ -191,20 +191,3 @@ pub fn find_unmerged_with_base(
}
Ok(matches)
}
/// Return commits between the merge base and a head.
pub fn patch_commits<'a>(
repo: &'a git::raw::Repository,
base: &Oid,
head: &Oid,
) -> anyhow::Result<Vec<git::raw::Commit<'a>>> {
let mut commits = Vec::new();
let mut revwalk = repo.revwalk()?;
revwalk.push_range(&format!("{base}..{head}"))?;
for rev in revwalk {
let commit = repo.find_commit(rev?)?;
commits.push(commit);
}
Ok(commits)
}

View File

@ -1,134 +0,0 @@
use radicle::cob::patch;
use radicle::git;
use radicle::node::Handle;
use radicle::prelude::*;
use radicle::storage::git::Repository;
use radicle::Node;
use crate::terminal as term;
use super::common::*;
use super::Options;
fn show_patch_commit_info(
storage: &Repository,
node_id: &NodeId,
head_branch: &git::raw::Branch,
target_ref: &git::RefStr,
target_oid: git::Oid,
) -> anyhow::Result<()> {
let head_oid = branch_oid(head_branch)?;
// The merge base is basically the commit at which the histories diverge.
let base_oid = storage.backend.merge_base(*target_oid, *head_oid)?;
let commits = patch_commits(&storage.backend, &base_oid, &head_oid)?;
term::info!(
"{} <- {}/{} ({})",
term::format::highlight(target_ref),
term::format::dim(term::format::node(node_id)),
term::format::highlight(branch_name(head_branch)?),
term::format::secondary(term::format::oid(head_oid)),
);
// TODO: Test case where the target branch has been re-written passed the merge-base, since the fork was created
// This can also happen *after* the patch is created.
term::patch::print_commits_ahead_behind(&storage.backend, *head_oid, *target_oid)?;
// List commits in patch that aren't in the target branch.
term::blank();
term::patch::list_commits(&commits)?;
Ok(())
}
/// Run patch creation.
pub fn run(
message: term::patch::Message,
draft: bool,
quiet: bool,
options: Options,
profile: &Profile,
storage: &Repository,
workdir: &git::raw::Repository,
) -> anyhow::Result<()> {
let mut patches = patch::Patches::open(storage)?;
let head_branch = try_branch(workdir.head()?)?;
let head_branch_name = push_to_storage(workdir, storage, &head_branch, &options)?;
let (target_ref, target_oid) = get_merge_target(storage, &head_branch)?;
if head_branch.upstream().is_err() {
radicle::git::set_upstream(
workdir,
&*radicle::rad::REMOTE_NAME,
branch_name(&head_branch)?,
head_branch_name,
)?;
}
// TODO: Handle case where `rad/master` isn't up to date with the target.
// In that case we should warn the user that their master branch is not up
// to date, and error out, unless the user specifies manually the merge
// base.
if !quiet {
show_patch_commit_info(storage, profile.id(), &head_branch, &target_ref, target_oid)?;
term::blank();
}
// TODO: List matching working copy refs for all targets.
let head_oid = branch_oid(&head_branch)?;
let base_oid = storage.backend.merge_base(*target_oid, *head_oid)?;
let (title, description) =
term::patch::get_create_message(message, &storage.backend, &base_oid.into(), &head_oid)?;
let signer = term::signer(profile)?;
let patch = if draft {
patches.draft(
title,
&description,
patch::MergeTarget::default(),
base_oid,
head_oid,
&[],
&signer,
)
} else {
patches.create(
title,
&description,
patch::MergeTarget::default(),
base_oid,
head_oid,
&[],
&signer,
)
}?;
if !quiet {
term::success!("Patch {} created", term::format::highlight(patch.id));
term::blank();
}
if options.announce {
let mut node = Node::new(profile.socket());
match node.announce_refs(storage.id()) {
Ok(()) => {}
Err(e) if e.is_connection_err() => {
term::warning("Could not announce patch refs: node is not running");
}
Err(e) => {
return Err(e.into());
}
}
} else if !quiet {
term::info!("To publish your patch to the network, run:");
term::indented(term::format::secondary("git push rad"));
}
if quiet {
term::print(patch.id);
}
Ok(())
}

View File

@ -1,4 +1,3 @@
use radicle::rad;
use radicle_term::{Element, Table};
use crate::git;
@ -13,18 +12,10 @@ pub fn run(repo: &git::Repository) -> anyhow::Result<()> {
continue;
};
let description = if r.name == rad::PATCHES_REMOTE_NAME.as_str() {
if dir == "fetch" {
// Fetching from the patches remote is not allowed.
continue;
}
term::format::dim("(patches upstream)".to_string()).italic()
} else {
url.namespace.map_or(
term::format::dim("(canonical upstream)".to_string()).italic(),
|namespace| term::format::tertiary(namespace.to_string()),
)
};
let description = url.namespace.map_or(
term::format::dim("(canonical upstream)".to_string()).italic(),
|namespace| term::format::tertiary(namespace.to_string()),
);
table.push([
term::format::bold(r.name.clone()),
description,

View File

@ -466,33 +466,6 @@ pub fn configure_remote<'r>(
Ok(remote)
}
/// Configure a repository's patches remote.
///
/// The entry for this remote will be:
/// ```text
/// [remote.<name>]
/// url = <url>
/// pushurl = <url>
/// push HEAD:refs/patches
/// ```
pub fn configure_patches_remote<'r>(
repo: &'r git2::Repository,
name: &str,
refspec: &Refspec<RefString, RefString>,
push: &Url,
) -> Result<git2::Remote<'r>, git2::Error> {
let push = push.to_string();
let remote = repo.remote(name, &push)?;
// This fetchspec basically prevents fetching from this remote,
// as it shouldn't be used for fetching.
repo.remote_add_fetch(name, "refs/patches:refs/patches")?;
repo.remote_set_pushurl(name, Some(&push))?;
repo.remote_add_push(name, refspec.to_string().as_str())?;
Ok(remote)
}
/// Fetch from the given `remote`.
pub fn fetch(repo: &git2::Repository, remote: &str) -> Result<(), git2::Error> {
repo.find_remote(remote)?.fetch::<&str>(

View File

@ -22,8 +22,6 @@ use crate::{identity, storage};
pub static REMOTE_NAME: Lazy<git::RefString> = Lazy::new(|| git::refname!("rad"));
/// Name of the radicle storage remote.
pub static REMOTE_COMPONENT: Lazy<git::Component> = Lazy::new(|| git::fmt::name::component!("rad"));
/// Name of the radicle patches remote.
pub static PATCHES_REMOTE_NAME: Lazy<git::RefString> = Lazy::new(|| git::refname!("rad/patches"));
/// Refname used for pushing patches.
pub static PATCHES_REFNAME: Lazy<git::RefString> = Lazy::new(|| git::refname!("refs/patches"));
@ -80,17 +78,6 @@ pub fn init<G: Signer, S: WriteStorage>(
git::configure_repository(repo)?;
git::configure_remote(repo, &REMOTE_NAME, &url, &url.clone().with_namespace(*pk))?;
git::configure_patches_remote(
repo,
&PATCHES_REMOTE_NAME,
&git::Refspec {
src: git::refname!("HEAD"),
dst: PATCHES_REFNAME.clone(),
force: false,
},
&url.with_namespace(*pk),
)?;
git::push(
repo,
&REMOTE_NAME,