cli: Merge patches on `git push`

Allows patches to have their state updated to `Merged` with a `git push`
to the default branch.

Currently, due to the limitations of the COB store, this requires a
linear search through all patches. This will eventually become a problem
when projects have thousands of patches, so we should look into
long-term solutions.
This commit is contained in:
Alexis Sellier 2023-05-15 12:07:51 +02:00
parent c267ff71f1
commit 94cd7658b1
No known key found for this signature in database
7 changed files with 165 additions and 12 deletions

View File

@ -0,0 +1,46 @@
Let's start by creating two patches.
``` (stderr) RAD_SOCKET=/dev/null
$ git checkout -b feature/1 -q
$ git commit --allow-empty -q -m "First change"
$ git push rad HEAD:refs/patches
✓ Patch f4e9dcffb21bee746e0eee965933c7e237aa207a opened
To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi
* [new reference] HEAD -> refs/patches
```
``` (stderr) RAD_SOCKET=/dev/null
$ git checkout -b feature/2 -q
$ git commit --allow-empty -q -m "Second change"
$ git push rad HEAD:refs/patches
✓ Patch dce2ff0b2baf6da67fae5143b828ebfab65d41e4 opened
To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi
* [new reference] HEAD -> refs/patches
```
Then let's merge the changes into `master`.
``` (stderr) RAD_SOCKET=/dev/null
$ git checkout master
Switched to branch 'master'
$ git merge feature/1
$ git merge feature/2
```
When we push to `rad/master`, we automatically merge the patches:
``` (stderr) RAD_SOCKET=/dev/null
$ git push rad master
✓ Patch dce2ff0b2baf6da67fae5143b828ebfab65d41e4 merged
✓ Patch f4e9dcffb21bee746e0eee965933c7e237aa207a merged
To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi
f2de534..e9fff34 master -> master
```
```
$ rad patch --merged
╭──────────────────────────────────────────────────────────────────────────────────╮
│ ● ID Title Author Head + - Updated │
├──────────────────────────────────────────────────────────────────────────────────┤
│ ✔ dce2ff0 Second change z6MknSL…StBU8Vi (you) e9fff34 +0 -0 [ ... ] │
│ ✔ f4e9dcf First change z6MknSL…StBU8Vi (you) 20aa5dd +0 -0 [ ... ] │
╰──────────────────────────────────────────────────────────────────────────────────╯
```

View File

@ -813,6 +813,35 @@ fn rad_remote() {
.unwrap();
}
#[test]
fn rad_merge_via_push() {
logger::init(log::Level::Debug);
let mut environment = Environment::new();
let alice = environment.node("alice");
let working = environment.tmp().join("working");
fixtures::repository(working.join("alice"));
test(
"examples/rad-init.md",
working.join("alice"),
Some(&alice.home),
[],
)
.unwrap();
let alice = alice.spawn(Config::default());
test(
"examples/rad-merge-via-push.md",
working.join("alice"),
Some(&alice.home),
[],
)
.unwrap();
}
#[test]
fn git_push_and_pull() {
logger::init(log::Level::Debug);

View File

@ -1,6 +1,6 @@
// Copyright © 2021 The Radicle Link Contributors
use std::{collections::HashMap, error::Error};
use std::{collections::BTreeMap, error::Error};
use git_ext::ref_format::RefString;
use git_ext::Oid;
@ -70,7 +70,7 @@ pub trait Storage {
/// Get all references to objects of a given type within a particular
/// identity
fn types(&self, typename: &TypeName) -> Result<HashMap<ObjectId, Objects>, Self::TypesError>;
fn types(&self, typename: &TypeName) -> Result<BTreeMap<ObjectId, Objects>, Self::TypesError>;
/// Update a ref to a particular collaborative object
fn update(

View File

@ -1,4 +1,4 @@
use std::{collections::HashMap, convert::TryFrom as _};
use std::{collections::BTreeMap, convert::TryFrom as _};
use tempfile::TempDir;
@ -130,8 +130,8 @@ impl object::Storage for Storage {
fn types(
&self,
typename: &crate::TypeName,
) -> Result<HashMap<ObjectId, object::Objects>, Self::TypesError> {
let mut objects = HashMap::new();
) -> Result<BTreeMap<ObjectId, object::Objects>, Self::TypesError> {
let mut objects = BTreeMap::new();
for r in self.raw.references_glob("refs/rad/*")? {
let r = r?;
let name = r.name().unwrap();

View File

@ -3,14 +3,14 @@ use std::ffi::OsStr;
use std::os::fd::{AsRawFd, FromRawFd};
use std::path::Path;
use std::str::FromStr;
use std::{io, process};
use std::{assert_eq, io, process};
use radicle::storage::git::cob::object::ParseObjectId;
use thiserror::Error;
use radicle::cob::patch;
use radicle::crypto::{PublicKey, Signer};
use radicle::node::{Handle, NodeId};
use radicle::storage::git::cob::object::ParseObjectId;
use radicle::storage::git::transport::local::Url;
use radicle::storage::WriteRepository;
use radicle::storage::{self, ReadRepository};
@ -129,6 +129,8 @@ pub fn run(
let mut line = String::new();
let mut ok = HashSet::new();
assert_eq!(signer.public_key(), &nid);
// Read all the `push` lines.
loop {
let tokens = read_line(stdin, &mut line)?;
@ -168,7 +170,7 @@ pub fn run(
} else if dst == &*rad::PATCHES_REFNAME {
patch_open(src, &nid, &working, stored, &signer)
} else {
push_ref(src, dst, *force, &nid, &working, stored.raw())
push(src, dst, *force, &nid, &working, stored, &signer)
}
}
};
@ -236,7 +238,7 @@ fn patch_open<G: Signer>(
let (_, target) = stored.canonical_head()?;
let base = stored.backend.merge_base(*target, commit.id())?;
let result = match patches.create(
title,
&title,
&description,
patch::MergeTarget::default(),
base,
@ -350,6 +352,80 @@ fn patch_update<G: Signer>(
Ok(())
}
fn push<G: Signer>(
src: &git::RefStr,
dst: &git::RefStr,
force: bool,
nid: &NodeId,
working: &git::raw::Repository,
stored: &storage::git::Repository,
signer: &G,
) -> Result<(), Error> {
let head = working.find_reference(src.as_str())?;
let head = head.peel_to_commit()?.id();
// It's ok for the destination reference to be unknown, eg. when pushing a new branch.
let old = stored
.backend
.find_reference(nid.to_namespace().join(dst).as_str())
.ok();
push_ref(src, dst, force, nid, working, stored.raw())?;
if let Some(old) = old {
let proj = stored.project()?;
let master = &*git::Qualified::from(git::lit::refs_heads(proj.default_branch()));
// If we're pushing to the project's default branch, we want to see if any patches got
// merged, and if so, update the patch COB.
if dst == master {
let old = old.peel_to_commit()?.id();
// Only delegates should publish the merge result to the COB.
if stored.delegates()?.contains(&nid.into()) {
patch_merge(old.into(), head.into(), working, stored, signer)?;
}
}
}
Ok(())
}
/// Merge a patch.
fn patch_merge<G: Signer>(
old: git::Oid,
new: git::Oid,
working: &git::raw::Repository,
stored: &storage::git::Repository,
signer: &G,
) -> Result<(), Error> {
let mut revwalk = working.revwalk()?;
revwalk.push_range(&format!("{old}..{new}"))?;
let commits = revwalk
.map(|r| r.map(git::Oid::from))
.collect::<Result<HashSet<git::Oid>, _>>()?;
let mut patches = patch::Patches::open(stored)?;
for patch in patches.all()? {
let (id, patch, clock) = patch?;
let Some((revision_id, revision)) = patch.latest() else {
continue;
};
if patch.is_open() && commits.contains(&revision.head()) {
let revision_id = *revision_id;
let mut patch = patch::PatchMut::new(id, patch, clock, &mut patches);
patch.merge(revision_id, new, signer)?;
eprintln!(
"{} Patch {} merged",
cli::format::positive(""),
cli::format::tertiary(id)
);
}
}
Ok(())
}
/// Push a single reference to storage.
fn push_ref(
src: &git::RefStr,

View File

@ -970,6 +970,7 @@ impl<'a, 'g> PatchMut<'a, 'g> {
commit: git::Oid,
signer: &G,
) -> Result<EntryId, Error> {
// TODO: Don't allow merging the same revision twice?
self.transaction("Merge revision", signer, |tx| tx.merge(revision, commit))
}

View File

@ -1,5 +1,5 @@
//! COB storage Git backend.
use std::collections::HashMap;
use std::collections::BTreeMap;
use radicle_cob as cob;
use radicle_cob::change;
@ -95,7 +95,8 @@ impl cob::object::Storage for Repository {
fn types(
&self,
typename: &cob::TypeName,
) -> Result<HashMap<cob::ObjectId, cob::object::Objects>, Self::TypesError> {
) -> Result<BTreeMap<cob::ObjectId, cob::object::Objects>, Self::TypesError> {
// TODO: Use glob here.
let mut references = self.backend.references()?.filter_map(|reference| {
let reference = reference.ok()?;
match RefStr::try_from_str(reference.name()?) {
@ -115,7 +116,7 @@ impl cob::object::Storage for Repository {
}
});
references.try_fold(HashMap::new(), |mut objects, result| {
references.try_fold(BTreeMap::new(), |mut objects, result| {
let (oid, reference) = result?;
objects
.entry(oid)