{"thread":{"id":"41288","subject":"[PATCH v4] push --force-with-lease: Fix ref status reporting","startedAt":"2016-01-29T23:18:42Z","lastAt":"2016-01-29T23:18:42Z","messageCount":1,"participants":["Andrew Wheeler"],"isPatch":true,"patchVersion":4,"patchTotal":null},"messages":[{"id":"277069","messageId":"1454109522-11186-1-git-send-email-agwheeler@gmail.com","threadId":"41288","inReplyTo":null,"subject":"[PATCH v4] push --force-with-lease: Fix ref status reporting","fromName":"Andrew Wheeler","fromEmail":"agwheeler@gmail.com","sentAt":"2016-01-29T23:18:42Z","receivedAt":"2016-01-29T23:18:42Z","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 | 15 ++++++++++-----\n 2 files changed, 18 insertions(+), 12 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..c732012 100755\n--- a/t/t5533-push-cas.sh\n+++ b/t/t5533-push-cas.sh\n@@ -25,7 +25,8 @@ test_expect_success 'push to update (protected)' '\n \t(\n \t\tcd dst &&\n \t\ttest_commit D &&\n-\t\ttest_must_fail git push --force-with-lease=master:master origin master\n+\t\ttest_must_fail git push --force-with-lease=master:master origin master 2>err &&\n+\t\tgrep \"stale info\" err\n \t) &&\n \tgit ls-remote . refs/heads/master >expect &&\n \tgit ls-remote src refs/heads/master >actual &&\n@@ -37,7 +38,8 @@ test_expect_success 'push to update (protected, forced)' '\n \t(\n \t\tcd dst &&\n \t\ttest_commit D &&\n-\t\tgit push --force --force-with-lease=master:master origin master\n+\t\tgit push --force --force-with-lease=master: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@@ -101,7 +103,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 +117,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 +151,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"}]}