{"thread":{"id":"33468","subject":"[PATCH 0/2] transport-helper: general fixes","startedAt":"2013-04-11T00:07:10Z","lastAt":"2013-04-16T00:56:58Z","messageCount":14,"participants":["Felipe Contreras","Jeff King","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"213875","messageId":"1365638832-9000-1-git-send-email-felipe.contreras@gmail.com","threadId":"33468","inReplyTo":null,"subject":"[PATCH 0/2] transport-helper: general fixes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-11T00:07:10Z","receivedAt":"2013-04-11T00:07:10Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nHere are a couple of patches that fix a few problems with remote-helpers.\n\nFelipe Contreras (2):\n transport-helper: improve push messages\n transport-helper: update remote helper namespace\n\n t/t5801-remote-helpers.sh | 26 ++++++++++++++++++++++++++\n transport-helper.c | 24 ++++++++++++++++++++----\n 2 files changed, 46 insertions(+), 4 deletions(-)\n\n-- \n1.8.2.1\n"},{"id":"213877","messageId":"1365638832-9000-2-git-send-email-felipe.contreras@gmail.com","threadId":"33468","inReplyTo":"1365638832-9000-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 1/2] transport-helper: improve push messages","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-11T00:07:11Z","receivedAt":"2013-04-11T00:07:11Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"If there's already a remote-helper tracking ref, we can fetch the SHA-1\nto report proper push messages (as opposed to always reporting\n[new branch]).\n\nThe remote-helper currently can specify the old SHA-1 to avoid this\nproblem, but there's no point in forcing all remote-helpers to be aware\nof git commit ids; they should be able to be agnostic of them.\n\nSigned-off-by: Felipe Contreras \n---\n t/t5801-remote-helpers.sh | 14 ++++++++++++++\n transport-helper.c | 1 +\n 2 files changed, 15 insertions(+)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex f387027..214aa40 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -166,4 +166,18 @@ test_expect_success 'push ref with existing object' '\n \tcompare_refs local dup server dup\n '\n \n+test_expect_success 'push messages' '\n+\t(cd local &&\n+\tgit checkout -b new_branch master &&\n+\techo new >>file &&\n+\tgit commit -a -m new &&\n+\tgit push origin new_branch &&\n+\tgit fetch origin &&\n+\techo new >>file &&\n+\tgit commit -a -m new &&\n+\tgit push origin new_branch 2> msg &&\n+\t! grep \"\\[new branch\\]\" msg\n+\t)\n+'\n+\n test_done\ndiff --git a/transport-helper.c b/transport-helper.c\nindex cb3ef7d..2257025 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -801,6 +801,7 @@ static int push_refs_with_export(struct transport *transport,\n \t\tif (private && !get_sha1(private, sha1)) {\n \t\t\tstrbuf_addf(&buf, \"^%s\", private);\n \t\t\tstring_list_append(&revlist_args, strbuf_detach(&buf, NULL));\n+\t\t\thashcpy(ref->old_sha1, sha1);\n \t\t}\n \t\tfree(private);\n \n-- \n1.8.2.1\n"},{"id":"213876","messageId":"1365638832-9000-3-git-send-email-felipe.contreras@gmail.com","threadId":"33468","inReplyTo":"1365638832-9000-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 2/2] transport-helper: update remote helper namespace","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-11T00:07:12Z","receivedAt":"2013-04-11T00:07:12Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"When pushing, the remote namespace is updated correctly\n(e.g. refs/origin/master), but not the remote helper's\n(e.g. refs/testgit/origin/master).\n\nLet's update it correctly.\n\nSigned-off-by: Felipe Contreras \n---\n t/t5801-remote-helpers.sh | 12 ++++++++++++\n transport-helper.c | 23 +++++++++++++++++++----\n 2 files changed, 31 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex 214aa40..07f2862 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -180,4 +180,16 @@ test_expect_success 'push messages' '\n \t)\n '\n \n+test_expect_success 'push update refs' '\n+\t(cd local &&\n+\tgit checkout -b update master &&\n+\techo update >>file &&\n+\tgit commit -a -m update &&\n+\tgit push origin update\n+\tgit rev-parse --verify testgit/origin/heads/update >expect &&\n+\tgit rev-parse --verify remotes/origin/update >actual\n+\ttest_cmp expect actual\n+\t)\n+'\n+\n test_done\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 2257025..7dff0c2 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -11,6 +11,7 @@\n #include \"thread-utils.h\"\n #include \"sigchain.h\"\n #include \"argv-array.h\"\n+#include \"refs.h\"\n \n static int debug;\n \n@@ -618,7 +619,7 @@ static int fetch(struct transport *transport,\n \treturn -1;\n }\n \n-static void push_update_ref_status(struct strbuf *buf,\n+static int push_update_ref_status(struct strbuf *buf,\n \t\t\t\t struct ref **ref,\n \t\t\t\t struct ref *remote_refs)\n {\n@@ -684,7 +685,7 @@ static void push_update_ref_status(struct strbuf *buf,\n \t\t*ref = find_ref_by_name(remote_refs, refname);\n \tif (!*ref) {\n \t\twarning(\"helper reported unexpected status of %s\", refname);\n-\t\treturn;\n+\t\treturn 1;\n \t}\n \n \tif ((*ref)->status != REF_STATUS_NONE) {\n@@ -693,11 +694,12 @@ static void push_update_ref_status(struct strbuf *buf,\n \t\t * status reported by the remote helper if the latter is 'no match'.\n \t\t */\n \t\tif (status == REF_STATUS_NONE)\n-\t\t\treturn;\n+\t\t\treturn 1;\n \t}\n \n \t(*ref)->status = status;\n \t(*ref)->remote_status = msg;\n+\treturn 0;\n }\n \n static void push_update_refs_status(struct helper_data *data,\n@@ -706,11 +708,24 @@ static void push_update_refs_status(struct helper_data *data,\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct ref *ref = remote_refs;\n \tfor (;;) {\n+\t\tchar *private;\n+\n \t\trecvline(data, &buf);\n \t\tif (!buf.len)\n \t\t\tbreak;\n \n-\t\tpush_update_ref_status(&buf, &ref, remote_refs);\n+\t\tif (push_update_ref_status(&buf, &ref, remote_refs))\n+\t\t\tcontinue;\n+\n+\t\tif (!data->refspecs)\n+\t\t\tcontinue;\n+\n+\t\t/* propagate back the update to the remote namespace */\n+\t\tprivate = apply_refspecs(data->refspecs, data->refspec_nr, ref->name);\n+\t\tif (!private)\n+\t\t\tcontinue;\n+\t\tupdate_ref(\"update by helper\", private, ref->new_sha1, NULL, 0, 0);\n+\t\tfree(private);\n \t}\n \tstrbuf_release(&buf);\n }\n-- \n1.8.2.1\n"},{"id":"213893","messageId":"20130411034102.GD14551@sigill.intra.peff.net","threadId":"33468","inReplyTo":"1365638832-9000-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 1/2] transport-helper: improve push messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-11T03:41:02Z","receivedAt":"2013-04-11T03:41:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 10, 2013 at 07:07:11PM -0500, Felipe Contreras wrote:\n\n> If there's already a remote-helper tracking ref, we can fetch the SHA-1\n> to report proper push messages (as opposed to always reporting\n> [new branch]).\n> \n> The remote-helper currently can specify the old SHA-1 to avoid this\n> problem, but there's no point in forcing all remote-helpers to be aware\n> of git commit ids; they should be able to be agnostic of them.\n> \n> Signed-off-by: Felipe Contreras \n\nYeah, this looks correct to me. Thanks.\n\n-Peff\n"},{"id":"213895","messageId":"20130411043346.GE14551@sigill.intra.peff.net","threadId":"33468","inReplyTo":"1365638832-9000-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 2/2] transport-helper: update remote helper namespace","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-11T04:33:46Z","receivedAt":"2013-04-11T04:33:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 10, 2013 at 07:07:12PM -0500, Felipe Contreras wrote:\n\n> When pushing, the remote namespace is updated correctly\n> (e.g. refs/origin/master), but not the remote helper's\n> (e.g. refs/testgit/origin/master).\n> \n> Let's update it correctly.\n\nI would have thought it was the helper's responsibility to update these.\nObviously remote-testgit can handle this fine, but will any other\nhelpers be using these refs as a marker to know the last point they\nimported, and get confused if we update the refs behind their back?\n\nFor example, during the import, a helper might know that it has imported\nup to X on a foreign vcs, and that resulted in commit Y in git, which it\nstored in refs/$helper/heads/master during the last import. When we\nfetch from it again, it picks up from X to the tip of the foreign vcs,\nand then imports that history on top of commit Y.\n\nBut if we push some commits to the helper, moving Y up to Z, then it\nwould build the new commit (which contains the foreign-vcs's equivalent of\nY..Z) on top of Z, not Y.\n\nI do not offhand know of any helpers that are implemented this way,\nthough. vcs-svn does not seem to use the refspec feature at all, and I\nassume that your hg/bzr helpers do not have this problem. So perhaps it\nis not worth worrying about.\n\n-Peff\n"},{"id":"213897","messageId":"CAMP44s0FkiwPMJVhVBNa32J3rgghRZy6xDTN-YnHKcQ4Fj0BMQ@mail.gmail.com","threadId":"33468","inReplyTo":"20130411043346.GE14551@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] transport-helper: update remote helper namespace","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-11T04:53:38Z","receivedAt":"2013-04-11T04:53:38Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Apr 10, 2013 at 11:33 PM, Jeff King wrote:\n> On Wed, Apr 10, 2013 at 07:07:12PM -0500, Felipe Contreras wrote:\n>\n>> When pushing, the remote namespace is updated correctly\n>> (e.g. refs/origin/master), but not the remote helper's\n>> (e.g. refs/testgit/origin/master).\n>>\n>> Let's update it correctly.\n>\n> I would have thought it was the helper's responsibility to update these.\n> Obviously remote-testgit can handle this fine, but will any other\n> helpers be using these refs as a marker to know the last point they\n> imported, and get confused if we update the refs behind their back?\n>\n> For example, during the import, a helper might know that it has imported\n> up to X on a foreign vcs, and that resulted in commit Y in git, which it\n> stored in refs/$helper/heads/master during the last import. When we\n> fetch from it again, it picks up from X to the tip of the foreign vcs,\n> and then imports that history on top of commit Y.\n>\n> But if we push some commits to the helper, moving Y up to Z, then it\n> would build the new commit (which contains the foreign-vcs's equivalent of\n> Y..Z) on top of Z, not Y.\n\nWhy would it do that? If X points to say revision 100, presumably it\nwas stored somewhere while doing a fetch. Similarly, if foreign\nversion of Z is 150, it can update that number while doing a push. The\nnext fetch it would start from 151.\n\nAll this is hypothetical of course, because...\n\n> I do not offhand know of any helpers that are implemented this way,\n> though. vcs-svn does not seem to use the refspec feature at all, and I\n> assume that your hg/bzr helpers do not have this problem. So perhaps it\n> is not worth worrying about.\n\nThey cannot be implemented this way, because as I have already argued\nand shown[1], remote helpers must be using marks, they don't work\notherwise (transport helper is broken for those cases).\n\nCheers.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/210306\n\n--\nFelipe Contreras\n"},{"id":"213899","messageId":"20130411050509.GC27795@sigill.intra.peff.net","threadId":"33468","inReplyTo":"CAMP44s0FkiwPMJVhVBNa32J3rgghRZy6xDTN-YnHKcQ4Fj0BMQ@mail.gmail.com","subject":"Re: [PATCH 2/2] transport-helper: update remote helper namespace","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-11T05:05:09Z","receivedAt":"2013-04-11T05:05:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 10, 2013 at 11:53:38PM -0500, Felipe Contreras wrote:\n\n> > But if we push some commits to the helper, moving Y up to Z, then it\n> > would build the new commit (which contains the foreign-vcs's equivalent of\n> > Y..Z) on top of Z, not Y.\n> \n> Why would it do that? If X points to say revision 100, presumably it\n> was stored somewhere while doing a fetch. Similarly, if foreign\n> version of Z is 150, it can update that number while doing a push. The\n> next fetch it would start from 151.\n\nI think the only reason not to bump the marker forward during the push\nwould be if the helper wants for some reason to \"re-import\" from the\nforeign source rather than accepting the git versions of the commits.\nSomething like git-svn's markup of the commit messages with revision ids\ncomes to mind. But if it matters, then by definition that would mean\nthat the import/export is not bidirectionally clean. git-svn is\ndefinitely not that, but I hope that vcs-svn will be (I have not kept up\non its status).\n\nSo I can buy the argument that bumping it forward ourselves will not\nmatter for any well-implemented helper.\n\nThat is the sort of thing that might be helpful to include in the commit\nmessage; if somebody does run across such a helper and bisects to your\ncommit, then they can understand the rationale for the decision.\n\n-Peff\n"},{"id":"213900","messageId":"CAMP44s1LF46VU0E4W=r-qog3JY+Y-qyYGfkqxLEnTG8X8GoWOA@mail.gmail.com","threadId":"33468","inReplyTo":"20130411050509.GC27795@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] transport-helper: update remote helper namespace","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-11T05:18:08Z","receivedAt":"2013-04-11T05:18:08Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Apr 11, 2013 at 12:05 AM, Jeff King wrote:\n> On Wed, Apr 10, 2013 at 11:53:38PM -0500, Felipe Contreras wrote:\n>\n>> > But if we push some commits to the helper, moving Y up to Z, then it\n>> > would build the new commit (which contains the foreign-vcs's equivalent of\n>> > Y..Z) on top of Z, not Y.\n>>\n>> Why would it do that? If X points to say revision 100, presumably it\n>> was stored somewhere while doing a fetch. Similarly, if foreign\n>> version of Z is 150, it can update that number while doing a push. The\n>> next fetch it would start from 151.\n>\n> I think the only reason not to bump the marker forward during the push\n> would be if the helper wants for some reason to \"re-import\" from the\n> foreign source rather than accepting the git versions of the commits.\n> Something like git-svn's markup of the commit messages with revision ids\n> comes to mind.\n\nYeah, but that's already a second level hypothesis. First,\nremote-helpers would need to be able to work without marks, and they\ncan't.\n\n> But if it matters, then by definition that would mean\n> that the import/export is not bidirectionally clean.\n\nI don't see how would that matter.\n\n> So I can buy the argument that bumping it forward ourselves will not\n> matter for any well-implemented helper.\n\nOr any helper.\n\n> That is the sort of thing that might be helpful to include in the commit\n> message; if somebody does run across such a helper and bisects to your\n> commit, then they can understand the rationale for the decision.\n\nIf it did matter, it would be mentioned. I will updated it later if\nthere's no further comments.\n\n--\nFelipe Contreras\n"},{"id":"214125","messageId":"CAMP44s3+eaRbeXP0bPXMSE8Z1K_Lqyu8e1XCvudkapkTJFBWnA@mail.gmail.com","threadId":"33468","inReplyTo":"CAMP44s1LF46VU0E4W=r-qog3JY+Y-qyYGfkqxLEnTG8X8GoWOA@mail.gmail.com","subject":"Re: [PATCH 2/2] transport-helper: update remote helper namespace","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-13T06:00:07Z","receivedAt":"2013-04-13T06:00:07Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nWhy wasn't this patch merged to 'pu'? To my knowledge nobody raised\nany real concerns.\n\nShould I explain in every commit that touches transport-helper how\nremote-helpers without marks are impossible? I know I said I was going\nto update the commit message, but I don't think that reason to not put\nit in 'pu'. Also, the only reason I said so was to make Jeff happy,\nbut now that I think again, it doesn't really belong there; remote\nhelpers cannot be using these refs, they just can't. They cannot work\nwithout marks, it's not possible. To think otherwise is simply a\nmistake.\n\nOn Thu, Apr 11, 2013 at 12:18 AM, Felipe Contreras\n wrote:\n> On Thu, Apr 11, 2013 at 12:05 AM, Jeff King wrote:\n>> On Wed, Apr 10, 2013 at 11:53:38PM -0500, Felipe Contreras wrote:\n>>\n>>> > But if we push some commits to the helper, moving Y up to Z, then it\n>>> > would build the new commit (which contains the foreign-vcs's equivalent of\n>>> > Y..Z) on top of Z, not Y.\n>>>\n>>> Why would it do that? If X points to say revision 100, presumably it\n>>> was stored somewhere while doing a fetch. Similarly, if foreign\n>>> version of Z is 150, it can update that number while doing a push. The\n>>> next fetch it would start from 151.\n>>\n>> I think the only reason not to bump the marker forward during the push\n>> would be if the helper wants for some reason to \"re-import\" from the\n>> foreign source rather than accepting the git versions of the commits.\n>> Something like git-svn's markup of the commit messages with revision ids\n>> comes to mind.\n>\n> Yeah, but that's already a second level hypothesis. First,\n> remote-helpers would need to be able to work without marks, and they\n> can't.\n>\n>> But if it matters, then by definition that would mean\n>> that the import/export is not bidirectionally clean.\n>\n> I don't see how would that matter.\n>\n>> So I can buy the argument that bumping it forward ourselves will not\n>> matter for any well-implemented helper.\n>\n> Or any helper.\n>\n>> That is the sort of thing that might be helpful to include in the commit\n>> message; if somebody does run across such a helper and bisects to your\n>> commit, then they can understand the rationale for the decision.\n>\n> If it did matter, it would be mentioned. I will updated it later if\n> there's no further comments.\n\n-- \nFelipe Contreras\n"},{"id":"214153","messageId":"7v8v4l7ils.fsf@alter.siamese.dyndns.org","threadId":"33468","inReplyTo":"CAMP44s3+eaRbeXP0bPXMSE8Z1K_Lqyu8e1XCvudkapkTJFBWnA@mail.gmail.com","subject":"Re: [PATCH 2/2] transport-helper: update remote helper namespace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-14T05:13:55Z","receivedAt":"2013-04-14T05:13:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Why wasn't this patch merged to 'pu'? To my knowledge nobody raised\n> any real concerns.\n\nThere are many reasons not to queue _everything_ ever posted to the\nlist on 'pu', and they are almost always not a deliberate rejection.\n\nThe maintainer may have thought he is not the best person to judge\nchanges to the area the patch touches, and may be expecting further\ncomments from others, but haven't said \"Comments?\" and waiting for\nthem to say something without being asked. Or the maintainer may\nhave judged that it is likely to result in wasted work if he queues\nthat version of the patch, fixing trivial nits himself, only to see\na reroll arrive before the day's integration cycle finishes (which\nmakes him run the cycle again). Or the maintainer may have been busy\ntending to other topics. Or the maintainer may have pushed the patch\ndown the queue for any of the above reasons to deal with it later,\nand after having tended to others' topics, may have forgotten about\nthat patch.\n\nDo I need to go on?\n\n> I know I said I was going\n> to update the commit message, but I don't think that reason to not put\n> it in 'pu'.\n\nFor this particular case, I think that was exactly the reason why it\nis not in 'pu' today. I actually did not remember the reason when\nyou asked above, until I read the above \"I said I'll update\".\n\nOne major reason why I queue a patch that is clearly not ready for\n'next' is because I do not want to forget about the topic and not\nbecause I want to keep the particular version of the patch. I do so\nespecially when contributor is unlikely to come back soon. If you\nsaid you would come back soon, I do not even have to judge if it is\n\"clearly not ready\" or \"it is good enough\" for next, and I have to\nremember one less thing. The more I can put in \"this will come back\nso I do not have to do anything\" bin, not in the \"queue it as-is for\nnow, because it is likely that it won't be rerolled soon and I'll\nforget about it\" bin, the easier for me and we as the development\nprocess as a whole can scale better.\n"},{"id":"214215","messageId":"CAMP44s22N9_E4oBG0UztXE0yAkX8TmQaE9-x1_wDHqv3bvZ0EQ@mail.gmail.com","threadId":"33468","inReplyTo":"7v8v4l7ils.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] transport-helper: update remote helper namespace","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-14T15:42:47Z","receivedAt":"2013-04-14T15:42:47Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Apr 14, 2013 at 12:13 AM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> Why wasn't this patch merged to 'pu'? To my knowledge nobody raised\n>> any real concerns.\n>\n> There are many reasons not to queue _everything_ ever posted to the\n> list on 'pu', and they are almost always not a deliberate rejection.\n>\n> The maintainer may have thought he is not the best person to judge\n> changes to the area the patch touches, and may be expecting further\n> comments from others, but haven't said \"Comments?\" and waiting for\n> them to say something without being asked. Or the maintainer may\n> have judged that it is likely to result in wasted work if he queues\n> that version of the patch, fixing trivial nits himself, only to see\n> a reroll arrive before the day's integration cycle finishes (which\n> makes him run the cycle again). Or the maintainer may have been busy\n> tending to other topics. Or the maintainer may have pushed the patch\n> down the queue for any of the above reasons to deal with it later,\n> and after having tended to others' topics, may have forgotten about\n> that patch.\n\nThe world is full of possibilities, but most of them are irrelevant,\nspecially since 'the maintainer' is right here and can mention the\nreason himself. Is there anything wrong in asking?\n\n>> I know I said I was going\n>> to update the commit message, but I don't think that reason to not put\n>> it in 'pu'.\n>\n> For this particular case, I think that was exactly the reason why it\n> is not in 'pu' today. I actually did not remember the reason when\n> you asked above, until I read the above \"I said I'll update\".\n>\n> One major reason why I queue a patch that is clearly not ready for\n> 'next' is because I do not want to forget about the topic and not\n> because I want to keep the particular version of the patch. I do so\n> especially when contributor is unlikely to come back soon. If you\n> said you would come back soon, I do not even have to judge if it is\n> \"clearly not ready\" or \"it is good enough\" for next, and I have to\n> remember one less thing. The more I can put in \"this will come back\n> so I do not have to do anything\" bin, not in the \"queue it as-is for\n> now, because it is likely that it won't be rerolled soon and I'll\n> forget about it\" bin, the easier for me and we as the development\n> process as a whole can scale better.\n\nWell, I'm telling you I think it's good as it is. Other people might\ndisagree, but if they do so, it's possibly on the basis that other\ntransport-helpers might not be using marks, which is something I have\nsaid time and again is not possible, and to think it is, is a mistake.\n\nI am willing to demonstrate the above claim beyond any reasonable\ndoubt to any sensible person (haven't I done so already?), so that we\ncan avoid these discussions again in the future, and include in the\ndocumentation that marks are necessary, or throw a warning for the\npeople developing a remote helper, or something along those lines.\n\nBut if that's not what you want me to do, then what is needed to get\nthis patch into 'pu'? That's what I'm wondering.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"214219","messageId":"7vobdh54uh.fsf@alter.siamese.dyndns.org","threadId":"33468","inReplyTo":"CAMP44s22N9_E4oBG0UztXE0yAkX8TmQaE9-x1_wDHqv3bvZ0EQ@mail.gmail.com","subject":"Re: [PATCH 2/2] transport-helper: update remote helper namespace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-14T18:45:10Z","receivedAt":"2013-04-14T18:45:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Sun, Apr 14, 2013 at 12:13 AM, Junio C Hamano wrote:\n>> Felipe Contreras writes:\n>>\n>>> Why wasn't this patch merged to 'pu'? To my knowledge nobody raised\n>>> any real concerns.\n>>\n>> There are many reasons not to queue _everything_ ever posted to the\n>> list on 'pu', and they are almost always not a deliberate rejection.\n>>\n>> The maintainer may have thought he is not the best person to judge\n>> changes to the area the patch touches, and may be expecting further\n>> comments from others, but haven't said \"Comments?\" and waiting for\n>> them to say something without being asked. Or the maintainer may\n>> have judged that it is likely to result in wasted work if he queues\n>> that version of the patch, fixing trivial nits himself, only to see\n>> a reroll arrive before the day's integration cycle finishes (which\n>> makes him run the cycle again). Or the maintainer may have been busy\n>> tending to other topics. Or the maintainer may have pushed the patch\n>> down the queue for any of the above reasons to deal with it later,\n>> and after having tended to others' topics, may have forgotten about\n>> that patch.\n>\n> The world is full of possibilities, but most of them are irrelevant,\n> specially since 'the maintainer' is right here and can mention the\n> reason himself. Is there anything wrong in asking?\n\nAn earlier draft of my message starte with \"Do you have to be\ncombative to ask a simple 'did you forget this?' question?\", but\nlater I removed it. That was what made it irrelevant ;-)\n\nJust rerolling with what _you_ think is an appropriate level of\nexplanation (either or both in log and in-code) and see what happens\nwould probably be the best way to proceed, I think, at this\npoint. Either you hear \"It still is wrong and too sketchy\", \"Yeah,\nthinking about it again, this is sufficient\" from others. Or a\nsilent, which I am inclined to take as much closer to the latter\nafter all the discussion.\n"},{"id":"214223","messageId":"20130414190050.GA4599@sigill.intra.peff.net","threadId":"33468","inReplyTo":"7vobdh54uh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] transport-helper: update remote helper namespace","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-14T19:00:50Z","receivedAt":"2013-04-14T19:00:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Apr 14, 2013 at 11:45:10AM -0700, Junio C Hamano wrote:\n\n> Just rerolling with what _you_ think is an appropriate level of\n> explanation (either or both in log and in-code) and see what happens\n> would probably be the best way to proceed, I think, at this\n> point. Either you hear \"It still is wrong and too sketchy\", \"Yeah,\n> thinking about it again, this is sufficient\" from others. Or a\n> silent, which I am inclined to take as much closer to the latter\n> after all the discussion.\n\nFWIW, the last email I wrote on this patch said:\n\n So I can buy the argument that bumping it forward ourselves will not\n matter for any well-implemented helper.\n\nand I was the only reviewer, so I think the code is probably OK. I also\nsaid:\n\n That is the sort of thing that might be helpful to include in the\n commit message[...]\n\nFelipe of course did not agree, but I have no interest in trying to\npersuade him on that front, as it seems to just waste everyone's time.\n\n-Peff\n"},{"id":"214410","messageId":"20130416005658.GA3262@elie.Belkin","threadId":"33468","inReplyTo":"7vobdh54uh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] transport-helper: update remote helper namespace","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-04-16T00:56:58Z","receivedAt":"2013-04-16T00:56:58Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> Just rerolling with what _you_ think is an appropriate level of\n> explanation (either or both in log and in-code) and see what happens\n> would probably be the best way to proceed, I think, at this\n> point. Either you hear \"It still is wrong and too sketchy\", \"Yeah,\n> thinking about it again, this is sufficient\" from others. Or a\n> silent, which I am inclined to take as much closer to the latter\n> after all the discussion.\n\nFor future reference, sometimes when I am silent it does not mean\nagreement but means \"I am fed up and don't consider it to be worth the\ndrain on my energy to deal with this mess.\" Which is pretty close to\n\"let's move on; this is sufficient\", yes.\n\nThanks,\nJonathan\n"}]}