# [PATCH v3] push --force-with-lease: Fix ref status reporting 3 messages from 2016-01-28 to 2016-01-29. Participants: Andrew Wheeler, Junio C Hamano. Thread: https://gitlist.dev/t/41278 ## Andrew Wheeler, 2016-01-28 20:28 Subject: [PATCH v3] push --force-with-lease: Fix ref status reporting Message-ID: <1454012898-10138-1-git-send-email-agwheeler@gmail.com> URL: https://gitlist.dev/e/1454012898-10138-1-git-send-email-agwheeler%40gmail.com ``` From: Andrew Wheeler 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 --- remote.c | 15 ++++++++------- t/t5533-push-cas.sh | 9 ++++++--- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/remote.c b/remote.c index 9d34b5a..3ceac07 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 @@ -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 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 ``` ## Junio C Hamano, 2016-01-28 23:13 Subject: Re: [PATCH v3] push --force-with-lease: Fix ref status reporting Message-ID: URL: https://gitlist.dev/e/xmqqmvrpl2lb.fsf%40gitster.mtv.corp.google.com In-Reply-To: <1454012898-10138-1-git-send-email-agwheeler@gmail.com> ``` Andrew Wheeler writes: > 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 && These all look OK (I am not sure about message i18n, though). Do we not test a case where --force-with-lease push is rejected due to REF_STATUS_REJECT_STALE? Thanks. ``` ## Andrew Wheeler, 2016-01-29 22:53 Subject: Re: [PATCH v3] push --force-with-lease: Fix ref status reporting Message-ID: URL: https://gitlist.dev/e/CAJay-wm%3D1Bf6HUvsZd-cRVh2ehU_aZ1SuiOBfBTQ2uafVnjNoQ%40mail.gmail.com In-Reply-To: ``` > These all look OK (I am not sure about message i18n, though). > > Do we not test a case where --force-with-lease push is rejected due > to REF_STATUS_REJECT_STALE? Good idea, new patch on the way. -andrew ```