{"thread":{"id":"10839","subject":"[PATCH] Add a test checking if send-pack updated local tracking branches correctly","startedAt":"2007-11-12T21:38:23Z","lastAt":"2007-11-15T20:46:11Z","messageCount":22,"participants":["Alex Riesen","Jeff King","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"59578","messageId":"20071112213823.GB2918@steel.home","threadId":"10839","inReplyTo":null,"subject":"[PATCH] Add a test checking if send-pack updated local tracking branches correctly","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-12T21:38:23Z","receivedAt":"2007-11-12T21:38:23Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Signed-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n t/t5404-tracking-branches.sh |   40 ++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 40 insertions(+), 0 deletions(-)\n create mode 100755 t/t5404-tracking-branches.sh\n\ndiff --git a/t/t5404-tracking-branches.sh b/t/t5404-tracking-branches.sh\nnew file mode 100755\nindex 0000000..20718d4\n--- /dev/null\n+++ b/t/t5404-tracking-branches.sh\n@@ -0,0 +1,40 @@\n+#!/bin/sh\n+\n+test_description='tracking branch update checks for git push'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\techo 1 >file &&\n+\tgit add file &&\n+\tgit commit -m 1 &&\n+\tgit branch b1 &&\n+\tgit branch b2 &&\n+\tgit clone . aa &&\n+\tgit checkout b1 &&\n+\techo b1 >>file &&\n+\tgit commit -a -m b1 &&\n+\tgit checkout b2 &&\n+\techo b2 >>file &&\n+\tgit commit -a -m b2\n+'\n+\n+test_expect_success 'check tracking branches updated correctly after push' '\n+\tcd aa &&\n+\tb1=$(git rev-parse origin/b1) &&\n+\tb2=$(git rev-parse origin/b2) &&\n+\tgit checkout -b b1 origin/b1 &&\n+\techo aa-b1 >>file &&\n+\tgit commit -a -m aa-b1 &&\n+\tgit checkout -b b2 origin/b2 &&\n+\techo aa-b2 >>file &&\n+\tgit commit -a -m aa-b2 &&\n+\tgit checkout master &&\n+\techo aa-master >>file &&\n+\tgit commit -a -m aa-master &&\n+\tgit push &&\n+\ttest \"$(git rev-parse origin/b1)\" = \"$b1\" &&\n+\ttest \"$(git rev-parse origin/b2)\" = \"$b2\"\n+'\n+\n+test_done\n-- \n1.5.3.5.648.g1e92c\n"},{"id":"59579","messageId":"20071112213938.GC2918@steel.home","threadId":"10839","inReplyTo":"20071112213823.GB2918@steel.home","subject":"[PATCH] Update the tracking references only if they were succesfully updated on remote","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-12T21:39:38Z","receivedAt":"2007-11-12T21:39:38Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"It fixes the bug where local tracing branches were filled with zeroed SHA-1\nif the remote branch was not updated because, for instance, it was not\nan ancestor of the local (i.e. had other changes).\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nJeff, I think your change (334f4831e5a77) was either not complete or\ngot broken some time later.\n\n send-pack.c |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/send-pack.c b/send-pack.c\nindex b74fd45..d56d980 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -349,7 +349,8 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha\n \n \tif (!dry_run && remote && ret == 0) {\n \t\tfor (ref = remote_refs; ref; ref = ref->next)\n-\t\t\tupdate_tracking_ref(remote, ref);\n+\t\t\tif (!is_null_sha1(ref->new_sha1))\n+\t\t\t\tupdate_tracking_ref(remote, ref);\n \t}\n \n \tif (!new_refs && ret == 0)\n-- \n1.5.3.5.648.g1e92c\n"},{"id":"59626","messageId":"20071113075240.GA21799@sigill.intra.peff.net","threadId":"10839","inReplyTo":"20071112213938.GC2918@steel.home","subject":"Re: [PATCH] Update the tracking references only if they were succesfully updated on remote","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-13T07:52:40Z","receivedAt":"2007-11-13T07:52:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 12, 2007 at 10:39:38PM +0100, Alex Riesen wrote:\n\n> It fixes the bug where local tracing branches were filled with zeroed SHA-1\n> if the remote branch was not updated because, for instance, it was not\n> an ancestor of the local (i.e. had other changes).\n> \n> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>\n> ---\n> \n> Jeff, I think your change (334f4831e5a77) was either not complete or\n> got broken some time later.\n\nYes, some of the error information placed in 'ret' was getting lost.\nDaniel and I discussed some fixes, but haven't done a final patch yet.\nYour patch doesn't work because the assumption that\nis_null_sha1(refs->new_sha1) signals error is not correct. This is also\nthe case for deleted refs, which means that we are failing to update\ntracking branches for successfully deleted refs.\n\nI'd like to have a patch that accurately tracks per-ref errors,\nincluding ones from the remote. That not only will give us more accurate\nstatus reporting, but fixes like this will be much easier. Let me see if\nI can put something together.\n\n-Peff\n"},{"id":"59703","messageId":"20071113194731.GC3268@steel.home","threadId":"10839","inReplyTo":"20071113075240.GA21799@sigill.intra.peff.net","subject":"Re: [PATCH] Update the tracking references only if they were succesfully updated on remote","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-13T19:47:31Z","receivedAt":"2007-11-13T19:47:31Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Jeff King, Tue, Nov 13, 2007 08:52:40 +0100:\n> On Mon, Nov 12, 2007 at 10:39:38PM +0100, Alex Riesen wrote:\n> \n> > It fixes the bug where local tracing branches were filled with zeroed SHA-1\n> > if the remote branch was not updated because, for instance, it was not\n> > an ancestor of the local (i.e. had other changes).\n> > \n> > Signed-off-by: Alex Riesen <raa.lkml@gmail.com>\n> > ---\n> > \n> > Jeff, I think your change (334f4831e5a77) was either not complete or\n> > got broken some time later.\n> \n> Yes, some of the error information placed in 'ret' was getting lost.\n> Daniel and I discussed some fixes, but haven't done a final patch yet.\n> Your patch doesn't work because the assumption that\n> is_null_sha1(refs->new_sha1) signals error is not correct. This is also\n> the case for deleted refs, which means that we are failing to update\n> tracking branches for successfully deleted refs.\n\nYep. I extended the test with this case, checking for deletion of the\nremote reference and its tracking reference. Will send it separetely.\nThe mainline fails the second test, my patch the third (first being\nthe test setup).\n\n> I'd like to have a patch that accurately tracks per-ref errors,\n> including ones from the remote. That not only will give us more accurate\n> status reporting, but fixes like this will be much easier. Let me see if\n> I can put something together.\n\nI actually started almost like that: by placing an error bit in struct\nref.  But than it looked like new_sha1 was just not updated for not\nsent references, and the deleted ref are checked if their\npeer_ref->new_sha1 is null_sha1.\n\nDate: Mon, 12 Nov 2007 17:09:24 +0100\nSubject: [PATCH] Do not update the tracking references if they could not be updated on remote\n\n---\n cache.h     |    5 +++--\n send-pack.c |   20 +++++++++++++-------\n 2 files changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex f0a25c7..081b697 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -493,8 +493,9 @@ struct ref {\n \tstruct ref *next;\n \tunsigned char old_sha1[20];\n \tunsigned char new_sha1[20];\n-\tunsigned char force;\n-\tunsigned char merge;\n+\tunsigned char force:1;\n+\tunsigned char merge:1;\n+\tunsigned char failed:1;\n \tstruct ref *peer_ref; /* when renaming */\n \tchar name[FLEX_ARRAY]; /* more */\n };\ndiff --git a/send-pack.c b/send-pack.c\nindex b74fd45..c7a4c0e 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -196,8 +196,8 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref)\n \t\treturn;\n \n \tif (!remote_find_tracking(remote, &rs)) {\n-\t\tfprintf(stderr, \"updating local tracking ref '%s'\\n\", rs.dst);\n-\t\tif (is_null_sha1(ref->peer_ref->new_sha1)) {\n+\t\tfprintf(stderr, \"updating local tracking ref '%s' with %s\\n\", rs.dst, sha1_to_hex(ref->new_sha1));\n+\t\tif (is_null_sha1(ref->new_sha1)) {\n \t\t\tif (delete_ref(rs.dst, NULL))\n \t\t\t\terror(\"Failed to delete\");\n \t\t} else\n@@ -211,6 +211,7 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha\n {\n \tstruct ref *ref;\n \tint new_refs;\n+#define REF_NOT_UPTODATE (-3)\n \tint ret = 0;\n \tint ask_for_status_report = 0;\n \tint allow_deleting_refs = 0;\n@@ -246,6 +247,7 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha\n \tfor (ref = remote_refs; ref; ref = ref->next) {\n \t\tchar old_hex[60], *new_hex;\n \t\tint will_delete_ref;\n+\t\tref->failed = 0;\n \n \t\tif (!ref->peer_ref)\n \t\t\tcontinue;\n@@ -253,6 +255,7 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha\n \n \t\twill_delete_ref = is_null_sha1(ref->peer_ref->new_sha1);\n \t\tif (will_delete_ref && !allow_deleting_refs) {\n+\t\t\tref->failed = 1;\n \t\t\terror(\"remote does not support deleting refs\");\n \t\t\tret = -2;\n \t\t\tcontinue;\n@@ -297,13 +300,12 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha\n \t\t\t\t * commits at the remote end and likely\n \t\t\t\t * we were not up to date to begin with.\n \t\t\t\t */\n+\t\t\t\tref->failed = 1;\n \t\t\t\terror(\"remote '%s' is not an ancestor of\\n\"\n-\t\t\t\t      \" local  '%s'.\\n\"\n-\t\t\t\t      \" Maybe you are not up-to-date and \"\n-\t\t\t\t      \"need to pull first?\",\n+\t\t\t\t      \" local  '%s'\",\n \t\t\t\t      ref->name,\n \t\t\t\t      ref->peer_ref->name);\n-\t\t\t\tret = -2;\n+\t\t\t\tret = REF_NOT_UPTODATE;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t}\n@@ -337,6 +339,9 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha\n \t\t}\n \t}\n \n+\tif (ret == REF_NOT_UPTODATE)\n+\t\tfprintf(stderr, \"\\nMaybe you are not up-to-date and need to pull first?\\n\\n\");\n+\n \tpacket_flush(out);\n \tif (new_refs && !dry_run)\n \t\tret = pack_objects(out, remote_refs);\n@@ -349,7 +354,8 @@ static int send_pack(int in, int out, struct remote *remote, int nr_refspec, cha\n \n \tif (!dry_run && remote && ret == 0) {\n \t\tfor (ref = remote_refs; ref; ref = ref->next)\n-\t\t\tupdate_tracking_ref(remote, ref);\n+\t\t\tif (!ref->failed)\n+\t\t\t\tupdate_tracking_ref(remote, ref);\n \t}\n \n \tif (!new_refs && ret == 0)\n-- \n1.5.3.5.668.g22088\n"},{"id":"59704","messageId":"20071113194909.GD3268@steel.home","threadId":"10839","inReplyTo":"20071113194731.GC3268@steel.home","subject":"[PATCH] Add a test for deleting remote branches","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-13T19:49:09Z","receivedAt":"2007-11-13T19:49:09Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Check if the tracking and the remote branches are both actually deleted.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nAs promised.\n\n t/t5404-tracking-branches.sh |   16 ++++++++++++++++\n 1 files changed, 16 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5404-tracking-branches.sh b/t/t5404-tracking-branches.sh\nindex 20718d4..f55143a 100755\n--- a/t/t5404-tracking-branches.sh\n+++ b/t/t5404-tracking-branches.sh\n@@ -10,6 +10,7 @@ test_expect_success 'setup' '\n \tgit commit -m 1 &&\n \tgit branch b1 &&\n \tgit branch b2 &&\n+\tgit branch b3 &&\n \tgit clone . aa &&\n \tgit checkout b1 &&\n \techo b1 >>file &&\n@@ -19,6 +20,8 @@ test_expect_success 'setup' '\n \tgit commit -a -m b2\n '\n \n+start_dir=\"$(pwd)\"\n+\n test_expect_success 'check tracking branches updated correctly after push' '\n \tcd aa &&\n \tb1=$(git rev-parse origin/b1) &&\n@@ -37,4 +40,17 @@ test_expect_success 'check tracking branches updated correctly after push' '\n \ttest \"$(git rev-parse origin/b2)\" = \"$b2\"\n '\n \n+test_expect_success 'delete remote branch' '\n+\tgit push origin :refs/heads/b3 &&\n+\t{\n+\t\tgit rev-parse origin/b3 --\n+\t\ttest $? != 0\n+        } &&\n+\tcd \"$start_dir\" &&\n+\t{\n+\t\tgit rev-parse refs/heads/b3 --\n+\t\ttest $? != 0\n+        }\n+'\n+\n test_done\n-- \n1.5.3.5.668.g22088\n"},{"id":"59765","messageId":"20071113230234.GI3268@steel.home","threadId":"10839","inReplyTo":"20071113194909.GD3268@steel.home","subject":"[PATCH] Improved and extended t5404","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-13T23:02:34Z","receivedAt":"2007-11-13T23:02:34Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Ignore exit code of git push in t5404, as it is not relevant for the\ntest: it already checks whether the references updated correctly.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nThis one is on top of what is in next. It also include the check for\ndeleting remote braches I sent before. Regarding this one: if a remote\nbranch is deleted, shouldn't the matching tracking branch be removed\nas well? The code in master seem to do that.\n\n t/t5404-tracking-branches.sh |   29 ++++++++++++++++++++++++++---\n 1 files changed, 26 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t5404-tracking-branches.sh b/t/t5404-tracking-branches.sh\nindex 20718d4..a51bbdc 100755\n--- a/t/t5404-tracking-branches.sh\n+++ b/t/t5404-tracking-branches.sh\n@@ -10,6 +10,7 @@ test_expect_success 'setup' '\n \tgit commit -m 1 &&\n \tgit branch b1 &&\n \tgit branch b2 &&\n+\tgit branch b3 &&\n \tgit clone . aa &&\n \tgit checkout b1 &&\n \techo b1 >>file &&\n@@ -19,10 +20,13 @@ test_expect_success 'setup' '\n \tgit commit -a -m b2\n '\n \n+start_dir=\"$(pwd)\"\n+\n test_expect_success 'check tracking branches updated correctly after push' '\n \tcd aa &&\n \tb1=$(git rev-parse origin/b1) &&\n \tb2=$(git rev-parse origin/b2) &&\n+\tb3=$(git rev-parse origin/b3) &&\n \tgit checkout -b b1 origin/b1 &&\n \techo aa-b1 >>file &&\n \tgit commit -a -m aa-b1 &&\n@@ -32,9 +36,28 @@ test_expect_success 'check tracking branches updated correctly after push' '\n \tgit checkout master &&\n \techo aa-master >>file &&\n \tgit commit -a -m aa-master &&\n-\tgit push &&\n-\ttest \"$(git rev-parse origin/b1)\" = \"$b1\" &&\n-\ttest \"$(git rev-parse origin/b2)\" = \"$b2\"\n+\t{\n+\t\tgit push\n+\t\ttest \"$(git rev-parse origin/b1)\" = \"$b1\" &&\n+\t\ttest \"$(git rev-parse origin/b2)\" = \"$b2\" &&\n+\t\ttest \"$(git rev-parse origin/b3)\" = \"$b3\" &&\n+\t\ttest \"$(git rev-parse origin/master)\" = \\\n+\t\t\"$(git rev-parse master)\"\n+\t}\n+'\n+\n+test_expect_success 'delete remote branch' '\n+\tgit push origin :refs/heads/b3\n+\t{\n+\t\tgit rev-parse origin/b3\n+\t\ttest $? != 0 || \\\n+\t\tsay \"Hmm... Maybe tracking ref should be deleted?\"\n+        } &&\n+\tcd \"$start_dir\" &&\n+\t{\n+\t\tgit rev-parse refs/heads/b3\n+\t\ttest $? != 0\n+        }\n '\n \n test_done\n"},{"id":"59769","messageId":"20071113231048.GB19444@sigill.intra.peff.net","threadId":"10839","inReplyTo":"20071113230234.GI3268@steel.home","subject":"Re: [PATCH] Improved and extended t5404","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-13T23:10:48Z","receivedAt":"2007-11-13T23:10:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 14, 2007 at 12:02:34AM +0100, Alex Riesen wrote:\n\n> This one is on top of what is in next. It also include the check for\n> deleting remote braches I sent before. Regarding this one: if a remote\n> branch is deleted, shouldn't the matching tracking branch be removed\n> as well? The code in master seem to do that.\n\nYes, it should (the code in update_tracking_ref seems to handle that\ncase, but I haven't tested, so I may have bungled something). I am\nliterally walking out the door, now, though, so I will be out of touch\nfor at least a day.\n\n-Peff\n"},{"id":"59782","messageId":"7vmythr8xf.fsf@gitster.siamese.dyndns.org","threadId":"10839","inReplyTo":"20071113230234.GI3268@steel.home","subject":"Re: [PATCH] Improved and extended t5404","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-14T00:02:20Z","receivedAt":"2007-11-14T00:02:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen <raa.lkml@gmail.com> writes:\n\n> Ignore exit code of git push in t5404, as it is not relevant for the\n> test\n\nThis proposed log message solicits a \"Huh? -- Since when\nignoring exit code is an improvement?\" reaction.  If this push\nis expected to error out, then wouldn't you want to make sure it\nerrors out as expected?  If the problem is that the exit status\nis unreliable, maybe we need to make it reliable instead?\n"},{"id":"59818","messageId":"20071114071929.GA2942@steel.home","threadId":"10839","inReplyTo":"7vmythr8xf.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Improved and extended t5404","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-14T07:19:29Z","receivedAt":"2007-11-14T07:19:29Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Wed, Nov 14, 2007 01:02:20 +0100:\n> Alex Riesen <raa.lkml@gmail.com> writes:\n> \n> > Ignore exit code of git push in t5404, as it is not relevant for the\n> > test\n> \n> This proposed log message solicits a \"Huh? -- Since when\n> ignoring exit code is an improvement?\" reaction.  If this push\n> is expected to error out, then wouldn't you want to make sure it\n> errors out as expected?  If the problem is that the exit status\n> is unreliable, maybe we need to make it reliable instead?\n\nWell, it is kind of undefined. git push just updated some remote\nreferences and failed on the others. It has had some failures, so it\nreturns non-0. And as I said, it really is not about the operation,\nbut about if the tracking and remote branches are set as we want them.\n"},{"id":"59825","messageId":"7vk5olkybv.fsf@gitster.siamese.dyndns.org","threadId":"10839","inReplyTo":"20071114071929.GA2942@steel.home","subject":"Re: [PATCH] Improved and extended t5404","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-14T08:47:48Z","receivedAt":"2007-11-14T08:47:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen <raa.lkml@gmail.com> writes:\n\n> Junio C Hamano, Wed, Nov 14, 2007 01:02:20 +0100:\n>> Alex Riesen <raa.lkml@gmail.com> writes:\n>> \n>> > Ignore exit code of git push in t5404, as it is not relevant for the\n>> > test\n>> \n>> This proposed log message solicits a \"Huh? -- Since when\n>> ignoring exit code is an improvement?\" reaction.  If this push\n>> is expected to error out, then wouldn't you want to make sure it\n>> errors out as expected?  If the problem is that the exit status\n>> is unreliable, maybe we need to make it reliable instead?\n>\n> Well, it is kind of undefined. git push just updated some remote\n> references and failed on the others. It has had some failures, so it\n> returns non-0. And as I said, it really is not about the operation,\n> but about if the tracking and remote branches are set as we want them.\n\nOk, I guess the proposed log message was not clear enough, at\nleast for me.\n\nWill apply.\n"},{"id":"59854","messageId":"Pine.LNX.4.64.0711141709410.4362@racer.site","threadId":"10839","inReplyTo":"20071114071929.GA2942@steel.home","subject":"Re: [PATCH] Improved and extended t5404","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-14T17:10:25Z","receivedAt":"2007-11-14T17:10:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 14 Nov 2007, Alex Riesen wrote:\n\n> Junio C Hamano, Wed, Nov 14, 2007 01:02:20 +0100:\n> > Alex Riesen <raa.lkml@gmail.com> writes:\n> > \n> > > Ignore exit code of git push in t5404, as it is not relevant for the \n> > > test\n> > \n> > This proposed log message solicits a \"Huh? -- Since when ignoring exit \n> > code is an improvement?\" reaction.  If this push is expected to error \n> > out, then wouldn't you want to make sure it errors out as expected?  \n> > If the problem is that the exit status is unreliable, maybe we need to \n> > make it reliable instead?\n> \n> Well, it is kind of undefined. git push just updated some remote \n> references and failed on the others. It has had some failures, so it \n> returns non-0. And as I said, it really is not about the operation, but \n> about if the tracking and remote branches are set as we want them.\n\nIf you know it should fail, why not make the test dependent on that \nfailure?  I mean, should git-push have a bug and not fail, it would be \nnice to catch this early...\n\nCiao,\nDscho\n"},{"id":"59865","messageId":"20071114194522.GA3973@steel.home","threadId":"10839","inReplyTo":"Pine.LNX.4.64.0711141709410.4362@racer.site","subject":"Re: [PATCH] Improved and extended t5404","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-14T19:45:22Z","receivedAt":"2007-11-14T19:45:22Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Johannes Schindelin, Wed, Nov 14, 2007 18:10:25 +0100:\n> On Wed, 14 Nov 2007, Alex Riesen wrote:\n> > Junio C Hamano, Wed, Nov 14, 2007 01:02:20 +0100:\n> > > Alex Riesen <raa.lkml@gmail.com> writes:\n> > > \n> > > > Ignore exit code of git push in t5404, as it is not relevant for the \n> > > > test\n> > > \n> > > This proposed log message solicits a \"Huh? -- Since when ignoring exit \n> > > code is an improvement?\" reaction.  If this push is expected to error \n> > > out, then wouldn't you want to make sure it errors out as expected?  \n> > > If the problem is that the exit status is unreliable, maybe we need to \n> > > make it reliable instead?\n> > \n> > Well, it is kind of undefined. git push just updated some remote \n> > references and failed on the others. It has had some failures, so it \n> > returns non-0. And as I said, it really is not about the operation, but \n> > about if the tracking and remote branches are set as we want them.\n> \n> If you know it should fail, why not make the test dependent on that \n> failure?  I mean, should git-push have a bug and not fail, it would be \n> nice to catch this early...\n> \n\nWell, I do not know it _should_ fail. Personally, I would not even\ncare: I see no way to cover with just one exit code multiple\nfailures. Some references were updated and I don't even know which.\nSo I'd better check whatever exit code.\n"},{"id":"59875","messageId":"20071114203409.GD3973@steel.home","threadId":"10839","inReplyTo":"20071114194522.GA3973@steel.home","subject":"Re: [PATCH] Improved and extended t5404","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-14T20:34:09Z","receivedAt":"2007-11-14T20:34:09Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Alex Riesen, Wed, Nov 14, 2007 20:45:22 +0100:\n> Johannes Schindelin, Wed, Nov 14, 2007 18:10:25 +0100:\n> > On Wed, 14 Nov 2007, Alex Riesen wrote:\n> > > Junio C Hamano, Wed, Nov 14, 2007 01:02:20 +0100:\n> > > > Alex Riesen <raa.lkml@gmail.com> writes:\n> > > > \n> > > > > Ignore exit code of git push in t5404, as it is not relevant for the \n> > > > > test\n> > > > \n> > > > This proposed log message solicits a \"Huh? -- Since when ignoring exit \n> > > > code is an improvement?\" reaction.  If this push is expected to error \n> > > > out, then wouldn't you want to make sure it errors out as expected?  \n> > > > If the problem is that the exit status is unreliable, maybe we need to \n> > > > make it reliable instead?\n> > > \n> > > Well, it is kind of undefined. git push just updated some remote \n> > > references and failed on the others. It has had some failures, so it \n> > > returns non-0. And as I said, it really is not about the operation, but \n> > > about if the tracking and remote branches are set as we want them.\n> > \n> > If you know it should fail, why not make the test dependent on that \n> > failure?  I mean, should git-push have a bug and not fail, it would be \n> > nice to catch this early...\n> > \n> \n> Well, I do not know it _should_ fail. Personally, I would not even\n> care: I see no way to cover with just one exit code multiple\n> failures. Some references were updated and I don't even know which.\n> So I'd better check whatever exit code.\n\n\"I'd better check whatever was updated and damn the exit code\"\n"},{"id":"59880","messageId":"7vbq9wfqb0.fsf@gitster.siamese.dyndns.org","threadId":"10839","inReplyTo":"20071113230234.GI3268@steel.home","subject":"Re: [PATCH] Improved and extended t5404","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-14T21:52:19Z","receivedAt":"2007-11-14T21:52:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen <raa.lkml@gmail.com> writes:\n\n> Ignore exit code of git push in t5404, as it is not relevant for the\n> test: it already checks whether the references updated correctly.\n\nI think the Subject: goes a lot better with a description like this:\n\n\tAdd test that checks the case where X does Y and make\n\tsure Z happens.\n\n\tBecause we haven't settled on what the exit status from\n\t\"git push\" command itself should be in such a partial\n\tfailure case, do not check the exit status from it for\n\tnow.\n\n> diff --git a/t/t5404-tracking-branches.sh b/t/t5404-tracking-branches.sh\n> index 20718d4..a51bbdc 100755\n> --- a/t/t5404-tracking-branches.sh\n> +++ b/t/t5404-tracking-branches.sh\n> @@ -10,6 +10,7 @@ test_expect_success 'setup' '\n>  \tgit commit -m 1 &&\n>  \tgit branch b1 &&\n>  \tgit branch b2 &&\n> +\tgit branch b3 &&\n>  \tgit clone . aa &&\n>  \tgit checkout b1 &&\n> ...\n\nSo it makes another ref \"b3\" point at the initial commit,... \n\n> ...\n>  test_expect_success 'check tracking branches updated correctly after push' '\n>  \tcd aa &&\n>  \tb1=$(git rev-parse origin/b1) &&\n>  \tb2=$(git rev-parse origin/b2) &&\n> +\tb3=$(git rev-parse origin/b3) &&\n>  \tgit checkout -b b1 origin/b1 &&\n>  \techo aa-b1 >>file &&\n>  \tgit commit -a -m aa-b1 &&\n\n... then records what was cloned,...\n\n> @@ -32,9 +36,28 @@ test_expect_success 'check tracking branches updated correctly after push' '\n>  \tgit checkout master &&\n>  \techo aa-master >>file &&\n>  \tgit commit -a -m aa-master &&\n> +\t{\n> +\t\tgit push\n> +\t\ttest \"$(git rev-parse origin/b1)\" = \"$b1\" &&\n> +\t\ttest \"$(git rev-parse origin/b2)\" = \"$b2\" &&\n> +\t\ttest \"$(git rev-parse origin/b3)\" = \"$b3\" &&\n> +\t\ttest \"$(git rev-parse origin/master)\" = \\\n> +\t\t\"$(git rev-parse master)\"\n> +\t}\n> +'\n\n... and checks that untouched \"b3\" stays the same (iow, tests\nup-to-date case).\n\n> +\n> +test_expect_success 'delete remote branch' '\n> +\tgit push origin :refs/heads/b3\n> +\t{\n> +\t\tgit rev-parse origin/b3\n> +\t\ttest $? != 0 || \\\n> +\t\tsay \"Hmm... Maybe tracking ref should be deleted?\"\n> +        } &&\n\nAh, you meant that tracking should be deleted so this should be\nfixed in the code but the test is disabled for now.  Let's be a\nbit more explicit about such a temporary disabled test, like\nthis:\n\n\tgit push origin :refs/heads/b3\n\n\t# The remote-tracking branch origin/b3 should be deleted;\n        # we need to update the code and enable this test.\n        : git rev-parse --verify origin/b3 &&\n\n> +\tcd \"$start_dir\" &&\n> +\t{\n> +\t\tgit rev-parse refs/heads/b3\n> +\t\ttest $? != 0\n> +        }\n>  '\n"},{"id":"59881","messageId":"7vabpgfqa4.fsf@gitster.siamese.dyndns.org","threadId":"10839","inReplyTo":"20071113230234.GI3268@steel.home","subject":"Re: [PATCH] Improved and extended t5404","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-14T21:52:51Z","receivedAt":"2007-11-14T21:52:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen <raa.lkml@gmail.com> writes:\n\n> Ignore exit code of git push in t5404, as it is not relevant for the\n> test: it already checks whether the references updated correctly.\n\nI think the Subject: goes a lot better with a description like this:\n\n\tEnhance the test to check the case where X does Y and to\n\tmake sure Z happens.\n\n\tBecause we haven't settled on what the exit status from\n\t\"git push\" command itself should be in such a partial\n\tfailure case, do not check the exit status from it for\n\tnow.\n\n> diff --git a/t/t5404-tracking-branches.sh b/t/t5404-tracking-branches.sh\n> index 20718d4..a51bbdc 100755\n> --- a/t/t5404-tracking-branches.sh\n> +++ b/t/t5404-tracking-branches.sh\n> @@ -10,6 +10,7 @@ test_expect_success 'setup' '\n>  \tgit commit -m 1 &&\n>  \tgit branch b1 &&\n>  \tgit branch b2 &&\n> +\tgit branch b3 &&\n>  \tgit clone . aa &&\n>  \tgit checkout b1 &&\n> ...\n\nSo it makes another ref \"b3\" point at the initial commit,... \n\n> ...\n>  test_expect_success 'check tracking branches updated correctly after push' '\n>  \tcd aa &&\n>  \tb1=$(git rev-parse origin/b1) &&\n>  \tb2=$(git rev-parse origin/b2) &&\n> +\tb3=$(git rev-parse origin/b3) &&\n>  \tgit checkout -b b1 origin/b1 &&\n>  \techo aa-b1 >>file &&\n>  \tgit commit -a -m aa-b1 &&\n\n... then records what was cloned,...\n\n> @@ -32,9 +36,28 @@ test_expect_success 'check tracking branches updated correctly after push' '\n>  \tgit checkout master &&\n>  \techo aa-master >>file &&\n>  \tgit commit -a -m aa-master &&\n> +\t{\n> +\t\tgit push\n> +\t\ttest \"$(git rev-parse origin/b1)\" = \"$b1\" &&\n> +\t\ttest \"$(git rev-parse origin/b2)\" = \"$b2\" &&\n> +\t\ttest \"$(git rev-parse origin/b3)\" = \"$b3\" &&\n> +\t\ttest \"$(git rev-parse origin/master)\" = \\\n> +\t\t\"$(git rev-parse master)\"\n> +\t}\n> +'\n\n... and checks that untouched \"b3\" stays the same (iow, tests\nup-to-date case).\n\n> +\n> +test_expect_success 'delete remote branch' '\n> +\tgit push origin :refs/heads/b3\n> +\t{\n> +\t\tgit rev-parse origin/b3\n> +\t\ttest $? != 0 || \\\n> +\t\tsay \"Hmm... Maybe tracking ref should be deleted?\"\n> +        } &&\n\nAh, you meant that tracking should be deleted so this should be\nfixed in the code but the test is disabled for now.  Let's be a\nbit more explicit about such a temporary disabled test, like\nthis:\n\n\tgit push origin :refs/heads/b3\n\n\t# The remote-tracking branch origin/b3 should be deleted;\n        # we need to update the code and enable this test.\n        : git rev-parse --verify origin/b3 &&\n\n> +\tcd \"$start_dir\" &&\n> +\t{\n> +\t\tgit rev-parse refs/heads/b3\n> +\t\ttest $? != 0\n> +        }\n>  '\n"},{"id":"59883","messageId":"Pine.LNX.4.64.0711142159400.4362@racer.site","threadId":"10839","inReplyTo":"20071114203409.GD3973@steel.home","subject":"Re: [PATCH] Improved and extended t5404","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-14T22:01:40Z","receivedAt":"2007-11-14T22:01:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 14 Nov 2007, Alex Riesen wrote:\n\n> Alex Riesen, Wed, Nov 14, 2007 20:45:22 +0100:\n>\n> > Well, I do not know it _should_ fail. Personally, I would not even \n> > care: I see no way to cover with just one exit code multiple failures. \n> > Some references were updated and I don't even know which. So I'd \n> > better check whatever exit code.\n> \n> \"I'd better check whatever was updated and damn the exit code\"\n\nMy point was: why not check both?  I mean, you know if it fails in your \ncase.  Better to test for this behaviour, than to have it succeed here, \nbut fail there.\n\nIt's really easy, too: if it does not succeed, it fails.  Just test for \nit.\n\nCiao,\nDscho \"consistency is good\"\n"},{"id":"59885","messageId":"20071114224926.GF3973@steel.home","threadId":"10839","inReplyTo":"7vbq9wfqb0.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] Add test that checks diverse aspects of updating remote and tracking branches","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-14T22:49:27Z","receivedAt":"2007-11-14T22:49:27Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Because we haven't settled on what the exit status from\n\"git push\" command itself should be in such a partial\nfailure case, do not check the exit status from it for\nnow.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nJunio C Hamano, Wed, Nov 14, 2007 22:52:19 +0100:\n> Alex Riesen <raa.lkml@gmail.com> writes:\n> \n> > Ignore exit code of git push in t5404, as it is not relevant for the\n> > test: it already checks whether the references updated correctly.\n> \n> I think the Subject: goes a lot better with a description like this:\n> \n> \tAdd test that checks the case where X does Y and make\n> \tsure Z happens.\n\nAdd test that checks diverse aspects of updating remote and tracking\nbranches.\n\n> \tBecause we haven't settled on what the exit status from\n> \t\"git push\" command itself should be in such a partial\n> \tfailure case, do not check the exit status from it for\n> \tnow.\n\nThis I'll leave as is.\n\n> > +\tgit branch b3 &&\n> \n> So it makes another ref \"b3\" point at the initial commit,... \n\nRight\n\n> > +\tb3=$(git rev-parse origin/b3) &&\n> \n> ... then records what was cloned,...\n\nPrecisely\n\n> > +\t\ttest \"$(git rev-parse origin/b3)\" = \"$b3\" &&\n> \n> ... and checks that untouched \"b3\" stays the same (iow, tests\n> up-to-date case).\n\nYep.\n\n> > +\n> > +test_expect_success 'delete remote branch' '\n> > +\tgit push origin :refs/heads/b3\n> > +\t{\n> > +\t\tgit rev-parse origin/b3\n> > +\t\ttest $? != 0 || \\\n> > +\t\tsay \"Hmm... Maybe tracking ref should be deleted?\"\n> > +        } &&\n> \n> Ah, you meant that tracking should be deleted so this should be\n> fixed in the code but the test is disabled for now.  Let's be a\n> bit more explicit about such a temporary disabled test, like\n> this:\n> \n> \tgit push origin :refs/heads/b3\n> \n> \t# The remote-tracking branch origin/b3 should be deleted;\n>         # we need to update the code and enable this test.\n>         : git rev-parse --verify origin/b3 &&\n\nNice, will take this. Except we have to check for absence of the\ntracking branch. git-rev-parse must fail.\n\n t/t5404-tracking-branches.sh |   64 ++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 64 insertions(+), 0 deletions(-)\n create mode 100755 t/t5404-tracking-branches.sh\n\ndiff --git a/t/t5404-tracking-branches.sh b/t/t5404-tracking-branches.sh\nnew file mode 100755\nindex 0000000..d861a14\n--- /dev/null\n+++ b/t/t5404-tracking-branches.sh\n@@ -0,0 +1,64 @@\n+#!/bin/sh\n+\n+test_description='tracking branch update checks for git push'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\techo 1 >file &&\n+\tgit add file &&\n+\tgit commit -m 1 &&\n+\tgit branch b1 &&\n+\tgit branch b2 &&\n+\tgit branch b3 &&\n+\tgit clone . aa &&\n+\tgit checkout b1 &&\n+\techo b1 >>file &&\n+\tgit commit -a -m b1 &&\n+\tgit checkout b2 &&\n+\techo b2 >>file &&\n+\tgit commit -a -m b2\n+'\n+\n+start_dir=\"$(pwd)\"\n+\n+test_expect_success 'check tracking branches updated correctly after push' '\n+\tcd aa &&\n+\tb1=$(git rev-parse origin/b1) &&\n+\tb2=$(git rev-parse origin/b2) &&\n+\tb3=$(git rev-parse origin/b3) &&\n+\tgit checkout -b b1 origin/b1 &&\n+\techo aa-b1 >>file &&\n+\tgit commit -a -m aa-b1 &&\n+\tgit checkout -b b2 origin/b2 &&\n+\techo aa-b2 >>file &&\n+\tgit commit -a -m aa-b2 &&\n+\tgit checkout master &&\n+\techo aa-master >>file &&\n+\tgit commit -a -m aa-master &&\n+\t{\n+\t\tgit push\n+\t\ttest \"$(git rev-parse origin/b1)\" = \"$b1\" &&\n+\t\ttest \"$(git rev-parse origin/b2)\" = \"$b2\" &&\n+\t\ttest \"$(git rev-parse origin/b3)\" = \"$b3\" &&\n+\t\ttest \"$(git rev-parse origin/master)\" = \\\n+\t\t\"$(git rev-parse master)\"\n+\t}\n+'\n+\n+test_expect_success 'delete remote branch' '\n+\tgit push origin :refs/heads/b3\n+\t{\n+\t# The remote-tracking branch origin/b3 should be deleted;\n+\t# we need to update the code and enable this test.\n+\t\t: git rev-parse --verify origin/b3\n+\t\t: test $? != 0\n+        } &&\n+\tcd \"$start_dir\" &&\n+\t{\n+\t\tgit rev-parse refs/heads/b3\n+\t\ttest $? != 0\n+        }\n+'\n+\n+test_done\n-- \n1.5.3.5.692.ge1737\n"},{"id":"59913","messageId":"20071115041801.GA9794@sigill.intra.peff.net","threadId":"10839","inReplyTo":"20071114071929.GA2942@steel.home","subject":"Re: [PATCH] Improved and extended t5404","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-15T04:18:03Z","receivedAt":"2007-11-15T04:18:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 14, 2007 at 08:19:29AM +0100, Alex Riesen wrote:\n\n> Well, it is kind of undefined. git push just updated some remote\n> references and failed on the others. It has had some failures, so it\n> returns non-0. And as I said, it really is not about the operation,\n> but about if the tracking and remote branches are set as we want them.\n\nMy goal with the recent patches is that _any_ failure will cause a non-0\nexit code (but you have to read the stderr output to find out which, if\nany, refs were successful).\n\n-Peff\n"},{"id":"59915","messageId":"20071115042625.GA10094@sigill.intra.peff.net","threadId":"10839","inReplyTo":"20071113231048.GB19444@sigill.intra.peff.net","subject":"Re: [PATCH] Improved and extended t5404","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-15T04:26:26Z","receivedAt":"2007-11-15T04:26:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 13, 2007 at 06:10:48PM -0500, Jeff King wrote:\n\n> > This one is on top of what is in next. It also include the check for\n> > deleting remote braches I sent before. Regarding this one: if a remote\n> > branch is deleted, shouldn't the matching tracking branch be removed\n> > as well? The code in master seem to do that.\n> \n> Yes, it should (the code in update_tracking_ref seems to handle that\n> case, but I haven't tested, so I may have bungled something). I am\n> literally walking out the door, now, though, so I will be out of touch\n> for at least a day.\n\nAfter I became disconnected, I looked at my 'next', and the reason for\nthe failure to delete the ref seems to be your is_null_sha1\nerror-checking patch, which Junio put in next. But maybe you have\nfigured that out in the intervening time. :)\n\n-Peff\n"},{"id":"59919","messageId":"20071115043513.GA10193@sigill.intra.peff.net","threadId":"10839","inReplyTo":"20071115041801.GA9794@sigill.intra.peff.net","subject":"Re: [PATCH] Improved and extended t5404","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-15T04:35:14Z","receivedAt":"2007-11-15T04:35:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 14, 2007 at 11:18:01PM -0500, Jeff King wrote:\n\n> My goal with the recent patches is that _any_ failure will cause a non-0\n> exit code (but you have to read the stderr output to find out which, if\n> any, refs were successful).\n\nBTW, since there seems to be some debate on how this _should_ work, I\nthink the \"signal failure if anything failed\" approach is the better.\n\nWhy?\n\nBecause either way you do it, there is an ambiguity, and I would rather\nthat ambiguity lie with the \"failure\" case. If I see exit code '0', I\n_know_ that all of my refs were updated. If I see exit code '1', then\nthere was some failure detected, but my refs might or might not have\nbeen updated. But that ambiguity _already_ exists. Consider the case\nwhere we send refs, but the connection dies in the middle. We have to\nsignal error, then, but for all we know the other side was about to\n\"successfully updated all refs\". So you can only ever _know_ success,\nand with failure, you simply guess (and presumably retry).\n\n-Peff\n"},{"id":"59925","messageId":"7vhcjo8337.fsf@gitster.siamese.dyndns.org","threadId":"10839","inReplyTo":"20071115043513.GA10193@sigill.intra.peff.net","subject":"Re: [PATCH] Improved and extended t5404","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-15T05:55:40Z","receivedAt":"2007-11-15T05:55:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> ... So you can only ever _know_ success,\n> and with failure, you simply guess (and presumably retry).\n\nGood thinking.  I agree.\n"},{"id":"60021","messageId":"20071115204611.GB3552@steel.home","threadId":"10839","inReplyTo":"20071115042625.GA10094@sigill.intra.peff.net","subject":"[PATCH] Add test that checks diverse aspects of updating remote and tracking branches","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-11-15T20:46:11Z","receivedAt":"2007-11-15T20:46:11Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Because we haven't settled on what the exit status from\n\"git push\" command itself should be in such a partial\nfailure case, do not check the exit status from it for\nnow.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nJeff King, Thu, Nov 15, 2007 05:26:26 +0100:\n> On Tue, Nov 13, 2007 at 06:10:48PM -0500, Jeff King wrote:\n> \n> > > This one is on top of what is in next. It also include the check for\n> > > deleting remote braches I sent before. Regarding this one: if a remote\n> > > branch is deleted, shouldn't the matching tracking branch be removed\n> > > as well? The code in master seem to do that.\n> > \n> > Yes, it should (the code in update_tracking_ref seems to handle that\n> > case, but I haven't tested, so I may have bungled something). I am\n> > literally walking out the door, now, though, so I will be out of touch\n> > for at least a day.\n> \n> After I became disconnected, I looked at my 'next', and the reason for\n> the failure to delete the ref seems to be your is_null_sha1\n> error-checking patch, which Junio put in next. But maybe you have\n> figured that out in the intervening time. :)\n\nI didn't. But Junio already has all your patches in pu, so I activated\nthe deletion test and rebased it on top of your patches in his tree\n(jk/send-pack, according to merge commit). Tried: works.\n\n t/t5404-tracking-branches.sh |   28 +++++++++++++++++++++++++---\n 1 files changed, 25 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t5404-tracking-branches.sh b/t/t5404-tracking-branches.sh\nindex 20718d4..a6f60ac 100755\n--- a/t/t5404-tracking-branches.sh\n+++ b/t/t5404-tracking-branches.sh\n@@ -10,6 +10,7 @@ test_expect_success 'setup' '\n \tgit commit -m 1 &&\n \tgit branch b1 &&\n \tgit branch b2 &&\n+\tgit branch b3 &&\n \tgit clone . aa &&\n \tgit checkout b1 &&\n \techo b1 >>file &&\n@@ -19,10 +20,13 @@ test_expect_success 'setup' '\n \tgit commit -a -m b2\n '\n \n+start_dir=\"$(pwd)\"\n+\n test_expect_success 'check tracking branches updated correctly after push' '\n \tcd aa &&\n \tb1=$(git rev-parse origin/b1) &&\n \tb2=$(git rev-parse origin/b2) &&\n+\tb3=$(git rev-parse origin/b3) &&\n \tgit checkout -b b1 origin/b1 &&\n \techo aa-b1 >>file &&\n \tgit commit -a -m aa-b1 &&\n@@ -32,9 +36,27 @@ test_expect_success 'check tracking branches updated correctly after push' '\n \tgit checkout master &&\n \techo aa-master >>file &&\n \tgit commit -a -m aa-master &&\n-\tgit push &&\n-\ttest \"$(git rev-parse origin/b1)\" = \"$b1\" &&\n-\ttest \"$(git rev-parse origin/b2)\" = \"$b2\"\n+\t{\n+\t\tgit push\n+\t\ttest \"$(git rev-parse origin/b1)\" = \"$b1\" &&\n+\t\ttest \"$(git rev-parse origin/b2)\" = \"$b2\" &&\n+\t\ttest \"$(git rev-parse origin/b3)\" = \"$b3\" &&\n+\t\ttest \"$(git rev-parse origin/master)\" = \\\n+\t\t\"$(git rev-parse master)\"\n+\t}\n+'\n+\n+test_expect_success 'delete remote branch' '\n+\tgit push origin :refs/heads/b3\n+\t{\n+\t\tgit rev-parse --verify origin/b3\n+\t\ttest $? != 0\n+        } &&\n+\tcd \"$start_dir\" &&\n+\t{\n+\t\tgit rev-parse refs/heads/b3\n+\t\ttest $? != 0\n+        }\n '\n \n test_done\n-- \n1.5.3.5.692.ge1737\n"}]}