{"thread":{"id":"41278","subject":"[PATCH v3] push --force-with-lease: Fix ref status reporting","startedAt":"2016-01-28T20:28:18Z","lastAt":"2016-01-29T22:53:46Z","messageCount":3,"participants":["Andrew Wheeler","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"277001","messageId":"1454012898-10138-1-git-send-email-agwheeler@gmail.com","threadId":"41278","inReplyTo":null,"subject":"[PATCH v3] push --force-with-lease: Fix ref status reporting","fromName":"Andrew Wheeler","fromEmail":"agwheeler@gmail.com","sentAt":"2016-01-28T20:28:18Z","receivedAt":"2016-01-28T20:28:18Z","isPatch":true,"sender":{"key":"agwheeler@gmail.com","avatar":null},"body":"From: Andrew Wheeler <awheeler@motorola.com>\n\nThe --force--with-lease push option leads to less\ndetailed status information than --force. In particular,\nthe output indicates that a reference was fast-forwarded,\neven when it was force-updated.\n\nModify the --force-with-lease ref status logic to leverage\nthe --force ref status logic when the \"lease\" conditions\nare met.\n\nAlso, enhance tests to validate output status reporting.\n\nSigned-off-by: Andrew Wheeler <awheeler@motorola.com>\n---\n remote.c            | 15 ++++++++-------\n t/t5533-push-cas.sh |  9 ++++++---\n 2 files changed, 14 insertions(+), 10 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 9d34b5a..3ceac07 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1545,11 +1545,8 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,\n \t\t}\n \n \t\t/*\n-\t\t * Bypass the usual \"must fast-forward\" check but\n-\t\t * replace it with a weaker \"the old value must be\n-\t\t * this value we observed\".  If the remote ref has\n-\t\t * moved and is now different from what we expect,\n-\t\t * reject any push.\n+\t\t * If the remote ref has moved and is now different\n+\t\t * from what we expect, reject any push.\n \t\t *\n \t\t * It also is an error if the user told us to check\n \t\t * with the remote-tracking branch to find the value\n@@ -1560,10 +1557,14 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,\n \t\t\tif (ref->expect_old_no_trackback ||\n \t\t\t    oidcmp(&ref->old_oid, &ref->old_oid_expect))\n \t\t\t\treject_reason = REF_STATUS_REJECT_STALE;\n+\t\t\telse\n+\t\t\t\t/* If the ref isn't stale then force the update. */\n+\t\t\t\tforce_ref_update = 1;\n \t\t}\n \n \t\t/*\n-\t\t * The usual \"must fast-forward\" rules.\n+\t\t * If the update isn't already rejected then check\n+\t\t * the usual \"must fast-forward\" rules.\n \t\t *\n \t\t * Decide whether an individual refspec A:B can be\n \t\t * pushed.  The push will succeed if any of the\n@@ -1582,7 +1583,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,\n \t\t *     passing the --force argument\n \t\t */\n \n-\t\telse if (!ref->deletion && !is_null_oid(&ref->old_oid)) {\n+\t\tif (!reject_reason && !ref->deletion && !is_null_oid(&ref->old_oid)) {\n \t\t\tif (starts_with(ref->name, \"refs/tags/\"))\n \t\t\t\treject_reason = REF_STATUS_REJECT_ALREADY_EXISTS;\n \t\t\telse if (!has_object_file(&ref->old_oid))\ndiff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh\nindex c402d8d..c65033f 100755\n--- a/t/t5533-push-cas.sh\n+++ b/t/t5533-push-cas.sh\n@@ -101,7 +101,8 @@ test_expect_success 'push to update (allowed, tracking)' '\n \t(\n \t\tcd dst &&\n \t\ttest_commit D &&\n-\t\tgit push --force-with-lease=master origin master\n+\t\tgit push --force-with-lease=master origin master 2>err &&\n+\t\t! grep \"forced update\" err\n \t) &&\n \tgit ls-remote dst refs/heads/master >expect &&\n \tgit ls-remote src refs/heads/master >actual &&\n@@ -114,7 +115,8 @@ test_expect_success 'push to update (allowed even though no-ff)' '\n \t\tcd dst &&\n \t\tgit reset --hard HEAD^ &&\n \t\ttest_commit D &&\n-\t\tgit push --force-with-lease=master origin master\n+\t\tgit push --force-with-lease=master origin master 2>err &&\n+\t\tgrep \"forced update\" err\n \t) &&\n \tgit ls-remote dst refs/heads/master >expect &&\n \tgit ls-remote src refs/heads/master >actual &&\n@@ -147,7 +149,8 @@ test_expect_success 'push to delete (allowed)' '\n \tsetup_srcdst_basic &&\n \t(\n \t\tcd dst &&\n-\t\tgit push --force-with-lease=master origin :master\n+\t\tgit push --force-with-lease=master origin :master 2>err &&\n+\t\tgrep deleted err\n \t) &&\n \t>expect &&\n \tgit ls-remote src refs/heads/master >actual &&\n-- \n1.7.11.2\n"},{"id":"277019","messageId":"xmqqmvrpl2lb.fsf@gitster.mtv.corp.google.com","threadId":"41278","inReplyTo":"1454012898-10138-1-git-send-email-agwheeler@gmail.com","subject":"Re: [PATCH v3] push --force-with-lease: Fix ref status reporting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-28T23:13:36Z","receivedAt":"2016-01-28T23:13:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Wheeler <agwheeler@gmail.com> writes:\n\n> diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh\n> index c402d8d..c65033f 100755\n> --- a/t/t5533-push-cas.sh\n> +++ b/t/t5533-push-cas.sh\n> @@ -101,7 +101,8 @@ test_expect_success 'push to update (allowed, tracking)' '\n>  \t(\n>  \t\tcd dst &&\n>  \t\ttest_commit D &&\n> -\t\tgit push --force-with-lease=master origin master\n> +\t\tgit push --force-with-lease=master origin master 2>err &&\n> +\t\t! grep \"forced update\" err\n>  \t) &&\n>  \tgit ls-remote dst refs/heads/master >expect &&\n>  \tgit ls-remote src refs/heads/master >actual &&\n> @@ -114,7 +115,8 @@ test_expect_success 'push to update (allowed even though no-ff)' '\n>  \t\tcd dst &&\n>  \t\tgit reset --hard HEAD^ &&\n>  \t\ttest_commit D &&\n> -\t\tgit push --force-with-lease=master origin master\n> +\t\tgit push --force-with-lease=master origin master 2>err &&\n> +\t\tgrep \"forced update\" err\n>  \t) &&\n>  \tgit ls-remote dst refs/heads/master >expect &&\n>  \tgit ls-remote src refs/heads/master >actual &&\n> @@ -147,7 +149,8 @@ test_expect_success 'push to delete (allowed)' '\n>  \tsetup_srcdst_basic &&\n>  \t(\n>  \t\tcd dst &&\n> -\t\tgit push --force-with-lease=master origin :master\n> +\t\tgit push --force-with-lease=master origin :master 2>err &&\n> +\t\tgrep deleted err\n>  \t) &&\n>  \t>expect &&\n>  \tgit ls-remote src refs/heads/master >actual &&\n\nThese all look OK (I am not sure about message i18n, though).\n\nDo we not test a case where --force-with-lease push is rejected due\nto REF_STATUS_REJECT_STALE?\n\nThanks.\n"},{"id":"277067","messageId":"CAJay-wm=1Bf6HUvsZd-cRVh2ehU_aZ1SuiOBfBTQ2uafVnjNoQ@mail.gmail.com","threadId":"41278","inReplyTo":"xmqqmvrpl2lb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3] push --force-with-lease: Fix ref status reporting","fromName":"Andrew Wheeler","fromEmail":"andrew@igmo.org","sentAt":"2016-01-29T22:53:46Z","receivedAt":"2016-01-29T22:53:46Z","isPatch":true,"sender":{"key":"andrew@igmo.org","avatar":null},"body":"> These all look OK (I am not sure about message i18n, though).\n>\n> Do we not test a case where --force-with-lease push is rejected due\n> to REF_STATUS_REJECT_STALE?\n\nGood idea, new patch on the way.\n\n\n-andrew\n"}]}