remote-helper: Support push --force-with-lease
Previously, the remote helper would not support the `--force-with-lease` option. This change introduces this support and ensures that it works as intended. Co-authored-by: Fintan Halpenny <fintan.halpenny@gmail.com>
This commit is contained in:
parent
6d0c571ea9
commit
0ec084fc23
|
|
@ -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
|
||||||
|
|
@ -9,7 +9,7 @@ Switched to a new branch 'feature/1'
|
||||||
$ git commit -a -m "Add things" -q --allow-empty
|
$ 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
|
$ git push -o patch.message="Add things #1" -o patch.message="See commits for details." rad HEAD:refs/patches
|
||||||
✓ Patch 6035d2f582afbe01ff23ea87528ae523d76875b6 opened
|
✓ 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: offline push, your node is not running
|
||||||
hint: to sync with the network, run `rad node start`
|
hint: to sync with the network, run `rad node start`
|
||||||
To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi
|
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
|
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)
|
``` (stderr)
|
||||||
$ git push --force
|
$ git push --force-with-lease
|
||||||
✓ Patch 9580891 updated to revision 670d02794aa05afd6e0851f4aa848bc87c4712c7
|
✓ Patch 9580891 updated to revision 670d02794aa05afd6e0851f4aa848bc87c4712c7
|
||||||
To compare against your previous revision d7040c6, run:
|
To compare against your previous revision d7040c6, run:
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -2425,6 +2425,11 @@ fn git_push_amend() {
|
||||||
.unwrap();
|
.unwrap();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn git_push_force_with_lease() {
|
||||||
|
Environment::alice(["rad-init", "git/git-push-force-with-lease"]);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn git_push_rollback() {
|
fn git_push_rollback() {
|
||||||
let mut environment = Environment::new();
|
let mut environment = Environment::new();
|
||||||
|
|
|
||||||
|
|
@ -240,6 +240,7 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> {
|
||||||
let stdin = io::stdin();
|
let stdin = io::stdin();
|
||||||
let mut line = String::new();
|
let mut line = String::new();
|
||||||
let mut opts = Options::default();
|
let mut opts = Options::default();
|
||||||
|
let mut expected_refs = Vec::new();
|
||||||
|
|
||||||
if let Err(e) = radicle::io::set_file_limit(4096) {
|
if let Err(e) = radicle::io::set_file_limit(4096) {
|
||||||
if debug {
|
if debug {
|
||||||
|
|
@ -278,6 +279,10 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> {
|
||||||
push_option(args, &mut opts)?;
|
push_option(args, &mut opts)?;
|
||||||
println!("ok");
|
println!("ok");
|
||||||
}
|
}
|
||||||
|
["option", "cas", refstr] => {
|
||||||
|
expected_refs.push((*refstr).to_owned());
|
||||||
|
println!("ok");
|
||||||
|
}
|
||||||
["option", "progress", ..] | ["option", ..] => {
|
["option", "progress", ..] | ["option", ..] => {
|
||||||
println!("unsupported");
|
println!("unsupported");
|
||||||
}
|
}
|
||||||
|
|
@ -301,6 +306,7 @@ pub fn run(profile: radicle::Profile) -> Result<(), Error> {
|
||||||
&profile,
|
&profile,
|
||||||
&stdin,
|
&stdin,
|
||||||
opts,
|
opts,
|
||||||
|
&expected_refs,
|
||||||
)?);
|
)?);
|
||||||
}
|
}
|
||||||
["list"] => {
|
["list"] => {
|
||||||
|
|
|
||||||
|
|
@ -251,6 +251,7 @@ pub fn run(
|
||||||
profile: &Profile,
|
profile: &Profile,
|
||||||
stdin: &io::Stdin,
|
stdin: &io::Stdin,
|
||||||
opts: Options,
|
opts: Options,
|
||||||
|
expected_refs: &[String],
|
||||||
) -> Result<(), Error> {
|
) -> Result<(), Error> {
|
||||||
// Don't allow push if either of these conditions is true:
|
// Don't allow push if either of these conditions is true:
|
||||||
//
|
//
|
||||||
|
|
@ -341,6 +342,7 @@ pub fn run(
|
||||||
patches,
|
patches,
|
||||||
&signer,
|
&signer,
|
||||||
opts.clone(),
|
opts.clone(),
|
||||||
|
expected_refs,
|
||||||
),
|
),
|
||||||
PushAction::PushRef { dst } => {
|
PushAction::PushRef { dst } => {
|
||||||
let identity = stored.identity()?;
|
let identity = stored.identity()?;
|
||||||
|
|
@ -361,6 +363,7 @@ pub fn run(
|
||||||
patches,
|
patches,
|
||||||
&signer,
|
&signer,
|
||||||
opts.verbosity,
|
opts.verbosity,
|
||||||
|
expected_refs,
|
||||||
)?;
|
)?;
|
||||||
// If we're trying to update the canonical head, make sure
|
// If we're trying to update the canonical head, make sure
|
||||||
// we don't diverge from the current head. This only applies
|
// 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> {
|
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
|
// rad/patches/deadbeef -> patches/deadbeef
|
||||||
let name = name.split_once('/').unwrap_or_default().1;
|
let name = name.split_once('/').unwrap_or_default().1;
|
||||||
hint(format!(
|
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<G>(
|
||||||
>,
|
>,
|
||||||
signer: &Device<G>,
|
signer: &Device<G>,
|
||||||
opts: Options,
|
opts: Options,
|
||||||
|
expected_refs: &[String],
|
||||||
) -> Result<Option<ExplorerResource>, Error>
|
) -> Result<Option<ExplorerResource>, Error>
|
||||||
where
|
where
|
||||||
G: crypto::signature::Signer<crypto::Signature>,
|
G: crypto::signature::Signer<crypto::Signature>,
|
||||||
|
|
@ -709,7 +720,14 @@ where
|
||||||
term::patch::get_update_message(opts.message, &stored.backend, &latest, &head.into())?;
|
term::patch::get_update_message(opts.message, &stored.backend, &latest, &head.into())?;
|
||||||
|
|
||||||
let dst = dst.with_namespace(nid.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 mut patch_mut = patch::PatchMut::new(patch_id, patch, &mut patches);
|
||||||
let revision = patch_mut.update(message, base, *head, signer)?;
|
let revision = patch_mut.update(message, base, *head, signer)?;
|
||||||
|
|
@ -758,6 +776,7 @@ fn push<G>(
|
||||||
>,
|
>,
|
||||||
signer: &Device<G>,
|
signer: &Device<G>,
|
||||||
verbosity: Verbosity,
|
verbosity: Verbosity,
|
||||||
|
expected_refs: &[String],
|
||||||
) -> Result<Option<ExplorerResource>, Error>
|
) -> Result<Option<ExplorerResource>, Error>
|
||||||
where
|
where
|
||||||
G: crypto::signature::Signer<crypto::Signature>,
|
G: crypto::signature::Signer<crypto::Signature>,
|
||||||
|
|
@ -767,7 +786,7 @@ where
|
||||||
// It's ok for the destination reference to be unknown, eg. when pushing a new branch.
|
// 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();
|
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 {
|
if let Some(old) = old {
|
||||||
let proj = stored.project()?;
|
let proj = stored.project()?;
|
||||||
|
|
@ -950,6 +969,7 @@ fn push_ref(
|
||||||
force: bool,
|
force: bool,
|
||||||
stored: &git::raw::Repository,
|
stored: &git::raw::Repository,
|
||||||
verbosity: Verbosity,
|
verbosity: Verbosity,
|
||||||
|
expected_refs: &[String],
|
||||||
) -> Result<(), Error> {
|
) -> Result<(), Error> {
|
||||||
let path = dunce::canonicalize(stored.path())?.display().to_string();
|
let path = dunce::canonicalize(stored.path())?.display().to_string();
|
||||||
// Nb. The *force* indicator (`+`) is processed by Git tooling before we even reach this code.
|
// 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()]);
|
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`.
|
// Rely on the environment variable `GIT_DIR`.
|
||||||
let working = None;
|
let working = None;
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue