{"thread":{"id":"41277","subject":"[PATCH v2] push --force-with-lease: Fix ref status reporting","startedAt":"2016-01-28T20:20:10Z","lastAt":"2016-01-28T20:30:58Z","messageCount":2,"participants":["Andrew Wheeler"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"277000","messageId":"1454012410-10049-1-git-send-email-agwheeler@gmail.com","threadId":"41277","inReplyTo":null,"subject":"[PATCH v2] push --force-with-lease: Fix ref status reporting","fromName":"Andrew Wheeler","fromEmail":"agwheeler@gmail.com","sentAt":"2016-01-28T20:20:10Z","receivedAt":"2016-01-28T20:20:10Z","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            | 16 +++++++++-------\n t/t5533-push-cas.sh |  9 ++++++---\n 2 files changed, 15 insertions(+), 10 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 9d34b5a..bad6213 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@@ -1580,9 +1581,10 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,\n \t\t *\n \t\t * (4) it is forced using the +A:B notation, or by\n \t\t *     passing the --force argument\n+\t\t *\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":"277002","messageId":"CAE=k0msr-5euhiKep9YgE_gvPXsQfLujoXGoq3=A3LnaRRBqgA@mail.gmail.com","threadId":"41277","inReplyTo":"1454012410-10049-1-git-send-email-agwheeler@gmail.com","subject":"Re: [PATCH v2] push --force-with-lease: Fix ref status reporting","fromName":"Andrew Wheeler","fromEmail":"awheeler@motorola.com","sentAt":"2016-01-28T20:30:58Z","receivedAt":"2016-01-28T20:30:58Z","isPatch":true,"sender":{"key":"awheeler@motorola.com","avatar":"https://avatars.githubusercontent.com/u/6362528?v=4"},"body":"Ignore -- I left an extra blank line. v3 is sent.\n\nOn Thu, Jan 28, 2016 at 2:20 PM, Andrew Wheeler <agwheeler@gmail.com> wrote:\n> From: Andrew Wheeler <awheeler@motorola.com>\n>\n> The --force--with-lease push option leads to less\n> detailed status information than --force. In particular,\n> the output indicates that a reference was fast-forwarded,\n> even when it was force-updated.\n>\n> Modify the --force-with-lease ref status logic to leverage\n> the --force ref status logic when the \"lease\" conditions\n> are met.\n>\n> Also, enhance tests to validate output status reporting.\n>\n> Signed-off-by: Andrew Wheeler <awheeler@motorola.com>\n> ---\n>  remote.c            | 16 +++++++++-------\n>  t/t5533-push-cas.sh |  9 ++++++---\n>  2 files changed, 15 insertions(+), 10 deletions(-)\n>\n> diff --git a/remote.c b/remote.c\n> index 9d34b5a..bad6213 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>                 }\n>\n>                 /*\n> -                * Bypass the usual \"must fast-forward\" check but\n> -                * replace it with a weaker \"the old value must be\n> -                * this value we observed\".  If the remote ref has\n> -                * moved and is now different from what we expect,\n> -                * reject any push.\n> +                * If the remote ref has moved and is now different\n> +                * from what we expect, reject any push.\n>                  *\n>                  * It also is an error if the user told us to check\n>                  * 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>                         if (ref->expect_old_no_trackback ||\n>                             oidcmp(&ref->old_oid, &ref->old_oid_expect))\n>                                 reject_reason = REF_STATUS_REJECT_STALE;\n> +                       else\n> +                               /* If the ref isn't stale then force the update. */\n> +                               force_ref_update = 1;\n>                 }\n>\n>                 /*\n> -                * The usual \"must fast-forward\" rules.\n> +                * If the update isn't already rejected then check\n> +                * the usual \"must fast-forward\" rules.\n>                  *\n>                  * Decide whether an individual refspec A:B can be\n>                  * pushed.  The push will succeed if any of the\n> @@ -1580,9 +1581,10 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,\n>                  *\n>                  * (4) it is forced using the +A:B notation, or by\n>                  *     passing the --force argument\n> +                *\n>                  */\n>\n> -               else if (!ref->deletion && !is_null_oid(&ref->old_oid)) {\n> +               if (!reject_reason && !ref->deletion && !is_null_oid(&ref->old_oid)) {\n>                         if (starts_with(ref->name, \"refs/tags/\"))\n>                                 reject_reason = REF_STATUS_REJECT_ALREADY_EXISTS;\n>                         else if (!has_object_file(&ref->old_oid))\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>         (\n>                 cd dst &&\n>                 test_commit D &&\n> -               git push --force-with-lease=master origin master\n> +               git push --force-with-lease=master origin master 2>err &&\n> +               ! grep \"forced update\" err\n>         ) &&\n>         git ls-remote dst refs/heads/master >expect &&\n>         git ls-remote src refs/heads/master >actual &&\n> @@ -114,7 +115,8 @@ test_expect_success 'push to update (allowed even though no-ff)' '\n>                 cd dst &&\n>                 git reset --hard HEAD^ &&\n>                 test_commit D &&\n> -               git push --force-with-lease=master origin master\n> +               git push --force-with-lease=master origin master 2>err &&\n> +               grep \"forced update\" err\n>         ) &&\n>         git ls-remote dst refs/heads/master >expect &&\n>         git ls-remote src refs/heads/master >actual &&\n> @@ -147,7 +149,8 @@ test_expect_success 'push to delete (allowed)' '\n>         setup_srcdst_basic &&\n>         (\n>                 cd dst &&\n> -               git push --force-with-lease=master origin :master\n> +               git push --force-with-lease=master origin :master 2>err &&\n> +               grep deleted err\n>         ) &&\n>         >expect &&\n>         git ls-remote src refs/heads/master >actual &&\n> --\n> 1.7.11.2\n>\n"}]}