push: fix ref status reporting for --force-with-lease

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/remote.c b/remote.c
index 9d34b5a5dafa217e2ce9cb1b7e9a4ab74908f7af..3ceac07620cef06810c8f92adec735cc5baef832 100644 (file)
--- 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
@@ -1582,7 +1583,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
                 *     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 c402d8d3d7c7e053373ffd98491ae0160481f3ba..c7320121ecfa74dce8389d3790364501c8bb12b8 100755 (executable)
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -25,7 +25,8 @@ test_expect_success 'push to update (protected)' '
        (
                cd dst &&
                test_commit D &&
-               test_must_fail git push --force-with-lease=master:master origin master
+               test_must_fail git push --force-with-lease=master:master origin master 2>err &&
+               grep "stale info" err
        ) &&
        git ls-remote . refs/heads/master >expect &&
        git ls-remote src refs/heads/master >actual &&
@@ -37,7 +38,8 @@ test_expect_success 'push to update (protected, forced)' '
        (
                cd dst &&
                test_commit D &&
-               git push --force --force-with-lease=master:master origin master
+               git push --force --force-with-lease=master: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 &&
@@ -101,7 +103,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 +117,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 +151,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 &&