{"thread":{"id":"8607","subject":"git-send-pack SIGSEGV..","startedAt":"2007-06-15T04:29:46Z","lastAt":"2007-06-16T21:05:10Z","messageCount":10,"participants":["Linus Torvalds","Junio C Hamano","Alex Riesen","Daniel Barkalow"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"45112","messageId":"alpine.LFD.0.98.0706142124380.14121@woody.linux-foundation.org","threadId":"8607","inReplyTo":null,"subject":"git-send-pack SIGSEGV..","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-06-15T04:29:46Z","receivedAt":"2007-06-15T04:29:46Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nI haevn't had time to debug this at all, but\n\n\tgit push --tags all\n\nactually SIGSEGV's for me on the git repository. \"all\" is just a shorthand \nfor the two repos I push my git repo to (so that I have it on some other \nmachines too)\n\n\t[remote \"all\"]\n\t\turl = master.kernel.org:git\n\t\turl = login.linux-foundation.org:git.git\n\nWithout any real debugging at all, here's what I get:\n\n\terror: failed to push to 'master.kernel.org:git'\n\terror: send-pack died with strange error\n\tfatal: The remote end hung up unexpectedly\n\terror: failed to push to 'login.linux-foundation.org:git.git'\n\terror: send-pack died with strange error\n\tfatal: The remote end hung up unexpectedly\n\nand a few core-files. The core-files look like so:\n\n\tCore was generated by `/home/torvalds/bin/git-send-pack --remote=all --thin master.kernel.org git refs'.\n\tProgram terminated with signal 11, Segmentation fault.\n\t#0  0x0000003959477180 in strlen () from /lib64/libc.so.6\n\t(gdb) where\n\t#0  0x0000003959477180 in strlen () from /lib64/libc.so.6\n\t#1  0x000000000041cebe in match_refs (src=0x64b540, dst=<value optimized out>, dst_tail=0x62c890, nr_refspec=1, refspec=<value optimized out>, all=0) at remote.c:539\n\t#2  0x0000000000402e46 in main (argc=<value optimized out>, argv=0x7fffab2657a8) at send-pack.c:202\n\nbut I'm about to put the kids to bed, and I doubt I'll get to debug this \nbefore tomorrow. So I thought I'd send this out to Junio, Daniel and the \nlist, in case somebody else gets to it first.\n\nI *suspect* it's due to the refspec pattern matching changes Daniel did, \nbut again - I haven't actually debugged it any deeper.\n\n\t\t\tLinus\n"},{"id":"45113","messageId":"alpine.LFD.0.98.0706142130190.14121@woody.linux-foundation.org","threadId":"8607","inReplyTo":"alpine.LFD.0.98.0706142124380.14121@woody.linux-foundation.org","subject":"Re: git-send-pack SIGSEGV..","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-06-15T04:31:06Z","receivedAt":"2007-06-15T04:31:06Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nOn Thu, 14 Jun 2007, Linus Torvalds wrote:\n\n> \n> I haevn't had time to debug this at all, but\n> \n> \tgit push --tags all\n> \n> actually SIGSEGV's for me on the git repository.\n\nOh, and this is with current 'master', of course. Commit 38570a47fc to be \nexact.\n\n\t\tLinus\n"},{"id":"45115","messageId":"7vvedp935y.fsf@assigned-by-dhcp.pobox.com","threadId":"8607","inReplyTo":"alpine.LFD.0.98.0706142124380.14121@woody.linux-foundation.org","subject":"Re: git-send-pack SIGSEGV..","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-15T05:50:49Z","receivedAt":"2007-06-15T05:50:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> I *suspect* it's due to the refspec pattern matching changes Daniel did, \n> but again - I haven't actually debugged it any deeper.\n\nI am officially recuperating from an operation I had today, so I\ncannot really take a deep look at this.\n\nI think what is going wrong is that struct refspec for pattern\nmatch that is parsed by parse_ref_spec does not have ->dst\ncomponent filled for \"refs/tags/*\" refspec, but match_refs()\ndoes not check if pat->dst is NULL, in which case it should\nreuse pat->src value.\n\nIncidentally I have other remote.c fixes queued in 'next'.  I\nhaven't yet checked if I (accidentally) fixed this already.\n"},{"id":"45117","messageId":"7vr6od92nj.fsf@assigned-by-dhcp.pobox.com","threadId":"8607","inReplyTo":"7vvedp935y.fsf@assigned-by-dhcp.pobox.com","subject":"Re: git-send-pack SIGSEGV..","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-15T06:01:52Z","receivedAt":"2007-06-15T06:01:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Linus Torvalds <torvalds@linux-foundation.org> writes:\n>\n>> I *suspect* it's due to the refspec pattern matching changes Daniel did, \n>> but again - I haven't actually debugged it any deeper.\n>\n> I am officially recuperating from an operation I had today, so I\n> cannot really take a deep look at this.\n>\n> I think what is going wrong is that struct refspec for pattern\n> match that is parsed by parse_ref_spec does not have ->dst\n> component filled for \"refs/tags/*\" refspec, but match_refs()\n> does not check if pat->dst is NULL, in which case it should\n> reuse pat->src value.\n>\n> Incidentally I have other remote.c fixes queued in 'next'.  I\n> haven't yet checked if I (accidentally) fixed this already.\n\nCompletely untested, but this may fix it.\n\nI suspect this has an side effect of allowing\n\n\tfetch = refs/heads/*\n\nto mean the same thing as\n\n\tfetch = refs/heads/*:refs/heads/*\n\nwhich is suitable for a bare mirroring repository, but I do not\nthink of any downside, so it might be Ok.\n\nBut that is something from a person who was under anesthesia a\nfew hours ago, so you should take it with a big grain of salt ;-)\n\n---\n\n remote.c |    2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex ed62a62..356e6bc 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -252,6 +252,8 @@ static struct refspec *parse_ref_spec(int nr_refspec, const char **refspec)\n \t\t\tep = gp;\n \t\t}\n \t\trs[i].src = xstrndup(sp, ep - sp);\n+\t\tif (rs[i].pattern && !rs[i].dst)\n+\t\t\trs[i].dst = xstrdup(rs[i].src);\n \t}\n \treturn rs;\n }\n"},{"id":"45123","messageId":"81b0412b0706150024m6a3c4d0bx866e1b25e770d299@mail.gmail.com","threadId":"8607","inReplyTo":"7vr6od92nj.fsf@assigned-by-dhcp.pobox.com","subject":"Re: git-send-pack SIGSEGV..","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-06-15T07:24:18Z","receivedAt":"2007-06-15T07:24:18Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 6/15/07, Junio C Hamano <gitster@pobox.com> wrote:\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -252,6 +252,8 @@ static struct refspec *parse_ref_spec\n>                         ep = gp;\n>                 }\n>                 rs[i].src = xstrndup(sp, ep - sp);\n> +               if (rs[i].pattern && !rs[i].dst)\n> +                       rs[i].dst = xstrdup(rs[i].src);\n\nIt may be possible to just reuse rs[i].src - there is no deallocation\nof refspecs anywhere nor are src or dst detached from refspec\n(which may be bad, but probably is covered in libification effort).\n"},{"id":"45135","messageId":"Pine.LNX.4.64.0706150949300.5848@iabervon.org","threadId":"8607","inReplyTo":"7vr6od92nj.fsf@assigned-by-dhcp.pobox.com","subject":"Re: git-send-pack SIGSEGV..","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-06-15T14:27:46Z","receivedAt":"2007-06-15T14:27:46Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Thu, 14 Jun 2007, Junio C Hamano wrote:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > Linus Torvalds <torvalds@linux-foundation.org> writes:\n> >\n> >> I *suspect* it's due to the refspec pattern matching changes Daniel did, \n> >> but again - I haven't actually debugged it any deeper.\n> >\n> > I am officially recuperating from an operation I had today, so I\n> > cannot really take a deep look at this.\n> >\n> > I think what is going wrong is that struct refspec for pattern\n> > match that is parsed by parse_ref_spec does not have ->dst\n> > component filled for \"refs/tags/*\" refspec, but match_refs()\n> > does not check if pat->dst is NULL, in which case it should\n> > reuse pat->src value.\n> >\n> > Incidentally I have other remote.c fixes queued in 'next'.  I\n> > haven't yet checked if I (accidentally) fixed this already.\n> \n> Completely untested, but this may fix it.\n> \n> I suspect this has an side effect of allowing\n> \n> \tfetch = refs/heads/*\n> \n> to mean the same thing as\n> \n> \tfetch = refs/heads/*:refs/heads/*\n> \n> which is suitable for a bare mirroring repository, but I do not\n> think of any downside, so it might be Ok.\n> \n> But that is something from a person who was under anesthesia a\n> few hours ago, so you should take it with a big grain of salt ;-)\n\nYeah, that's not right; \"push = refs/heads/*\" works like that, but \n\"fetch = refs/heads/*\" puts them in MERGE_HEAD without storing them \nanywhere, unlike \"fetch = refs/heads/*:refs/heads/*\". That's why I didn't \njust copy the left side to the right side, which is what the old parsing \ncode did: what you're going to do with the refspec determines how you \ninterpret a missing rhs. So it needs to be the code that uses the refspec \nthat handles this case.\n\nYour analysis of the failure was right, though. I reproduced it, and this \nfixes it for me:\n\n--- cut here ---\nAuthor: Daniel Barkalow <barkalow@iabervon.org>\nDate:   Fri Jun 15 10:22:37 2007 -0400\n\n    Fix pushing to a pattern with no dst\n    \n    Refspecs with no colons are left with no dst value, because they are\n    interepreted differently for fetch and push. For push, they mean to\n    reuse the src side. Fix this for patterns.\n    \n    Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>\n\ndiff --git a/remote.c b/remote.c\nindex 6121416..c860740 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -546,10 +546,11 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,\n \t\t}\n \n \t\tif (pat) {\n-\t\t\tdst_name = xmalloc(strlen(pat->dst) +\n+\t\t\tconst char *dst_side = pat->dst ? pat->dst : pat->src;\n+ \t\t\tdst_name = xmalloc(strlen(dst_side) +\n \t\t\t\t\t   strlen(src->name) -\n \t\t\t\t\t   strlen(pat->src) + 2);\n-\t\t\tstrcpy(dst_name, pat->dst);\n+\t\t\tstrcpy(dst_name, dst_side);\n \t\t\tstrcat(dst_name, src->name + strlen(pat->src));\n \t\t} else\n \t\t\tdst_name = xstrdup(src->name);\n"},{"id":"45145","messageId":"alpine.LFD.0.98.0706151025490.14121@woody.linux-foundation.org","threadId":"8607","inReplyTo":"Pine.LNX.4.64.0706150949300.5848@iabervon.org","subject":"Re: git-send-pack SIGSEGV..","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-06-15T17:26:43Z","receivedAt":"2007-06-15T17:26:43Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 15 Jun 2007, Daniel Barkalow wrote:\n> \n> Your analysis of the failure was right, though. I reproduced it, and this \n> fixes it for me:\n> \n> --- cut here ---\n> Author: Daniel Barkalow <barkalow@iabervon.org>\n> Date:   Fri Jun 15 10:22:37 2007 -0400\n> \n>     Fix pushing to a pattern with no dst\n>     \n>     Refspecs with no colons are left with no dst value, because they are\n>     interepreted differently for fetch and push. For push, they mean to\n>     reuse the src side. Fix this for patterns.\n>     \n>     Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>\n\nAnd in case anybody cares, I've also verified it, so here's an\n\n\tAcked-by: Linus Torvalds <torvalds@linux-foundation.org>\n\nin case Junio didn't already apply it.\n\n\t\tLinus\n"},{"id":"45156","messageId":"7v7iq47j5z.fsf@assigned-by-dhcp.pobox.com","threadId":"8607","inReplyTo":"Pine.LNX.4.64.0706150949300.5848@iabervon.org","subject":"Re: git-send-pack SIGSEGV..","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-16T02:00:24Z","receivedAt":"2007-06-16T02:00:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n>> I suspect this has an side effect of allowing\n>> \n>> \tfetch = refs/heads/*\n>> \n>> to mean the same thing as\n>> \n>> \tfetch = refs/heads/*:refs/heads/*\n>> \n>> which is suitable for a bare mirroring repository, but I do not\n>> think of any downside, so it might be Ok.\n>> \n>> But that is something from a person who was under anesthesia a\n>> few hours ago, so you should take it with a big grain of salt ;-)\n>\n> Yeah, that's not right; \"push = refs/heads/*\" works like that, but \n> \"fetch = refs/heads/*\" puts them in MERGE_HEAD without storing them \n> anywhere, unlike \"fetch = refs/heads/*:refs/heads/*\".\n\nWhile I appreciate the proper fix, I have to say \"Wait a minute\".\n\nFirst of all, do you really mean MERGE_HEAD in the above?  My\nfix would obviously change the way how \"git fetch\" would STORE\nwhat was fetched when you have \"fetch = refs/heads/*\" line in\nthe configuration file, which is what I already said in my\nmessage.  However, using or not using tracking branches to store\nthe result should never affect if the fetched object is used in\nMERGE_HEAD, so the fix you are responding to should not affect\nwhat goes there at all.  If it does, then there is something\nwrong in the current code.\n\nI did not even realize that \"push/fetch = $prefix/*\" was\nallowed.  IIRC, it has always been \"$prefix1/*:$prefix2/*\" form,\nand \"$prefix/*\" was illegal.  Somehow a change in sematics\nsneaked in without me knowing that makes it legal, and the\nupdated semantics is that for push \"$prefix/*\" is the same as\nlisting every ref under the prefix (without any colon, so it\npushes to the same name), and for fetch \"$prefix/*\" is the same\nas $prefix/$x: (i.e. with colon and empty RHS) for all refs\nunder $prefix.\n\nNow, I am not opposed to _having_ enhanced semantics.  Earlier,\n\"$prefix/*\" was illegal, so it is not like you have introduced\nany incompatibility.  On the push side, I do not think of any\nsane interpretation for it other than treating it exactly as\n\"$prefix/*:$prefix/*\".  On the fetch side, however, I am not\nsure not to use any tracking _and_ grabbing everything is a sane\nsemantics.  You fetch everything under $prefix/ and list them in\nFETCH_HEAD --- now what?  On the other handl, treating it as if\nthe user wrote \"$prefix/*:$prefix/*\" makes some sense.  It is a\nnatural operation to keep a bare mirroring repository up to\ndate.\n\nBut I am somewhat upset about us having the enhanced semantics\nwithout me knowing.  I see two possible reasons for it: (1) I\nwas not being extra careful when I accepted your patches, and\n(2) the change in the semantics were not documented, either in\nthe log nor Documentation/.\n\nWe obviously cannot fix (1) overnight, but could you please at\nleast fix (2)?  git-push.txt and pull-fetch-param.txt would be\nthe two places that talk about \"$prefix1/*:$prefix2/*\", and they\nshould talk about the new \"$prefix/*\" syntax as well.\n\nI think \"fetch = refs/heads/*\" syntax, if it just fetches\nwithout storing, does not make much sense.  In a separate-remote\nrepository, \"[remote \"foo\"] fetch = refs/heads/*\" would probably\nbe useful if we treated it as \"refs/heads/*:refs/remotes/foo/*\".\nIn a bare repository used for mirrors, it would be useful if it\nstood for \"refs/heads/*:refs/heads/*\".\n"},{"id":"45157","messageId":"Pine.LNX.4.64.0706152342490.4740@iabervon.org","threadId":"8607","inReplyTo":"7v7iq47j5z.fsf@assigned-by-dhcp.pobox.com","subject":"Re: git-send-pack SIGSEGV..","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-06-16T04:57:46Z","receivedAt":"2007-06-16T04:57:46Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Fri, 15 Jun 2007, Junio C Hamano wrote:\n\n> Daniel Barkalow <barkalow@iabervon.org> writes:\n> \n> >> I suspect this has an side effect of allowing\n> >> \n> >> \tfetch = refs/heads/*\n> >> \n> >> to mean the same thing as\n> >> \n> >> \tfetch = refs/heads/*:refs/heads/*\n> >> \n> >> which is suitable for a bare mirroring repository, but I do not\n> >> think of any downside, so it might be Ok.\n> >> \n> >> But that is something from a person who was under anesthesia a\n> >> few hours ago, so you should take it with a big grain of salt ;-)\n> >\n> > Yeah, that's not right; \"push = refs/heads/*\" works like that, but \n> > \"fetch = refs/heads/*\" puts them in MERGE_HEAD without storing them \n> > anywhere, unlike \"fetch = refs/heads/*:refs/heads/*\".\n> \n> While I appreciate the proper fix, I have to say \"Wait a minute\".\n> \n> First of all, do you really mean MERGE_HEAD in the above? \n\nSorry, FETCH_HEAD. MERGE_HEAD would be seriously wrong. I was trying to \nsay that it didn't store them in any tracking location, and messed up the \nirrelevant bit of was it does do with them.\n\n> I did not even realize that \"push/fetch = $prefix/*\" was\n> allowed.  IIRC, it has always been \"$prefix1/*:$prefix2/*\" form,\n> and \"$prefix/*\" was illegal. \n\nI thought when I was first writing the remotes.{c,h} code, I found that at \nleast \"push = foo/*\" was supported already, due to nothing actually \nexcluding it, and the code for copying the lhs to a blank rhs was being \napplied to patterns just like non-patterns.\n\nThat is, since \"push = refs/heads/master\" was already the same as \"push = \nrefs/heads/master:refs/heads/master\", the pattern fell into that code. I \nwas at least confidant enough of this to translate --tags into \n\"refs/tags/*\" instead of \"refs/tags/*:refs/tags/*\" when I got to that \npoint.\n\nI'm not sure if \"fetch = foo/*\" was permitted (or is, since fetch is still \nusing other parsing in general), but, whatever it does, if it's allowed, \nit should certainly match \"fetch = refs/head/master\", which is pretty well \nagreed on at this point.\n\n> Somehow a change in sematics sneaked in without me knowing that makes it \n> legal, and the updated semantics is that for push \"$prefix/*\" is the \n> same as listing every ref under the prefix (without any colon, so it\n> pushes to the same name), and for fetch \"$prefix/*\" is the same\n> as $prefix/$x: (i.e. with colon and empty RHS) for all refs\n> under $prefix.\n> \n> Now, I am not opposed to _having_ enhanced semantics.  Earlier,\n> \"$prefix/*\" was illegal, so it is not like you have introduced\n> any incompatibility.  On the push side, I do not think of any\n> sane interpretation for it other than treating it exactly as\n> \"$prefix/*:$prefix/*\".  On the fetch side, however, I am not\n> sure not to use any tracking _and_ grabbing everything is a sane\n> semantics.  You fetch everything under $prefix/ and list them in\n> FETCH_HEAD --- now what?  On the other handl, treating it as if\n> the user wrote \"$prefix/*:$prefix/*\" makes some sense.  It is a\n> natural operation to keep a bare mirroring repository up to\n> date.\n\nI think it could make sense to prepare an octopus merge of a bunch of \nheads in a subdirectory. And it should either be prohibited or match the \nbehavior of a non-pattern, or it would be really confusing.\n\n> But I am somewhat upset about us having the enhanced semantics\n> without me knowing.  I see two possible reasons for it: (1) I\n> was not being extra careful when I accepted your patches, and\n> (2) the change in the semantics were not documented, either in\n> the log nor Documentation/.\n\nActually, I thought I was just overlooking the part of the documentation \nthat explained patterns for push, but it isn't actually there. \nbuiltin-push.c put a lot of effort into supporting them when I got to it, \nbut none of the documentation even suggests that you can put wildcards in \ntwo-sided refspecs for push, let alone one (and the code I was looking at \nwas yours, added without documentation but with a hefty commit message in \nd46ae3f0). Based on the meaning of wildcards in push refspecs, however, \nthe documentation for one-sided refspecs in git-push:\n\n A parameter <ref> without a colon pushes the <ref> from the source \n repository to the destination repository under the same name.\n\nclearly implies that anything that, whatever refs/heads/* matches, it \npushes to the destination under the same name.\n\nOn the fetch side, the code isn't using my parser yet, anyway. But my \nparser should be able to distinguish the one-sided refspec case, so that \nthe fetch logic can do whatever is determined to be right with the \ncombination of features.\n\n> I think \"fetch = refs/heads/*\" syntax, if it just fetches\n> without storing, does not make much sense.  In a separate-remote\n> repository, \"[remote \"foo\"] fetch = refs/heads/*\" would probably\n> be useful if we treated it as \"refs/heads/*:refs/remotes/foo/*\".\n> In a bare repository used for mirrors, it would be useful if it\n> stood for \"refs/heads/*:refs/heads/*\".\n\nI'm kind of uncomfortable with this level of complexity for a fully \nspecified lhs pattern. Maybe \"fetch = refs/heads/*\" should be prohibited, \nwhile something DWIM-y like \"fetch = heads/*\" or even \"fetch = head *\" \ncould come up with clever and useful patterns. In any case, the parser \nshould report \"refs/heads/*\" as pattern,refs/heads/,NULL and let the fetch \ncode decide what to do with it, rather than having the special case in the \nparser.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"45180","messageId":"7vfy4r3915.fsf@assigned-by-dhcp.pobox.com","threadId":"8607","inReplyTo":"Pine.LNX.4.64.0706152342490.4740@iabervon.org","subject":"Re: git-send-pack SIGSEGV..","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-16T21:05:10Z","receivedAt":"2007-06-16T21:05:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> That is, since \"push = refs/heads/master\" was already the same as \"push = \n> refs/heads/master:refs/heads/master\", the pattern fell into that code. I \n> was at least confidant enough of this to translate --tags into \n> \"refs/tags/*\" instead of \"refs/tags/*:refs/tags/*\" when I got to that \n> point.\n\nThis change sounds sensible.\n\n> On the fetch side, the code isn't using my parser yet, anyway. But my \n> parser should be able to distinguish the one-sided refspec case, so that \n> the fetch logic can do whatever is determined to be right with the \n> combination of features.\n\nYes, but at the same time I know you are planning to rewrite\ngit-fetch using this code, so...\n\n>> I think \"fetch = refs/heads/*\" syntax, if it just fetches\n>> without storing, does not make much sense.  In a separate-remote\n>> repository, \"[remote \"foo\"] fetch = refs/heads/*\" would probably\n>> be useful if we treated it as \"refs/heads/*:refs/remotes/foo/*\".\n>> In a bare repository used for mirrors, it would be useful if it\n>> stood for \"refs/heads/*:refs/heads/*\".\n>\n> I'm kind of uncomfortable with this level of complexity for a fully \n> specified lhs pattern. Maybe \"fetch = refs/heads/*\" should be prohibited, \n> while something DWIM-y like \"fetch = heads/*\" or even \"fetch = head *\" \n> could come up with clever and useful patterns. In any case, the parser \n> should report \"refs/heads/*\" as pattern,refs/heads/,NULL and let the fetch \n> code decide what to do with it, rather than having the special case in the \n> parser.\n\nYeah, I am not very happy about DWIMmery either, especially a\nmagic that changes behaviour depending on the bareness [*1*].\nAs long as the parser can distinguish these five cases, I'd be\nhappy.\n\n                        [push]\t\t\t[fetch]\n (1) $any/*\t\tsame name\t\tTBD\n (2) $any/*:\t\terror\t\t\tTBD\n (3) $any1/*:$any2/*\tmap any1 => any2\tmap any1 => any2\n (4) :$any/*\t\terror\t\t\terror\n (5) $any1/*:$any2\terror\t\t\terror\n     $any1:$any2/*\n\n(1-fetch) should probably be an error to avoid newbie confusion.\n\n(2-push) is clearly an error.\n\n(2-fetch) does not make much sense (octopus of random branches\n    you do not even control what they are), but I do not have\n    too strong an objection against it, so marked it TBD for now.\n\n(3) is what we have always done.\n\n(4-fetch) is clearly an error.\n\n(4-push) could be \"remove all refs\", but I'd rather make that an\nerror.\n\n(5) is clearly an error.  If you explicitly have both sides,\n    they should both be wildcard, or non-wildcard.\n\n[Footnote]\n\n*1* Sometimes I see an incorrect suggestion given on #git to do\n\"mv .git ../project.git\" to \"make an incorrectly made repository\nbare\".  This is incorrect---at least you must update core.bare,\nand also should make sure .git/object/info/alternates points at\na right directory if you used a relative path to specify one.\nLuckily, we see much smaller number of such incorrect \"help\" on\nthe mailing list.\n"}]}