Git development
 help / color / mirror / Atom feed
* [PATCH v2] push --force-with-lease: Fix ref status reporting
@ 2016-01-28 20:20 Andrew Wheeler
  2016-01-28 20:30 ` Andrew Wheeler
  0 siblings, 1 reply; 2+ messages in thread
From: Andrew Wheeler @ 2016-01-28 20:20 UTC (permalink / raw)
  To: git; +Cc: gitster, Andrew Wheeler

From: Andrew Wheeler <awheeler@motorola.com>

The --force--with-lease push option leads to less
detailed status information than --force. In particular,
the output indicates that a reference was fast-forwarded,
even when it was force-updated.

Modify the --force-with-lease ref status logic to leverage
the --force ref status logic when the "lease" conditions
are met.

Also, enhance tests to validate output status reporting.

Signed-off-by: Andrew Wheeler <awheeler@motorola.com>
---
 remote.c            | 16 +++++++++-------
 t/t5533-push-cas.sh |  9 ++++++---
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/remote.c b/remote.c
index 9d34b5a..bad6213 100644
--- a/remote.c
+++ b/remote.c
@@ -1545,11 +1545,8 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 		}
 
 		/*
-		 * Bypass the usual "must fast-forward" check but
-		 * replace it with a weaker "the old value must be
-		 * this value we observed".  If the remote ref has
-		 * moved and is now different from what we expect,
-		 * reject any push.
+		 * If the remote ref has moved and is now different
+		 * from what we expect, reject any push.
 		 *
 		 * It also is an error if the user told us to check
 		 * with the remote-tracking branch to find the value
@@ -1560,10 +1557,14 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 			if (ref->expect_old_no_trackback ||
 			    oidcmp(&ref->old_oid, &ref->old_oid_expect))
 				reject_reason = REF_STATUS_REJECT_STALE;
+			else
+				/* If the ref isn't stale then force the update. */
+				force_ref_update = 1;
 		}
 
 		/*
-		 * The usual "must fast-forward" rules.
+		 * If the update isn't already rejected then check
+		 * the usual "must fast-forward" rules.
 		 *
 		 * Decide whether an individual refspec A:B can be
 		 * pushed.  The push will succeed if any of the
@@ -1580,9 +1581,10 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 		 *
 		 * (4) it is forced using the +A:B notation, or by
 		 *     passing the --force argument
+		 *
 		 */
 
-		else if (!ref->deletion && !is_null_oid(&ref->old_oid)) {
+		if (!reject_reason && !ref->deletion && !is_null_oid(&ref->old_oid)) {
 			if (starts_with(ref->name, "refs/tags/"))
 				reject_reason = REF_STATUS_REJECT_ALREADY_EXISTS;
 			else if (!has_object_file(&ref->old_oid))
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index c402d8d..c65033f 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -101,7 +101,8 @@ test_expect_success 'push to update (allowed, tracking)' '
 	(
 		cd dst &&
 		test_commit D &&
-		git push --force-with-lease=master origin master
+		git push --force-with-lease=master origin master 2>err &&
+		! grep "forced update" err
 	) &&
 	git ls-remote dst refs/heads/master >expect &&
 	git ls-remote src refs/heads/master >actual &&
@@ -114,7 +115,8 @@ test_expect_success 'push to update (allowed even though no-ff)' '
 		cd dst &&
 		git reset --hard HEAD^ &&
 		test_commit D &&
-		git push --force-with-lease=master origin master
+		git push --force-with-lease=master origin master 2>err &&
+		grep "forced update" err
 	) &&
 	git ls-remote dst refs/heads/master >expect &&
 	git ls-remote src refs/heads/master >actual &&
@@ -147,7 +149,8 @@ test_expect_success 'push to delete (allowed)' '
 	setup_srcdst_basic &&
 	(
 		cd dst &&
-		git push --force-with-lease=master origin :master
+		git push --force-with-lease=master origin :master 2>err &&
+		grep deleted err
 	) &&
 	>expect &&
 	git ls-remote src refs/heads/master >actual &&
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] push --force-with-lease: Fix ref status reporting
  2016-01-28 20:20 [PATCH v2] push --force-with-lease: Fix ref status reporting Andrew Wheeler
@ 2016-01-28 20:30 ` Andrew Wheeler
  0 siblings, 0 replies; 2+ messages in thread
From: Andrew Wheeler @ 2016-01-28 20:30 UTC (permalink / raw)
  To: git

Ignore -- I left an extra blank line. v3 is sent.

On Thu, Jan 28, 2016 at 2:20 PM, Andrew Wheeler <agwheeler@gmail.com> wrote:
> From: Andrew Wheeler <awheeler@motorola.com>
>
> The --force--with-lease push option leads to less
> detailed status information than --force. In particular,
> the output indicates that a reference was fast-forwarded,
> even when it was force-updated.
>
> Modify the --force-with-lease ref status logic to leverage
> the --force ref status logic when the "lease" conditions
> are met.
>
> Also, enhance tests to validate output status reporting.
>
> Signed-off-by: Andrew Wheeler <awheeler@motorola.com>
> ---
>  remote.c            | 16 +++++++++-------
>  t/t5533-push-cas.sh |  9 ++++++---
>  2 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index 9d34b5a..bad6213 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1545,11 +1545,8 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
>                 }
>
>                 /*
> -                * Bypass the usual "must fast-forward" check but
> -                * replace it with a weaker "the old value must be
> -                * this value we observed".  If the remote ref has
> -                * moved and is now different from what we expect,
> -                * reject any push.
> +                * If the remote ref has moved and is now different
> +                * from what we expect, reject any push.
>                  *
>                  * It also is an error if the user told us to check
>                  * with the remote-tracking branch to find the value
> @@ -1560,10 +1557,14 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
>                         if (ref->expect_old_no_trackback ||
>                             oidcmp(&ref->old_oid, &ref->old_oid_expect))
>                                 reject_reason = REF_STATUS_REJECT_STALE;
> +                       else
> +                               /* If the ref isn't stale then force the update. */
> +                               force_ref_update = 1;
>                 }
>
>                 /*
> -                * The usual "must fast-forward" rules.
> +                * If the update isn't already rejected then check
> +                * the usual "must fast-forward" rules.
>                  *
>                  * Decide whether an individual refspec A:B can be
>                  * pushed.  The push will succeed if any of the
> @@ -1580,9 +1581,10 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
>                  *
>                  * (4) it is forced using the +A:B notation, or by
>                  *     passing the --force argument
> +                *
>                  */
>
> -               else if (!ref->deletion && !is_null_oid(&ref->old_oid)) {
> +               if (!reject_reason && !ref->deletion && !is_null_oid(&ref->old_oid)) {
>                         if (starts_with(ref->name, "refs/tags/"))
>                                 reject_reason = REF_STATUS_REJECT_ALREADY_EXISTS;
>                         else if (!has_object_file(&ref->old_oid))
> diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
> index c402d8d..c65033f 100755
> --- a/t/t5533-push-cas.sh
> +++ b/t/t5533-push-cas.sh
> @@ -101,7 +101,8 @@ test_expect_success 'push to update (allowed, tracking)' '
>         (
>                 cd dst &&
>                 test_commit D &&
> -               git push --force-with-lease=master origin master
> +               git push --force-with-lease=master origin master 2>err &&
> +               ! grep "forced update" err
>         ) &&
>         git ls-remote dst refs/heads/master >expect &&
>         git ls-remote src refs/heads/master >actual &&
> @@ -114,7 +115,8 @@ test_expect_success 'push to update (allowed even though no-ff)' '
>                 cd dst &&
>                 git reset --hard HEAD^ &&
>                 test_commit D &&
> -               git push --force-with-lease=master origin master
> +               git push --force-with-lease=master origin master 2>err &&
> +               grep "forced update" err
>         ) &&
>         git ls-remote dst refs/heads/master >expect &&
>         git ls-remote src refs/heads/master >actual &&
> @@ -147,7 +149,8 @@ test_expect_success 'push to delete (allowed)' '
>         setup_srcdst_basic &&
>         (
>                 cd dst &&
> -               git push --force-with-lease=master origin :master
> +               git push --force-with-lease=master origin :master 2>err &&
> +               grep deleted err
>         ) &&
>         >expect &&
>         git ls-remote src refs/heads/master >actual &&
> --
> 1.7.11.2
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-01-28 20:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-28 20:20 [PATCH v2] push --force-with-lease: Fix ref status reporting Andrew Wheeler
2016-01-28 20:30 ` Andrew Wheeler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox