diff --git a/crates/radicle-cli/examples/git/git-push-force-with-lease.md b/crates/radicle-cli/examples/git/git-push-force-with-lease.md new file mode 100644 index 00000000..4e92588a --- /dev/null +++ b/crates/radicle-cli/examples/git/git-push-force-with-lease.md @@ -0,0 +1,82 @@ +Here we show that the Radicle remote helper supports the use of +`--force-with-lease`[^1]. + +First we will set things up by pushing an initial commit: + +``` +$ git commit -m "New changes" --allow-empty -q +$ git push rad master +``` + +Now, we will create a new commit, and use the `--force-with-lease`, which should +succeed. In fact, since the current setup ensures that you can only push to your +namespace, `--force-with-lease` should always work! No other person should be +able to push to your namespace, and so the commit should never have changed from +the last time you pushed. + +``` (stderr) +$ git commit --amend -m "Neue Änderungen" --allow-empty -q +$ git push rad master --force-with-lease +✓ Canonical reference refs/heads/master updated to target commit 9170c8795d3a78f0381a0ffafb20ea69fb0f5b6b +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + + fb25886...9170c87 master -> master (forced update) +``` + +As per the documentation, you can also pass the reference name, as the expected +value, to `--force-push-lease`: + +``` (stderr) +$ git commit --amend -m "Noch mehr Änderungen" --allow-empty -q +$ git push rad master --force-with-lease=master +✓ Canonical reference refs/heads/master updated to target commit 1e4213811eb4ce67360e4a0222cab81ad11a7ffe +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + + 9170c87...1e42138 master -> master (forced update) +``` + +As well as the named reference, and its expected value: + +``` (stderr) +$ git commit --amend -m "Even more changes" --allow-empty -q +$ git push rad master --force-with-lease=master:1e4213811eb4ce67360e4a0222cab81ad11a7ffe +✓ Canonical reference refs/heads/master updated to target commit c4b74ef30953598852a82e0cd22b2ebb0d8d9e18 +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + + 1e42138...c4b74ef master -> master (forced update) +``` + +If we try use the same expected value as the last push, it should fail since the +reference was updated in the last commit: + +``` +$ git commit --amend -m "And even more" --allow-empty -q +``` + +``` (stderr) (fail) +$ git push rad master --force-with-lease=master:1e4213811eb4ce67360e4a0222cab81ad11a7ffe +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + ! [rejected] master -> master (stale info) +error: failed to push some refs to 'rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi' +``` + +And if we do not supply the commit, it should also fail, since this implies that +we expect the reference to not exist: + +``` +$ git commit --amend -m "And even more" --allow-empty -q +``` + +``` (stderr) (fail) +$ git push rad master --force-with-lease=master: +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + ! [rejected] master -> master (stale info) +error: failed to push some refs to 'rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi' +``` + +So, let's create a new branch: + +``` (stderr) +$ git push rad master:dev --force-with-lease=dev: +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + * [new branch] master -> dev +``` + +[^1]: https://git-scm.com/docs/git-push#Documentation/git-push.txt---force-with-lease diff --git a/crates/radicle-cli/examples/rad-patch-via-push.md b/crates/radicle-cli/examples/rad-patch-via-push.md index 4abe8960..7fb298fb 100644 --- a/crates/radicle-cli/examples/rad-patch-via-push.md +++ b/crates/radicle-cli/examples/rad-patch-via-push.md @@ -9,7 +9,7 @@ Switched to a new branch 'feature/1' $ git commit -a -m "Add things" -q --allow-empty $ git push -o patch.message="Add things #1" -o patch.message="See commits for details." rad HEAD:refs/patches ✓ Patch 6035d2f582afbe01ff23ea87528ae523d76875b6 opened -hint: to update, run `git push` or `git push rad -f HEAD:patches/6035d2f582afbe01ff23ea87528ae523d76875b6` +hint: to update, run `git push` or `git push rad --force-with-lease HEAD:patches/6035d2f582afbe01ff23ea87528ae523d76875b6` hint: offline push, your node is not running hint: to sync with the network, run `rad node start` To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi @@ -203,10 +203,10 @@ hint: See the 'Note about fast-forwards' in 'git push --help' for details. ``` The push fails because it's not a fast-forward update. To remedy this, we can -use `--force` to force the update. +use `--force-with-lease` (or `--force`) to force the update. ``` (stderr) -$ git push --force +$ git push --force-with-lease ✓ Patch 9580891 updated to revision 670d02794aa05afd6e0851f4aa848bc87c4712c7 To compare against your previous revision d7040c6, run: diff --git a/crates/radicle-cli/tests/commands.rs b/crates/radicle-cli/tests/commands.rs index 8f5ade9a..d5278763 100644 --- a/crates/radicle-cli/tests/commands.rs +++ b/crates/radicle-cli/tests/commands.rs @@ -2425,6 +2425,11 @@ fn git_push_amend() { .unwrap(); } +#[test] +fn git_push_force_with_lease() { + Environment::alice(["rad-init", "git/git-push-force-with-lease"]); +} + #[test] fn git_push_rollback() { let mut environment = Environment::new(); diff --git a/crates/radicle-remote-helper/src/main.rs b/crates/radicle-remote-helper/src/main.rs index e1c85d90..525b5255 100644 --- a/crates/radicle-remote-helper/src/main.rs +++ b/crates/radicle-remote-helper/src/main.rs @@ -240,6 +240,7 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> { let stdin = io::stdin(); let mut line = String::new(); let mut opts = Options::default(); + let mut expected_refs = Vec::new(); if let Err(e) = radicle::io::set_file_limit(4096) { if debug { @@ -278,6 +279,10 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> { push_option(args, &mut opts)?; println!("ok"); } + ["option", "cas", refstr] => { + expected_refs.push((*refstr).to_owned()); + println!("ok"); + } ["option", "progress", ..] | ["option", ..] => { println!("unsupported"); } @@ -301,6 +306,7 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> { &profile, &stdin, opts, + &expected_refs, )?); } ["list"] => { diff --git a/crates/radicle-remote-helper/src/push.rs b/crates/radicle-remote-helper/src/push.rs index b45f38df..a02da395 100644 --- a/crates/radicle-remote-helper/src/push.rs +++ b/crates/radicle-remote-helper/src/push.rs @@ -251,6 +251,7 @@ pub fn run( profile: &Profile, stdin: &io::Stdin, opts: Options, + expected_refs: &[String], ) -> Result<(), Error> { // Don't allow push if either of these conditions is true: // @@ -341,6 +342,7 @@ pub fn run( patches, &signer, opts.clone(), + expected_refs, ), PushAction::PushRef { dst } => { let identity = stored.identity()?; @@ -361,6 +363,7 @@ pub fn run( patches, &signer, opts.verbosity, + expected_refs, )?; // If we're trying to update the canonical head, make sure // we don't diverge from the current head. This only applies @@ -512,7 +515,14 @@ impl<'a> TempPatchRef<'a> { } fn push(&self, src: &git::Oid, verbosity: Verbosity) -> Result<(), Error> { - push_ref(src, &self.reference, false, self.stored.raw(), verbosity) + push_ref( + src, + &self.reference, + false, + self.stored.raw(), + verbosity, + &[], + ) } } @@ -655,7 +665,7 @@ where // rad/patches/deadbeef -> patches/deadbeef let name = name.split_once('/').unwrap_or_default().1; hint(format!( - "to update, run `git push` or `git push {upstream} -f HEAD:{name}`" + "to update, run `git push` or `git push {upstream} --force-with-lease HEAD:{name}`" )); } } @@ -681,6 +691,7 @@ fn patch_update( >, signer: &Device, opts: Options, + expected_refs: &[String], ) -> Result, Error> where G: crypto::signature::Signer, @@ -709,7 +720,14 @@ where term::patch::get_update_message(opts.message, &stored.backend, &latest, &head.into())?; let dst = dst.with_namespace(nid.into()); - push_ref(head, &dst, force, stored.raw(), opts.verbosity)?; + push_ref( + head, + &dst, + force, + stored.raw(), + opts.verbosity, + expected_refs, + )?; let mut patch_mut = patch::PatchMut::new(patch_id, patch, &mut patches); let revision = patch_mut.update(message, base, *head, signer)?; @@ -758,6 +776,7 @@ fn push( >, signer: &Device, verbosity: Verbosity, + expected_refs: &[String], ) -> Result, Error> where G: crypto::signature::Signer, @@ -767,7 +786,7 @@ where // It's ok for the destination reference to be unknown, eg. when pushing a new branch. let old = stored.backend.find_reference(dst.as_str()).ok(); - push_ref(src, &dst, force, stored.raw(), verbosity)?; + push_ref(src, &dst, force, stored.raw(), verbosity, expected_refs)?; if let Some(old) = old { let proj = stored.project()?; @@ -950,6 +969,7 @@ fn push_ref( force: bool, stored: &git::raw::Repository, verbosity: Verbosity, + expected_refs: &[String], ) -> Result<(), Error> { let path = dunce::canonicalize(stored.path())?.display().to_string(); // Nb. The *force* indicator (`+`) is processed by Git tooling before we even reach this code. @@ -963,6 +983,13 @@ fn push_ref( args.extend([path.to_string(), refspec.to_string()]); + for expected in expected_refs { + args.push(format!( + "--force-with-lease=refs/namespaces/{}/{expected}", + dst.namespace() + )); + } + // Rely on the environment variable `GIT_DIR`. let working = None;