diff --git a/radicle-cli/examples/git/git-push-amend.md b/radicle-cli/examples/git/git-push-amend.md new file mode 100644 index 00000000..f656d626 --- /dev/null +++ b/radicle-cli/examples/git/git-push-amend.md @@ -0,0 +1,27 @@ +``` ~alice +$ rad id update --title "Add Bob" --delegate did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk --repo rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji -q +c036c0d89ce26aef3ad7da402157dba16b5163b4 +``` + +``` ~bob +$ rad sync --fetch +✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6MknSL…StBU8Vi.. +✓ Fetched repository from 1 seed(s) +``` + +``` ~alice +$ git commit -m "New changes" --allow-empty -q +$ git push rad master -o no-sync +``` + +``` ~alice +$ git commit --amend -m "Neue Änderungen" --allow-empty -q +``` + +``` ~alice (stderr) +$ git push rad master -f +✓ Canonical head updated to 9170c8795d3a78f0381a0ffafb20ea69fb0f5b6b +✓ Synced with 1 node(s) +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + + fb25886...9170c87 master -> master (forced update) +``` diff --git a/radicle-cli/examples/git/git-push-converge.md b/radicle-cli/examples/git/git-push-converge.md index 9ae27f7f..c99ecec8 100644 --- a/radicle-cli/examples/git/git-push-converge.md +++ b/radicle-cli/examples/git/git-push-converge.md @@ -2,11 +2,11 @@ In this scenario we check that we can easily reset our canonical head to the head of another delegate when there is divergence between the 3 delegates. First we add our new delegates, Bob & Eve, to our repo, while also setting the -`threshold` to `2`: +`threshold` to `3`: ``` ~alice -$ rad id update --title "Add Bob & Eve" --description "" --delegate did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk --delegate did:key:z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z --threshold 2 --repo rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji -q -73bd6ea3f88eb0687afd13ee13bfb31a9eb0ccd2 +$ rad id update --title "Add Bob & Eve" --delegate did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk --delegate did:key:z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z --threshold 3 --repo rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji -q +3143236b2e40338f5574ec04e935a5ab80a6868a ``` Bob and Eve will fetch the changes to ensure they hear about their delegate @@ -37,14 +37,16 @@ f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354 refs/heads/master ``` ``` ~bob -$ git commit -m "Bob's commit" --allow-empty -q +$ git add README +$ git commit -m "Bob's commit" -q $ git push rad -o no-sync $ git ls-remote rad f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354 refs/heads/master ``` ``` ~eve -$ git commit -m "Eve's commit" --allow-empty -q +$ git add README +$ git commit -m "Eve's commit" -q $ git push rad -o no-sync $ git ls-remote rad f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354 refs/heads/master @@ -74,8 +76,12 @@ remedied by the delegates agreeing upon which way to move forward. In this case, Alice resets her `master` to `bob/master`: ``` ~alice -$ git reset bob/master --hard -HEAD is now at 0801f02 Bob's commit +$ git merge bob/master +Merge made by the 'ort' strategy. + README | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) +$ git merge eve/master -s ours +Merge made by the 'ours' strategy. ``` She can then force push to update the canonical head to the new agreed upon @@ -83,29 +89,54 @@ commit: ``` ~alice (stderr) $ git push rad -f -✓ Canonical head updated to 0801f020f11a08ec7a96d1710ec24e5e005499d1 -✓ Synced with 2 node(s) +warn: no quorum was found for `refs/heads/master` +warn: it is recommended to find a commit to agree upon +✓ Synced with 1 node(s) To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi - + d09e634...0801f02 master -> master (forced update) -``` - -We can convince ourselves that the canonical branch has indeed changed by using -`git ls-remote` for each of the delegates: - -``` ~alice -$ git ls-remote rad -0801f020f11a08ec7a96d1710ec24e5e005499d1 refs/heads/master + d09e634..0f9bd80 master -> master ``` ``` ~bob -$ git ls-remote rad -0801f020f11a08ec7a96d1710ec24e5e005499d1 refs/heads/master +$ rad remote add did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi --name alice +✓ Follow policy updated for z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (alice) +✗ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6MknSL…StBU8Vi.. error: no quorum was found +✗ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkux1…nVhib7Z.. error: no quorum was found +✓ Remote alice added +✓ Remote-tracking branch alice/master created for z6MknSL…StBU8Vi +$ git reset --hard alice/master +HEAD is now at 0f9bd80 Merge remote-tracking branch 'eve/master' ``` +When we check Bob's `rad` remote, we see that the commit for `refs/heads/master` +has changed. This is actually Eve's commit as part of Alice merging above. Now +that Alice, Bob, and Eve all have this commit as part of their history it has +become the canonical `master`. + +``` ~bob (stderr) +$ git push rad +✓ Canonical head updated to 3a75f66dd0020c9a0355cc6ec21f15de989e2001 +✓ Synced with 2 node(s) +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk + 2a37862..0f9bd80 master -> master +``` + +Once Eve also resets to the merge commits, the canonical `master` is set to this tip. + ``` ~eve -$ git ls-remote rad -0801f020f11a08ec7a96d1710ec24e5e005499d1 refs/heads/master +$ rad remote add did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi --name alice +✓ Follow policy updated for z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (alice) +✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkt67…v4N1tRk.. +✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6MknSL…StBU8Vi.. +✓ Remote alice added +✓ Remote-tracking branch alice/master created for z6MknSL…StBU8Vi +$ git reset --hard alice/master +HEAD is now at 0f9bd80 Merge remote-tracking branch 'eve/master' ``` - - +``` ~eve (stderr) +$ git push rad +✓ Canonical head updated to 0f9bd8035c04b3f73f5408e73e8454879b20800b +✓ Synced with 2 node(s) +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z + 3a75f66..0f9bd80 master -> master +``` diff --git a/radicle-cli/examples/git/git-push-diverge.md b/radicle-cli/examples/git/git-push-diverge.md index 09adb837..cbf8fb3d 100644 --- a/radicle-cli/examples/git/git-push-diverge.md +++ b/radicle-cli/examples/git/git-push-diverge.md @@ -66,19 +66,14 @@ To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkE f2de534..f6cff86 master -> master ``` -One thing of note is that we can't push an older commit either, by default: +One thing of note is that we can revert to an older commit as long as we are +still ahead of the other delegates. ``` ~alice $ git reset --hard HEAD^ -q ``` -``` ~alice (fail) -$ git push -f -``` - -We have to use the `allow.rollback` option: - ``` ~alice RAD_SOCKET=/dev/null (stderr) -$ git push -f -o allow.rollback +$ git push -f ✓ Canonical head updated to 319a7dc3b195368ded4b099f8c90bbb80addccd3 To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + f6cff86...319a7dc master -> master (forced update) diff --git a/radicle-cli/examples/git/git-push-rollback.md b/radicle-cli/examples/git/git-push-rollback.md new file mode 100644 index 00000000..1994624b --- /dev/null +++ b/radicle-cli/examples/git/git-push-rollback.md @@ -0,0 +1,60 @@ +In this scenario, we will explore being able to rollback to a previous commit. + +First we add a second delegate, Bob, to our repo. We also change the threshold +to 2: + +``` ~alice +$ rad id update --title "Add Bob" --delegate did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk --repo rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji --threshold 2 -q +069e7d58faa9a7473d27f5510d676af33282796f +``` + +Bob then syncs these changes and adds a new commit: + +``` ~bob +$ rad sync --fetch +✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6MknSL…StBU8Vi.. +✓ Fetched repository from 1 seed(s) +$ git commit -m "Third commit" --allow-empty -q +$ git push rad +$ git branch -arv + alice@z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/master f2de534 Second commit + rad/master 319a7dc Third commit +``` + +Alice merges these changes and pushes them, which updates the canonical head: + +``` ~alice +$ rad remote add did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk --name bob --fetch --no-sync +✓ Remote bob added +✓ Remote-tracking branch bob/master created for z6Mkt67…v4N1tRk +$ git merge bob/master +Updating f2de534..319a7dc +Fast-forward +``` + +``` ~alice (stderr) +$ git push rad +✓ Canonical head updated to 319a7dc3b195368ded4b099f8c90bbb80addccd3 +✓ Synced with 1 node(s) +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + f2de534..319a7dc master -> master +``` + +Alice decides that she changes her mind about these changes and rolls back to +the previous commit: + +``` ~alice +$ git reset --hard f2de534 +HEAD is now at f2de534 Second commit +``` + +Since the canonical head is still decidable from this commit she is allowed to +push and the new canonical head becomes the previous commit again: + +``` ~alice (stderr) +$ git push rad -f +✓ Canonical head updated to f2de534b5e81d7c6e2dcaf58c3dd91573c0a0354 +✓ Synced with 1 node(s) +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + + 319a7dc...f2de534 master -> master (forced update) +``` diff --git a/radicle-cli/tests/commands.rs b/radicle-cli/tests/commands.rs index c3fa230e..1eddcdd4 100644 --- a/radicle-cli/tests/commands.rs +++ b/radicle-cli/tests/commands.rs @@ -2443,6 +2443,8 @@ fn git_push_diverge() { #[test] fn git_push_converge() { + use std::fs; + let mut environment = Environment::new(); let alice = environment.node(Config::test(Alias::new("alice"))); let bob = environment.node(Config::test(Alias::new("bob"))); @@ -2471,6 +2473,17 @@ fn git_push_converge() { alice.has_remote_of(&acme, &bob.id); alice.has_remote_of(&acme, &eve.id); + fs::write( + working.join("bob").join("heartwood").join("README"), + "Hello\n", + ) + .unwrap(); + fs::write( + working.join("eve").join("heartwood").join("README"), + "Hello, world!\n", + ) + .unwrap(); + formula(&environment.tmp(), "examples/git/git-push-converge.md") .unwrap() .home( @@ -2492,6 +2505,88 @@ fn git_push_converge() { .unwrap(); } +#[test] +fn git_push_amend() { + let mut environment = Environment::new(); + let alice = environment.node(Config::test(Alias::new("alice"))); + let bob = environment.node(Config::test(Alias::new("bob"))); + let working = environment.tmp().join("working"); + let acme = RepoId::from_str("z42hL2jL4XNk6K8oHQaSWfMgCL7ji").unwrap(); + + fixtures::repository(working.join("alice")); + + test( + "examples/rad-init.md", + working.join("alice"), + Some(&alice.home), + [], + ) + .unwrap(); + + let alice = alice.spawn(); + let mut bob = bob.spawn(); + + bob.connect(&alice).converge([&alice]); + bob.fork(acme, working.join("bob")).unwrap(); + alice.has_remote_of(&acme, &bob.id); + + formula(&environment.tmp(), "examples/git/git-push-amend.md") + .unwrap() + .home( + "alice", + working.join("alice"), + [("RAD_HOME", alice.home.path().display())], + ) + .home( + "bob", + working.join("bob").join("heartwood"), + [("RAD_HOME", bob.home.path().display())], + ) + .run() + .unwrap(); +} + +#[test] +fn git_push_rollback() { + let mut environment = Environment::new(); + let alice = environment.node(Config::test(Alias::new("alice"))); + let bob = environment.node(Config::test(Alias::new("bob"))); + let working = environment.tmp().join("working"); + let acme = RepoId::from_str("z42hL2jL4XNk6K8oHQaSWfMgCL7ji").unwrap(); + + fixtures::repository(working.join("alice")); + + test( + "examples/rad-init.md", + working.join("alice"), + Some(&alice.home), + [], + ) + .unwrap(); + + let alice = alice.spawn(); + let mut bob = bob.spawn(); + + bob.connect(&alice).converge([&alice]); + bob.fork(acme, working.join("bob")).unwrap(); + alice.has_remote_of(&acme, &bob.id); + + formula(&environment.tmp(), "examples/git/git-push-rollback.md") + .unwrap() + .home( + "alice", + working.join("alice"), + [("RAD_HOME", alice.home.path().display())], + ) + .home( + "bob", + working.join("bob").join("heartwood"), + [("RAD_HOME", bob.home.path().display())], + ) + .run() + .unwrap(); +} + #[test] fn rad_push_and_pull_patches() { let mut environment = Environment::new(); diff --git a/radicle-remote-helper/src/lib.rs b/radicle-remote-helper/src/lib.rs index bcee1452..165a27e1 100644 --- a/radicle-remote-helper/src/lib.rs +++ b/radicle-remote-helper/src/lib.rs @@ -68,11 +68,6 @@ pub enum Error { List(#[from] list::Error), } -#[derive(Debug, Default, Clone)] -pub struct Allow { - rollback: bool, -} - #[derive(Debug, Default, Clone)] pub struct Options { /// Don't sync after push. @@ -87,8 +82,6 @@ pub struct Options { base: Option, /// Patch message. message: cli::patch::Message, - /// Operations allowed. - allow: Allow, } /// Run the radicle remote helper using the given profile. @@ -215,7 +208,6 @@ fn push_option(args: &[&str], opts: &mut Options) -> Result<(), Error> { ["sync.debug"] => opts.sync_debug = true, ["no-sync"] => opts.no_sync = true, ["patch.draft"] => opts.draft = true, - ["allow.rollback"] => opts.allow.rollback = true, _ => { let args = args.join(" "); @@ -259,3 +251,8 @@ pub(crate) fn read_line<'a>(stdin: &io::Stdin, line: &'a mut String) -> io::Resu pub(crate) fn hint(s: impl fmt::Display) { eprintln!("{}", cli::format::hint(format!("hint: {s}"))); } + +/// Write a warning to the user. +pub(crate) fn warn(s: impl fmt::Display) { + eprintln!("{}", cli::format::hint(format!("warn: {s}"))); +} diff --git a/radicle-remote-helper/src/push.rs b/radicle-remote-helper/src/push.rs index fb63b40d..d2bb13f4 100644 --- a/radicle-remote-helper/src/push.rs +++ b/radicle-remote-helper/src/push.rs @@ -13,6 +13,8 @@ use radicle::cob::patch; use radicle::cob::patch::cache::Patches as _; use radicle::crypto::Signer; use radicle::explorer::ExplorerResource; +use radicle::git::canonical; +use radicle::git::canonical::Canonical; use radicle::identity::Did; use radicle::node; use radicle::node::{Handle, NodeId}; @@ -24,7 +26,7 @@ use radicle::{git, rad}; use radicle_cli as cli; use radicle_cli::terminal as term; -use crate::{hint, read_line, Options}; +use crate::{hint, read_line, warn, Options}; #[derive(Debug, Error)] pub enum Error { @@ -191,7 +193,6 @@ pub fn run( _ => return Err(Error::InvalidCommand(line.trim().to_owned())), } } - let canonical = stored.head()?; let delegates = stored.delegates()?; // For each refspec, push a ref or delete a ref. @@ -203,7 +204,7 @@ pub fn run( Command::Delete(dst) => { // Delete refs. let refname = nid.to_namespace().join(dst); - let (canonical_ref, _) = &canonical; + let (canonical_ref, _) = &stored.head()?; if *dst == canonical_ref.to_ref_string() && delegates.contains(&Did::from(nid)) { return Err(Error::DeleteForbidden(dst.clone())); @@ -249,7 +250,10 @@ pub fn run( opts.clone(), ) } else { - let (canonical_ref, canonical_oid) = &canonical; + let identity = stored.identity()?; + let project = identity.project()?; + let canonical_ref = git::refs::branch(project.default_branch()); + let me = Did::from(nid); // If we're trying to update the canonical head, make sure // we don't diverge from the current head. This only applies @@ -257,41 +261,51 @@ pub fn run( // // Note that we *do* allow rolling back to a previous commit on the // canonical branch. - if dst == *canonical_ref - && delegates.contains(&Did::from(nid)) - && delegates.len() > 1 - { - if let Err(e) = working.find_commit(**canonical_oid) { - return if git::ext::is_not_found_err(&e) { - Err(Error::MissingCanonicalHead(*canonical_oid)) - } else { - Err(e.into()) - }; - } + if dst == canonical_ref && delegates.contains(&me) && delegates.len() > 1 { let head = working.find_reference(src.as_str())?; let head = head.peel_to_commit()?.id(); - // Rollback is allowed and head is an ancestor of the canonical head. - let rollback = opts.allow.rollback - && working.graph_descendant_of(**canonical_oid, head)?; - - if head != **canonical_oid - // Canonical head is *not* an ancestor of head. - && !working.graph_descendant_of(head, **canonical_oid)? - // Not a rollback. - && !rollback - { - if hints { - hint( - "you are attempting to push a commit that would cause \ - your upstream to diverge from the canonical head", - ); - hint( - "to integrate the remote changes, run `git pull --rebase` \ - and try again", - ); - } - return Err(Error::HeadsDiverge(head.into(), *canonical_oid)); + let mut canonical = + Canonical::default_branch(stored, &project, &identity.delegates)?; + let converges = canonical::converges( + canonical + .tips() + .filter_map(|(did, tip)| (*did != me).then_some(tip)), + head.into(), + &working, + )?; + if converges { + canonical.modify_vote(me, head.into()); } + + match canonical.quorum(identity.threshold, &working) { + Ok(canonical_oid) => { + // Canonical head is an ancestor of head. + let is_ff = head == *canonical_oid + || working.graph_descendant_of(head, *canonical_oid)?; + + if !is_ff && !converges { + if hints { + hint( + "you are attempting to push a commit that would cause \ + your upstream to diverge from the canonical head", + ); + hint( + "to integrate the remote changes, run `git pull --rebase` \ + and try again", + ); + } + return Err(Error::HeadsDiverge( + head.into(), + canonical_oid, + )); + } + } + Err(canonical::QuorumError::NoQuorum) => { + warn(format!("no quorum was found for `{canonical_ref}`")); + warn("it is recommended to find a commit to agree upon"); + } + Err(e) => return Err(e.into()), + }; } push( src, @@ -322,15 +336,17 @@ pub fn run( // Sign refs and sync if at least one ref pushed successfully. if !ok.is_empty() { let _ = stored.sign_refs(&signer)?; - let head = stored.set_head()?; - if head.is_updated() { - eprintln!( - "{} Canonical head updated to {}", - term::format::positive("✓"), - term::format::secondary(head.new), - ); - } + // N.b. if an error occurs then there may be no quorum + if let Ok(head) = stored.set_head() { + if head.is_updated() { + eprintln!( + "{} Canonical head updated to {}", + term::format::positive("✓"), + term::format::secondary(head.new), + ); + } + }; if !opts.no_sync { if profile.policies()?.is_seeding(&stored.id)? {