{"thread":{"id":"30233","subject":"[PATCHv2] fetch: Only call a new ref a \"branch\" if it's under refs/heads/.","startedAt":"2012-04-13T17:08:24Z","lastAt":"2012-04-17T22:29:59Z","messageCount":27,"participants":["marcnarc@xiplink.com","Jonathan Nieder","Jeff King","Junio C Hamano","Marc Branchaud","Zbigniew Jędrzejewski-Szmek"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"189199","messageId":"1334336904-18649-1-git-send-email-marcnarc@xiplink.com","threadId":"30233","inReplyTo":null,"subject":"[PATCHv2] fetch: Only call a new ref a \"branch\" if it's under refs/heads/.","fromName":"","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-04-13T17:08:24Z","receivedAt":"2012-04-13T17:08:24Z","isPatch":false,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"From: Marc Branchaud \n\nSigned-off-by: Marc Branchaud \n---\n\nRe-rolled atop of Jens's fix.\n\nThanks guys -- glad I finally itched this scratch!\n\n\t\tM.\n\n\n builtin/fetch.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex cfb43df..b6f737e 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -293,14 +293,18 @@ static int update_local_ref(struct ref *ref,\n \t\tconst char *msg;\n \t\tconst char *what;\n \t\tint r;\n-\t\tif (!strncmp(ref->name, \"refs/tags/\", 10)) {\n+\t\tif (!prefixcmp(ref->name, \"refs/tags/\")) {\n \t\t\tmsg = \"storing tag\";\n \t\t\twhat = _(\"[new tag]\");\n \t\t}\n-\t\telse {\n+\t\telse if (!prefixcmp(ref->name, \"refs/heads/\")) {\n \t\t\tmsg = \"storing head\";\n \t\t\twhat = _(\"[new branch]\");\n \t\t}\n+\t\telse {\n+\t\t\tmsg = \"storing ref\";\n+\t\t\twhat = _(\"[new ref]\");\n+\t\t}\n \n \t\tif ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&\n \t\t (recurse_submodules != RECURSE_SUBMODULES_ON))\n-- \n1.7.10.2.ge89da\n"},{"id":"189213","messageId":"20120413200721.GA3549@burratino","threadId":"30233","inReplyTo":"1334336904-18649-1-git-send-email-marcnarc@xiplink.com","subject":"Re: [PATCHv2] fetch: Only call a new ref a \"branch\" if it's under refs/heads/.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-13T20:07:21Z","receivedAt":"2012-04-13T20:07:21Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"marcnarc@xiplink.com wrote:\n\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -293,14 +293,18 @@ static int update_local_ref(struct ref *ref,\n> \t\tconst char *msg;\n> \t\tconst char *what;\n> \t\tint r;\n> -\t\tif (!strncmp(ref->name, \"refs/tags/\", 10)) {\n> +\t\tif (!prefixcmp(ref->name, \"refs/tags/\")) {\n\nThis part is just a clean-up, right?\n\n> \t\t\tmsg = \"storing tag\";\n> \t\t\twhat = _(\"[new tag]\");\n> \t\t}\n> -\t\telse {\n> +\t\telse if (!prefixcmp(ref->name, \"refs/heads/\")) {\n> \t\t\tmsg = \"storing head\";\n> \t\t\twhat = _(\"[new branch]\");\n> \t\t}\n> +\t\telse {\n> +\t\t\tmsg = \"storing ref\";\n> +\t\t\twhat = _(\"[new ref]\");\n> +\t\t}\n\nNeat. I like it, for what it's worth.\n\nSincerely,\nJonathan\n"},{"id":"189218","messageId":"20120413211350.GD7919@sigill.intra.peff.net","threadId":"30233","inReplyTo":"1334336904-18649-1-git-send-email-marcnarc@xiplink.com","subject":"Re: [PATCHv2] fetch: Only call a new ref a \"branch\" if it's under refs/heads/.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-13T21:13:50Z","receivedAt":"2012-04-13T21:13:50Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 13, 2012 at 01:08:24PM -0400, marcnarc@xiplink.com wrote:\n\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index cfb43df..b6f737e 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -293,14 +293,18 @@ static int update_local_ref(struct ref *ref,\n> \t\tconst char *msg;\n> \t\tconst char *what;\n> \t\tint r;\n> -\t\tif (!strncmp(ref->name, \"refs/tags/\", 10)) {\n> +\t\tif (!prefixcmp(ref->name, \"refs/tags/\")) {\n> \t\t\tmsg = \"storing tag\";\n> \t\t\twhat = _(\"[new tag]\");\n> \t\t}\n> -\t\telse {\n> +\t\telse if (!prefixcmp(ref->name, \"refs/heads/\")) {\n> \t\t\tmsg = \"storing head\";\n> \t\t\twhat = _(\"[new branch]\");\n> \t\t}\n> +\t\telse {\n> +\t\t\tmsg = \"storing ref\";\n> +\t\t\twhat = _(\"[new ref]\");\n> +\t\t}\n\nHmm. The ref->name we are comparing here is the local side. So if I am\nfetching a new branch \"foo\" from the remote into a local\n\"refs/remotes/origin/foo\" tracking ref, it used to say:\n\n From ../parent\n * [new branch] master -> origin/master\n\nNow it says:\n\n From ../parent\n * [new ref] master -> origin/master\n\nwhile refs/remotes/* are not technically branches in our side, I think\nfrom the user's perspective, it is true that we have fetched a branch.\nShould we be calling refs/remotes/* branches, too? Should we be checking\nthe remote's name for the item instead of the local one?\n\n-Peff\n"},{"id":"189220","messageId":"20120413215316.GA19826@burratino","threadId":"30233","inReplyTo":"20120413211350.GD7919@sigill.intra.peff.net","subject":"Re: [PATCHv2] fetch: Only call a new ref a \"branch\" if it's under refs/heads/.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-13T21:53:16Z","receivedAt":"2012-04-13T21:53:16Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> Hmm. The ref->name we are comparing here is the local side. So if I am\n> fetching a new branch \"foo\" from the remote into a local\n> \"refs/remotes/origin/foo\" tracking ref, it used to say:\n>\n> From ../parent\n> * [new branch] master -> origin/master\n>\n> Now it says:\n>\n> From ../parent\n> * [new ref] master -> origin/master\n>\n> while refs/remotes/* are not technically branches in our side, I think\n> from the user's perspective, it is true that we have fetched a branch.\n> Should we be calling refs/remotes/* branches, too? Should we be checking\n> the remote's name for the item instead of the local one?\n\nThe former sounds sensible. Then once the default refspec learns to\nfetch into separate refs/remotes/origin/heads/* and\nrefs/remotes/origin/notes/* namespaces the logic could be updated to\nwrite [new branch] or [new note collection] according to the\nsituation.\n\nJonathan\n"},{"id":"189223","messageId":"7vy5pz1cjk.fsf@alter.siamese.dyndns.org","threadId":"30233","inReplyTo":"20120413215316.GA19826@burratino","subject":"Re: [PATCHv2] fetch: Only call a new ref a \"branch\" if it's under refs/heads/.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-13T22:39:59Z","receivedAt":"2012-04-13T22:39:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Jeff King wrote:\n>\n>> Hmm. The ref->name we are comparing here is the local side. So if I am\n>> fetching a new branch \"foo\" from the remote into a local\n>> \"refs/remotes/origin/foo\" tracking ref, it used to say:\n>>\n>> From ../parent\n>> * [new branch] master -> origin/master\n>>\n>> Now it says:\n>>\n>> From ../parent\n>> * [new ref] master -> origin/master\n>>\n>> while refs/remotes/* are not technically branches in our side, I think\n>> from the user's perspective, it is true that we have fetched a branch.\n>> Should we be calling refs/remotes/* branches, too? Should we be checking\n>> the remote's name for the item instead of the local one?\n>\n> The former sounds sensible. Then once the default refspec learns to\n> fetch into separate refs/remotes/origin/heads/* and\n> refs/remotes/origin/notes/* namespaces the logic could be updated to\n> write [new branch] or [new note collection] according to the\n> situation.\n\nIf we give 'new branch' label for this case because we store it in our\n'refs/remotes/*', a natural extension of it would be to redefine the rule\nto narrow it to 'refs/remotes/*/heads/*' for using 'branch' when we\nintroduce 'new notes collection' label to give refs we are going to store\nin 'refs/remotes/origin/notes/*'. That is consistent with the former.\n\nIf we give 'new branch' label because refs/heads/master on the originating\nend is what is shown on the line, a natural extension would be to use 'new\nnotes collection' label when we are fetching from refs/notes/* on the\noriginating end, and it does not matter where we store it, either our own\nrefs/notes/* or refs/remotes/origin/notes/*. That is consistent with the\nlatter.\n\nThere is no concensus if refs/remotes/origin/notes/* hierarchy is a good\nidea or not, but your argument does not support either side between the\nformer or the latter anyway, so I think it is irrelevant point to raise in\nthis discussion.\n\nThe choice between the two really depends on what information we are\ntrying to convey with this label. Are we saying \"Hey, we now have a new\n'branch' on our side\"? Or are we saying \"We found a new 'branch' over\nthere\"? It is unclear and you can argue both ways. Although I personally\nthink it is the latter, I do not have a strong opinion either way.\n\nI am actually fine with just saying '[new]' without indicating what kind\nat all, because the label is there only to fill the space where old..new\nobject names are usually shown. We don't even say \"[rejected branch]\",\njust \"[rejected]\" in the same place.\n"},{"id":"189405","messageId":"4F8C2C20.6080300@xiplink.com","threadId":"30233","inReplyTo":"20120413200721.GA3549@burratino","subject":"Re: [PATCHv2] fetch: Only call a new ref a \"branch\" if it's under refs/heads/.","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-04-16T14:26:40Z","receivedAt":"2012-04-16T14:26:40Z","isPatch":false,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 12-04-13 04:07 PM, Jonathan Nieder wrote:\n> marcnarc@xiplink.com wrote:\n> \n>> --- a/builtin/fetch.c\n>> +++ b/builtin/fetch.c\n>> @@ -293,14 +293,18 @@ static int update_local_ref(struct ref *ref,\n>> \t\tconst char *msg;\n>> \t\tconst char *what;\n>> \t\tint r;\n>> -\t\tif (!strncmp(ref->name, \"refs/tags/\", 10)) {\n>> +\t\tif (!prefixcmp(ref->name, \"refs/tags/\")) {\n> \n> This part is just a clean-up, right?\n\nYes.\n\nIt seems most of the code in fetch.c uses prefixcmp(), so I thought I'd use\nit for my patch. But then I thought it looked weird for one check to use\nstrncmp() and the other to use prefixcmp().\n\nAfter a cursory search for strncmp() and strcmp() in fetch.c, I think this is\nthe only place where prefixcmp() should've been used but wasn't.\n\nIt didn't seem worth making an extra patch for this change, but I've happy to\nbreak it out if that's what folks want.\n\n>> \t\t\tmsg = \"storing tag\";\n>> \t\t\twhat = _(\"[new tag]\");\n>> \t\t}\n>> -\t\telse {\n>> +\t\telse if (!prefixcmp(ref->name, \"refs/heads/\")) {\n>> \t\t\tmsg = \"storing head\";\n>> \t\t\twhat = _(\"[new branch]\");\n>> \t\t}\n>> +\t\telse {\n>> +\t\t\tmsg = \"storing ref\";\n>> +\t\t\twhat = _(\"[new ref]\");\n>> +\t\t}\n> \n> Neat. I like it, for what it's worth.\n\nThanks!\n\n\t\tM.\n"},{"id":"189409","messageId":"4F8C338D.1050805@xiplink.com","threadId":"30233","inReplyTo":"7vy5pz1cjk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv2] fetch: Only call a new ref a \"branch\" if it's under refs/heads/.","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-04-16T14:58:21Z","receivedAt":"2012-04-16T14:58:21Z","isPatch":false,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 12-04-13 06:39 PM, Junio C Hamano wrote:\n> Jonathan Nieder writes:\n> \n>> Jeff King wrote:\n>>\n>>> Hmm. The ref->name we are comparing here is the local side. So if I am\n>>> fetching a new branch \"foo\" from the remote into a local\n>>> \"refs/remotes/origin/foo\" tracking ref, it used to say:\n>>>\n>>> From ../parent\n>>> * [new branch] master -> origin/master\n>>>\n>>> Now it says:\n>>>\n>>> From ../parent\n>>> * [new ref] master -> origin/master\n>>>\n>>> while refs/remotes/* are not technically branches in our side, I think\n>>> from the user's perspective, it is true that we have fetched a branch.\n>>> Should we be calling refs/remotes/* branches, too? Should we be checking\n>>> the remote's name for the item instead of the local one?\n>>\n>> The former sounds sensible. Then once the default refspec learns to\n>> fetch into separate refs/remotes/origin/heads/* and\n>> refs/remotes/origin/notes/* namespaces the logic could be updated to\n>> write [new branch] or [new note collection] according to the\n>> situation.\n> \n> If we give 'new branch' label for this case because we store it in our\n> 'refs/remotes/*', a natural extension of it would be to redefine the rule\n> to narrow it to 'refs/remotes/*/heads/*' for using 'branch' when we\n> introduce 'new notes collection' label to give refs we are going to store\n> in 'refs/remotes/origin/notes/*'. That is consistent with the former.\n> \n> If we give 'new branch' label because refs/heads/master on the originating\n> end is what is shown on the line, a natural extension would be to use 'new\n> notes collection' label when we are fetching from refs/notes/* on the\n> originating end, and it does not matter where we store it, either our own\n> refs/notes/* or refs/remotes/origin/notes/*. That is consistent with the\n> latter.\n> \n> There is no concensus if refs/remotes/origin/notes/* hierarchy is a good\n> idea or not, but your argument does not support either side between the\n> former or the latter anyway, so I think it is irrelevant point to raise in\n> this discussion.\n> \n> The choice between the two really depends on what information we are\n> trying to convey with this label. Are we saying \"Hey, we now have a new\n> 'branch' on our side\"? Or are we saying \"We found a new 'branch' over\n> there\"? It is unclear and you can argue both ways. Although I personally\n> think it is the latter, I do not have a strong opinion either way.\n\nI think git should describe what it finds in the remote repo, because as a\npublished repo it's refs are more likely to follow the standard layout.\n\nThe local repo is more likely to be configured with a fetch refspec like\n\t+refs/heads/*:refs/crazy/*\nIn such a case there's no point in keying off of the local names.\n\nGit is better off describing what's appeared in the remote repo, and not\nworrying about describing how the user might've mapped those things to local\nrefs.\n\n(That said, patching fetch.c to do that is a bit beyond me at the moment.\nWhere would I find the remote's name for the ref?)\n\n> I am actually fine with just saying '[new]' without indicating what kind\n> at all, because the label is there only to fill the space where old..new\n> object names are usually shown. We don't even say \"[rejected branch]\",\n> just \"[rejected]\" in the same place.\n\nI'd be disappointed if git didn't take the extra step to tell me a bit more\nabout what's going on. I like to see what kinds of new refs the remote has.\n\n\t\tM.\n"},{"id":"189411","messageId":"20120416150036.GA15009@sigill.intra.peff.net","threadId":"30233","inReplyTo":"4F8C338D.1050805@xiplink.com","subject":"Re: [PATCHv2] fetch: Only call a new ref a \"branch\" if it's under refs/heads/.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-16T15:00:36Z","receivedAt":"2012-04-16T15:00:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 16, 2012 at 10:58:21AM -0400, Marc Branchaud wrote:\n\n> Git is better off describing what's appeared in the remote repo, and not\n> worrying about describing how the user might've mapped those things to local\n> refs.\n\nThat's my preference, as well, so it sounds like you, Junio, and I are\nall in agreement.\n\n> (That said, patching fetch.c to do that is a bit beyond me at the moment.\n> Where would I find the remote's name for the ref?)\n\nDuring transfer operations, the remote side of the pair is generally\npointed to by the peer_ref member of \"struct ref\" (so just use\n\"ref->peer_ref->name\" instead of \"ref->name\").\n\n-Peff\n"},{"id":"189419","messageId":"1334591542-25136-1-git-send-email-marcnarc@xiplink.com","threadId":"30233","inReplyTo":"20120416150036.GA15009@sigill.intra.peff.net","subject":"[PATCHv3] fetch: Use the remote's ref name to decide how to describe new refs.","fromName":"","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-04-16T15:52:22Z","receivedAt":"2012-04-16T15:52:22Z","isPatch":false,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"From: Marc Branchaud \n\nAlso, only call a new ref a \"branch\" if it's under refs/heads/.\n\nSigned-off-by: Marc Branchaud \n---\n\nRe-rolled to work with the remote's ref names.\n\nAs before, this is atop of Jens's submodule-recursion fix.\n\nTechnically there are now 3 different changes in this patch:\n\t1. Switch to using remote ref names.\n\t2. Use prefixcomp() consistently.\n\t3. Only call a new ref a \"branch\" if its' under refs/heads.\n\nShould I split this up?\n\n\t\tM.\n\n\n builtin/fetch.c | 13 +++++++++++--\n 1 file changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex cfb43df..063c63b 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -293,14 +293,23 @@ static int update_local_ref(struct ref *ref,\n \t\tconst char *msg;\n \t\tconst char *what;\n \t\tint r;\n-\t\tif (!strncmp(ref->name, \"refs/tags/\", 10)) {\n+\t\t/*\n+\t\t * Nicely describe what we're fetching.\n+\t\t * Base this on the remote's ref names, as they're\n+\t\t * more likely to follow a standard layout.\n+\t\t */\n+\t\tif (!prefixcmp(ref->peer_ref->name, \"refs/tags/\")) {\n \t\t\tmsg = \"storing tag\";\n \t\t\twhat = _(\"[new tag]\");\n \t\t}\n-\t\telse {\n+\t\telse if (!prefixcmp(ref->peer_ref->name, \"refs/heads/\")) {\n \t\t\tmsg = \"storing head\";\n \t\t\twhat = _(\"[new branch]\");\n \t\t}\n+\t\telse {\n+\t\t\tmsg = \"storing ref\";\n+\t\t\twhat = _(\"[new ref]\");\n+\t\t}\n \n \t\tif ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&\n \t\t (recurse_submodules != RECURSE_SUBMODULES_ON))\n-- \n1.7.10.2.ge89da.dirty\n"},{"id":"189427","messageId":"20120416160850.GN5813@burratino","threadId":"30233","inReplyTo":"20120416150036.GA15009@sigill.intra.peff.net","subject":"Re: [PATCHv2] fetch: Only call a new ref a \"branch\" if it's under refs/heads/.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-16T16:08:50Z","receivedAt":"2012-04-16T16:08:50Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n> On Mon, Apr 16, 2012 at 10:58:21AM -0400, Marc Branchaud wrote:\n\n>> Git is better off describing what's appeared in the remote repo, and not\n>> worrying about describing how the user might've mapped those things to local\n>> refs.\n>\n> That's my preference, as well, so it sounds like you, Junio, and I are\n> all in agreement.\n\nMy preference after reading Junio's message was \"[new] \", for what it's worth.\n"},{"id":"189428","messageId":"20120416161048.GO5813@burratino","threadId":"30233","inReplyTo":"1334591542-25136-1-git-send-email-marcnarc@xiplink.com","subject":"Re: [PATCHv3] fetch: Use the remote's ref name to decide how to describe new refs.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-16T16:10:48Z","receivedAt":"2012-04-16T16:10:48Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"marcnarc@xiplink.com wrote:\n\n> Also, only call a new ref a \"branch\" if it's under refs/heads/.\n\nLooks good (though I haven't tested it). Perhaps some new tests would\nhelp make sure this doesn't get broken in the future.\n"},{"id":"189443","messageId":"7vy5pvv9q0.fsf@alter.siamese.dyndns.org","threadId":"30233","inReplyTo":"1334591542-25136-1-git-send-email-marcnarc@xiplink.com","subject":"Re: [PATCHv3] fetch: Use the remote's ref name to decide how to describe new refs.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-16T17:59:35Z","receivedAt":"2012-04-16T17:59:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"marcnarc@xiplink.com writes:\n\n> Re-rolled to work with the remote's ref names.\n>\n> As before, this is atop of Jens's submodule-recursion fix.\n\nThanks; will queue.\n\nAs Jonathan mentioned, it would probably be a good idea to protect this\nwith a new test or two, but I am fine if it is done as a separate patch.\n"},{"id":"189456","messageId":"4F8C7F3A.9070801@xiplink.com","threadId":"30233","inReplyTo":"1334591542-25136-1-git-send-email-marcnarc@xiplink.com","subject":"Re: [PATCHv3] fetch: Use the remote's ref name to decide how to describe new refs.","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-04-16T20:21:14Z","receivedAt":"2012-04-16T20:21:14Z","isPatch":false,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"Apologies - this breaks quite a few tests!\n\nFor example, t1507-rev-parse-upstream.sh test 5 segfaults.\n\nI'll take another stab at it...\n\n\t\tM.\n\n\nOn 12-04-16 11:52 AM, marcnarc@xiplink.com wrote:\n> From: Marc Branchaud \n> \n> Also, only call a new ref a \"branch\" if it's under refs/heads/.\n> \n> Signed-off-by: Marc Branchaud \n> ---\n> \n> Re-rolled to work with the remote's ref names.\n> \n> As before, this is atop of Jens's submodule-recursion fix.\n> \n> Technically there are now 3 different changes in this patch:\n> \t1. Switch to using remote ref names.\n> \t2. Use prefixcomp() consistently.\n> \t3. Only call a new ref a \"branch\" if its' under refs/heads.\n> \n> Should I split this up?\n> \n> \t\tM.\n> \n> \n> builtin/fetch.c | 13 +++++++++++--\n> 1 file changed, 11 insertions(+), 2 deletions(-)\n> \n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index cfb43df..063c63b 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -293,14 +293,23 @@ static int update_local_ref(struct ref *ref,\n> \t\tconst char *msg;\n> \t\tconst char *what;\n> \t\tint r;\n> -\t\tif (!strncmp(ref->name, \"refs/tags/\", 10)) {\n> +\t\t/*\n> +\t\t * Nicely describe what we're fetching.\n> +\t\t * Base this on the remote's ref names, as they're\n> +\t\t * more likely to follow a standard layout.\n> +\t\t */\n> +\t\tif (!prefixcmp(ref->peer_ref->name, \"refs/tags/\")) {\n> \t\t\tmsg = \"storing tag\";\n> \t\t\twhat = _(\"[new tag]\");\n> \t\t}\n> -\t\telse {\n> +\t\telse if (!prefixcmp(ref->peer_ref->name, \"refs/heads/\")) {\n> \t\t\tmsg = \"storing head\";\n> \t\t\twhat = _(\"[new branch]\");\n> \t\t}\n> +\t\telse {\n> +\t\t\tmsg = \"storing ref\";\n> +\t\t\twhat = _(\"[new ref]\");\n> +\t\t}\n> \n> \t\tif ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&\n> \t\t (recurse_submodules != RECURSE_SUBMODULES_ON))\n"},{"id":"189475","messageId":"1334614130-31826-1-git-send-email-marcnarc@xiplink.com","threadId":"30233","inReplyTo":"1334591542-25136-1-git-send-email-marcnarc@xiplink.com","subject":"[PATCHv4 0/3] fetch: Only call a new ref a \"branch\" if it's under refs/heads/.","fromName":"","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-04-16T22:08:47Z","receivedAt":"2012-04-16T22:08:47Z","isPatch":false,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"\nIt turns out that ref->peer_ref is always NULL in update_local_ref(). So I\nmade its caller pass in the full remote ref as a new parameter. I also added\na unit test.\n\nThis series is a complete resend of all the patches discussed in these\nthreads, including Jens's submodule recursion fix.\n\n builtin/fetch.c | 39 +++++++++++++++++++++++++--------------\n t/t5510-fetch.sh | 28 ++++++++++++++++++++++++++++\n 2 files changed, 53 insertions(+), 14 deletions(-)\n\nJens Lehmann (1):\n submodules: recursive fetch also checks new tags for submodule commits\n\nMarc Branchaud (2):\n fetch: Pass both the full remote ref and its short name to update_local_ref().\n fetch: Use the remote's ref name to decide how to describe new refs.\n\n\t\tM.\n"},{"id":"189474","messageId":"1334614130-31826-2-git-send-email-marcnarc@xiplink.com","threadId":"30233","inReplyTo":"1334614130-31826-1-git-send-email-marcnarc@xiplink.com","subject":"[PATCH 1/3] submodules: recursive fetch also checks new tags for submodule commits","fromName":"","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-04-16T22:08:48Z","receivedAt":"2012-04-16T22:08:48Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"From: Jens Lehmann \n\nSince 88a21979c (fetch/pull: recurse into submodules when necessary) all\nfetched commits are examined if they contain submodule changes (unless\nconfiguration or command line options inhibit that). If a newly recorded\nsubmodule commit is not present in the submodule, a fetch is run inside\nit to download that commit.\n\nChecking new refs was done in an else branch where it wasn't executed for\ntags. This normally isn't a problem because tags are only fetched with\nthe branches they live on, then checking the new commits in the fetched\nbranches for submodule commits will also process all tags. But when a\nspecific tag is fetched (or the refspec contains refs/tags/) commits only\nreachable by tags won't be searched for submodule commits, which is a bug.\n\nFix that by moving the code outside the if/else construct to handle new\ntags just like any other ref. The performance impact of adding tags that\nmost of the time lie on a branch which is checked anyway for new submodule\ncommit should be minimal, as since 6859de4 (fetch: avoid quadratic loop\nchecking for updated submodules) all ref-tips are collected first and then\nfed to a single rev-list.\n\nSpotted-by: Jeff King \nSigned-off-by: Jens Lehmann \n---\n builtin/fetch.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 65f5f9b..cfb43df 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -300,11 +300,11 @@ static int update_local_ref(struct ref *ref,\n \t\telse {\n \t\t\tmsg = \"storing head\";\n \t\t\twhat = _(\"[new branch]\");\n-\t\t\tif ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&\n-\t\t\t (recurse_submodules != RECURSE_SUBMODULES_ON))\n-\t\t\t\tcheck_for_new_submodule_commits(ref->new_sha1);\n \t\t}\n \n+\t\tif ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&\n+\t\t (recurse_submodules != RECURSE_SUBMODULES_ON))\n+\t\t\tcheck_for_new_submodule_commits(ref->new_sha1);\n \t\tr = s_update_ref(msg, ref, 0);\n \t\tstrbuf_addf(display, \"%c %-*s %-*s -> %s%s\",\n \t\t\t r ? '!' : '*',\n-- \n1.7.10.2.ge89da.dirty\n"},{"id":"189476","messageId":"1334614130-31826-3-git-send-email-marcnarc@xiplink.com","threadId":"30233","inReplyTo":"1334614130-31826-1-git-send-email-marcnarc@xiplink.com","subject":"[PATCH 2/3] fetch: Pass both the full remote ref and its short name to update_local_ref().","fromName":"","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-04-16T22:08:49Z","receivedAt":"2012-04-16T22:08:49Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"From: Marc Branchaud \n\nSigned-off-by: Marc Branchaud \n---\n builtin/fetch.c | 19 ++++++++++---------\n 1 file changed, 10 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex cfb43df..6c19975 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -239,7 +239,8 @@ static int s_update_ref(const char *action,\n #define REFCOL_WIDTH 10\n \n static int update_local_ref(struct ref *ref,\n-\t\t\t const char *remote,\n+\t\t\t const char *remote_ref_shortname,\n+\t\t\t const struct ref *remote_ref,\n \t\t\t struct strbuf *display)\n {\n \tstruct commit *current = NULL, *updated;\n@@ -256,7 +257,7 @@ static int update_local_ref(struct ref *ref,\n \t\t\tstrbuf_addf(display, \"= %-*s %-*s -> %s\",\n \t\t\t\t TRANSPORT_SUMMARY_WIDTH,\n \t\t\t\t _(\"[up to date]\"), REFCOL_WIDTH,\n-\t\t\t\t remote, pretty_ref);\n+\t\t\t\t remote_ref_shortname, pretty_ref);\n \t\treturn 0;\n \t}\n \n@@ -271,7 +272,7 @@ static int update_local_ref(struct ref *ref,\n \t\tstrbuf_addf(display,\n \t\t\t _(\"! %-*s %-*s -> %s (can't fetch in current branch)\"),\n \t\t\t TRANSPORT_SUMMARY_WIDTH, _(\"[rejected]\"),\n-\t\t\t REFCOL_WIDTH, remote, pretty_ref);\n+\t\t\t REFCOL_WIDTH, remote_ref_shortname, pretty_ref);\n \t\treturn 1;\n \t}\n \n@@ -282,7 +283,7 @@ static int update_local_ref(struct ref *ref,\n \t\tstrbuf_addf(display, \"%c %-*s %-*s -> %s%s\",\n \t\t\t r ? '!' : '-',\n \t\t\t TRANSPORT_SUMMARY_WIDTH, _(\"[tag update]\"),\n-\t\t\t REFCOL_WIDTH, remote, pretty_ref,\n+\t\t\t REFCOL_WIDTH, remote_ref_shortname, pretty_ref,\n \t\t\t r ? _(\" (unable to update local ref)\") : \"\");\n \t\treturn r;\n \t}\n@@ -309,7 +310,7 @@ static int update_local_ref(struct ref *ref,\n \t\tstrbuf_addf(display, \"%c %-*s %-*s -> %s%s\",\n \t\t\t r ? '!' : '*',\n \t\t\t TRANSPORT_SUMMARY_WIDTH, what,\n-\t\t\t REFCOL_WIDTH, remote, pretty_ref,\n+\t\t\t REFCOL_WIDTH, remote_ref_shortname, pretty_ref,\n \t\t\t r ? _(\" (unable to update local ref)\") : \"\");\n \t\treturn r;\n \t}\n@@ -327,7 +328,7 @@ static int update_local_ref(struct ref *ref,\n \t\tstrbuf_addf(display, \"%c %-*s %-*s -> %s%s\",\n \t\t\t r ? '!' : ' ',\n \t\t\t TRANSPORT_SUMMARY_WIDTH, quickref,\n-\t\t\t REFCOL_WIDTH, remote, pretty_ref,\n+\t\t\t REFCOL_WIDTH, remote_ref_shortname, pretty_ref,\n \t\t\t r ? _(\" (unable to update local ref)\") : \"\");\n \t\treturn r;\n \t} else if (force || ref->force) {\n@@ -343,13 +344,13 @@ static int update_local_ref(struct ref *ref,\n \t\tstrbuf_addf(display, \"%c %-*s %-*s -> %s (%s)\",\n \t\t\t r ? '!' : '+',\n \t\t\t TRANSPORT_SUMMARY_WIDTH, quickref,\n-\t\t\t REFCOL_WIDTH, remote, pretty_ref,\n+\t\t\t REFCOL_WIDTH, remote_ref_shortname, pretty_ref,\n \t\t\t r ? _(\"unable to update local ref\") : _(\"forced update\"));\n \t\treturn r;\n \t} else {\n \t\tstrbuf_addf(display, \"! %-*s %-*s -> %s %s\",\n \t\t\t TRANSPORT_SUMMARY_WIDTH, _(\"[rejected]\"),\n-\t\t\t REFCOL_WIDTH, remote, pretty_ref,\n+\t\t\t REFCOL_WIDTH, remote_ref_shortname, pretty_ref,\n \t\t\t _(\"(non-fast-forward)\"));\n \t\treturn 1;\n \t}\n@@ -466,7 +467,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n \n \t\t\tstrbuf_reset(¬e);\n \t\t\tif (ref) {\n-\t\t\t\trc |= update_local_ref(ref, what, ¬e);\n+\t\t\t rc |= update_local_ref(ref, what, rm, ¬e);\n \t\t\t\tfree(ref);\n \t\t\t} else\n \t\t\t\tstrbuf_addf(¬e, \"* %-*s %-*s -> FETCH_HEAD\",\n-- \n1.7.10.2.ge89da.dirty\n"},{"id":"189477","messageId":"1334614130-31826-4-git-send-email-marcnarc@xiplink.com","threadId":"30233","inReplyTo":"1334614130-31826-1-git-send-email-marcnarc@xiplink.com","subject":"[PATCH 3/3] fetch: Use the remote's ref name to decide how to describe new refs.","fromName":"","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-04-16T22:08:50Z","receivedAt":"2012-04-16T22:08:50Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"From: Marc Branchaud \n\nAlso, only call a new ref a \"branch\" if it's under refs/heads/.\n\nSigned-off-by: Marc Branchaud \n---\n builtin/fetch.c | 14 ++++++++++++--\n t/t5510-fetch.sh | 28 ++++++++++++++++++++++++++++\n 2 files changed, 40 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 6c19975..3ede8fd 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -294,14 +294,24 @@ static int update_local_ref(struct ref *ref,\n \t\tconst char *msg;\n \t\tconst char *what;\n \t\tint r;\n-\t\tif (!strncmp(ref->name, \"refs/tags/\", 10)) {\n+\t\t/*\n+\t\t * Nicely describe the new ref we're fetching.\n+\t\t * Base this on the remote's ref name, as it's\n+\t\t * more likely to follow a standard layout.\n+\t\t */\n+\t\tconst char *name = remote_ref ? remote_ref->name : \"\";\n+\t\tif (!prefixcmp(name, \"refs/tags/\")) {\n \t\t\tmsg = \"storing tag\";\n \t\t\twhat = _(\"[new tag]\");\n \t\t}\n-\t\telse {\n+\t\telse if (!prefixcmp(name, \"refs/heads/\")) {\n \t\t\tmsg = \"storing head\";\n \t\t\twhat = _(\"[new branch]\");\n \t\t}\n+\t\telse {\n+\t\t\tmsg = \"storing ref\";\n+\t\t\twhat = _(\"[new ref]\");\n+\t\t}\n \n \t\tif ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&\n \t\t (recurse_submodules != RECURSE_SUBMODULES_ON))\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 308c02e..d17a2cd 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -162,6 +162,34 @@ test_expect_success 'fetch following tags' '\n \n '\n \n+test_expect_success 'fetch uses remote ref names to describe new refs' '\n+\tcd \"$D\" &&\n+\tmkdir descriptive &&\n+\t(\n+\t\tcd descriptive &&\n+\t\tgit init &&\n+\t\tgit config remote.o.url .. &&\n+\t\tgit config remote.o.fetch \"refs/heads/*:refs/crazyheads/*\" &&\n+\t\tgit config --add remote.o.fetch \"refs/others/*:refs/heads/*\" &&\n+\t\tgit fetch o\n+\t) &&\n+\tgit tag -a -m \"Descriptive tag\" descriptive-tag &&\n+\tgit branch descriptive-branch &&\n+\tgit checkout descriptive-branch &&\n+\techo \"Nuts\" >> crazy &&\n+\tgit add crazy &&\n+\tgit commit -a -m \"descriptive commit\" &&\n+\tgit update-ref refs/others/crazy HEAD &&\n+\t(\n+\t\tcd descriptive &&\n+\t\tgit fetch o 2> actual\n+\t\tgrep descriptive-branch actual | grep \"[new branch]\" &&\n+\t\tgrep descriptive-tag actual | grep \"[new tag]\" &&\n+\t\tgrep others/crazy actual | grep \"[new ref]\"\n+\t) &&\n+\tgit checkout master\n+'\n+\n test_expect_success 'fetch must not resolve short tag name' '\n \n \tcd \"$D\" &&\n-- \n1.7.10.2.ge89da.dirty\n"},{"id":"189485","messageId":"20120416223429.GA13440@burratino","threadId":"30233","inReplyTo":"1334614130-31826-4-git-send-email-marcnarc@xiplink.com","subject":"Re: [PATCH 3/3] fetch: Use the remote's ref name to decide how to describe new refs.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-16T22:34:29Z","receivedAt":"2012-04-16T22:34:29Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"marcnarc@xiplink.com wrote:\n\n> +\t\tgrep descriptive-branch actual | grep \"[new branch]\" &&\n> +\t\tgrep descriptive-tag actual | grep \"[new tag]\" &&\n> +\t\tgrep others/crazy actual | grep \"[new ref]\"\n\nCareful. :) I suppose the simplest fix would be to leave out the\nbrackets so the '[new ref]' tag is not misinterpreted as a character\nclass, like so:\n\n\tgrep \"new branch.*descriptive-branch\" actual &&\n\tgrep \"new tag.*descriptive-tag\" actual &&\n\tgrep \"new ref.*others/crazy\" actual\n"},{"id":"189506","messageId":"4F8D2195.9030507@in.waw.pl","threadId":"30233","inReplyTo":"20120416223429.GA13440@burratino","subject":"Re: [PATCH 3/3] fetch: Use the remote's ref name to decide how to describe new refs.","fromName":"Zbigniew Jędrzejewski-Szmek","fromEmail":"zbyszek@in.waw.pl","sentAt":"2012-04-17T07:53:57Z","receivedAt":"2012-04-17T07:53:57Z","isPatch":true,"sender":{"key":"zbyszek@in.waw.pl","avatar":"https://avatars.githubusercontent.com/u/349618?v=4"},"body":"On 04/17/2012 12:34 AM, Jonathan Nieder wrote:\n> marcnarc@xiplink.com wrote:\n>\n>> +\t\tgrep descriptive-branch actual | grep \"[new branch]\"&&\n>> +\t\tgrep descriptive-tag actual | grep \"[new tag]\"&&\n>> +\t\tgrep others/crazy actual | grep \"[new ref]\"\n>\n> Careful. :) I suppose the simplest fix would be to leave out the\n> brackets so the '[new ref]' tag is not misinterpreted as a character\n> class, like so:\n>\n> \tgrep \"new branch.*descriptive-branch\" actual&&\n> \tgrep \"new tag.*descriptive-tag\" actual&&\n> \tgrep \"new ref.*others/crazy\" actual\n\ns/grep/test_i18ngrep/\n\nAlso:\n>> +\techo \"Nuts\" >> crazy &&\n>> +\tgit add crazy &&\n>> +\tgit commit -a -m \"descriptive commit\" &&\n>> +\tgit update-ref refs/others/crazy HEAD &&\n>> +\t(\n>> +\t\tcd descriptive &&\n>> +\t\tgit fetch o 2> actual\nredirections should be without spaces between '>' and the filename\n(>>crazy, 2>actual), for portability.\n\nZbyszek\n"},{"id":"189508","messageId":"20120417075740.GE20017@burratino","threadId":"30233","inReplyTo":"4F8D2195.9030507@in.waw.pl","subject":"Re: [PATCH 3/3] fetch: Use the remote's ref name to decide how to describe new refs.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-17T07:57:40Z","receivedAt":"2012-04-17T07:57:40Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Zbigniew Jędrzejewski-Szmek wrote:\n> On 04/17/2012 12:34 AM, Jonathan Nieder wrote:\n>> marcnarc@xiplink.com wrote:\n\n>>> +\techo \"Nuts\" >> crazy &&\n>>> +\tgit add crazy &&\n>>> +\tgit commit -a -m \"descriptive commit\" &&\n>>> +\tgit update-ref refs/others/crazy HEAD &&\n>>> +\t(\n>>> +\t\tcd descriptive &&\n>>> +\t\tgit fetch o 2> actual\n>\n> redirections should be without spaces between '>' and the filename\n> (>>crazy, 2>actual), for portability.\n\nI think you mean for consistency. A space between the operator and\nfilename is perfectly portable, though git's tests tend to use a\nstyle without the space.\n\nThanks,\nJonathan\n"},{"id":"189512","messageId":"4F8D2C50.3060502@in.waw.pl","threadId":"30233","inReplyTo":"20120417075740.GE20017@burratino","subject":"Re: [PATCH 3/3] fetch: Use the remote's ref name to decide how to describe new refs.","fromName":"Zbigniew Jędrzejewski-Szmek","fromEmail":"zbyszek@in.waw.pl","sentAt":"2012-04-17T08:39:44Z","receivedAt":"2012-04-17T08:39:44Z","isPatch":true,"sender":{"key":"zbyszek@in.waw.pl","avatar":"https://avatars.githubusercontent.com/u/349618?v=4"},"body":"On 04/17/2012 09:57 AM, Jonathan Nieder wrote:\n> Zbigniew Jędrzejewski-Szmek wrote:\n>> On 04/17/2012 12:34 AM, Jonathan Nieder wrote:\n>>> marcnarc@xiplink.com wrote:\n>\n>>>> +\techo \"Nuts\" >> crazy &&\n>>>> +\tgit add crazy &&\n>>>> +\tgit commit -a -m \"descriptive commit\" &&\n>>>> +\tgit update-ref refs/others/crazy HEAD &&\n>>>> +\t(\n>>>> +\t\tcd descriptive &&\n>>>> +\t\tgit fetch o 2> actual\n>>\n>> redirections should be without spaces between '>' and the filename\n>> (>>crazy, 2>actual), for portability.\n>\n> I think you mean for consistency. A space between the operator and\n> filename is perfectly portable, though git's tests tend to use a\n> style without the space.\nYes, you're right. I mixed up the motivation for two different rules...\n\nQuoting Junio C Hamano:\n> - Strictly speaking, the target of I/O redirection (e.g. >\"$name\") does\n> not have to have quotes around it, but some versions of bash are known\n> to give misguided warnings against it;\n>\n> - We do not write SP between the redirection and filename, but we do have\n> one SP before the redirection; and\nSo rule #2 is for consistency, rule #1 for portability.\n\n-\nZbyszek\n"},{"id":"189524","messageId":"4F8D7CD5.3080508@xiplink.com","threadId":"30233","inReplyTo":"4F8D2195.9030507@in.waw.pl","subject":"Re: [PATCH 3/3] fetch: Use the remote's ref name to decide how to describe new refs.","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-04-17T14:23:17Z","receivedAt":"2012-04-17T14:23:17Z","isPatch":true,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 12-04-17 03:53 AM, Zbigniew Jędrzejewski-Szmek wrote:\n> On 04/17/2012 12:34 AM, Jonathan Nieder wrote:\n>> marcnarc@xiplink.com wrote:\n>>\n>>> + grep descriptive-branch actual | grep \"[new branch]\"&&\n>>> + grep descriptive-tag actual | grep \"[new tag]\"&&\n>>> + grep others/crazy actual | grep \"[new ref]\"\n>>\n>> Careful. :) I suppose the simplest fix would be to leave out the\n>> brackets so the '[new ref]' tag is not misinterpreted as a character\n>> class, like so:\n>>\n>> grep \"new branch.*descriptive-branch\" actual&&\n>> grep \"new tag.*descriptive-tag\" actual&&\n>> grep \"new ref.*others/crazy\" actual\n> \n> s/grep/test_i18ngrep/\n\nWith the switch to test_i18ngrep do I have to worry about possible regexp\nmisinterpretations of \"[new branch]\" et al?\n\nJonathan, I figured regexps aren't an issue with plain \"grep\" (unlike \"egrep\"\nor \"grep -e\"). I take it this is a portability concern -- are there systems\nthat actually replace plain \"grep\" like a \"egrep\"?\n\nAlso, in my test's pipes I believe only the second \"grep\" needs to be\n\"test_i18ngrep\", right? (Only strings like \"[new branch]\" are\ninternationalized.)\n\n> Also:\n>>> + echo \"Nuts\" >> crazy &&\n>>> + git add crazy &&\n>>> + git commit -a -m \"descriptive commit\" &&\n>>> + git update-ref refs/others/crazy HEAD &&\n>>> + (\n>>> + cd descriptive &&\n>>> + git fetch o 2> actual\n> redirections should be without spaces between '>' and the filename\n> (>>crazy, 2>actual), for portability.\n\nConsistency; got it.\n\nThanks guys!\n\n\t\tM.\n"},{"id":"189525","messageId":"20120417151826.GA21534@burratino","threadId":"30233","inReplyTo":"4F8D7CD5.3080508@xiplink.com","subject":"Re: [PATCH 3/3] fetch: Use the remote's ref name to decide how to describe new refs.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-17T15:18:27Z","receivedAt":"2012-04-17T15:18:27Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi again,\n\nMarc Branchaud wrote:\n\n> Jonathan, I figured regexps aren't an issue with plain \"grep\" (unlike \"egrep\"\n> or \"grep -e\"). I take it this is a portability concern -- are there systems\n> that actually replace plain \"grep\" like a \"egrep\"?\n\nNo, \"grep\" always means to globally match against a regexp and print.\nThere is an \"fgrep\" command to search for a fixed string, but why not\ntake advantage of regexps while they're available and use one grep\ninstead of two?\n\n> Also, in my test's pipes I believe only the second \"grep\" needs to be\n> \"test_i18ngrep\", right? (Only strings like \"[new branch]\" are\n> internationalized.)\n\nI'm not sure what the point would be. The exit status from the\nupstream of a pipe doesn't affect the outcome of the test, so no one\nwould be able to tell the difference.\n\n[...]\n> Thanks guys!\n\nThank you.\n\nSincerely,\nJonathan\n"},{"id":"189526","messageId":"7vbomqpef1.fsf@alter.siamese.dyndns.org","threadId":"30233","inReplyTo":"1334614130-31826-1-git-send-email-marcnarc@xiplink.com","subject":"Re: [PATCHv4 0/3] fetch: Only call a new ref a \"branch\" if it's under refs/heads/.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-17T15:26:58Z","receivedAt":"2012-04-17T15:26:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"marcnarc@xiplink.com writes:\n\n> It turns out that ref->peer_ref is always NULL in update_local_ref(). So I\n> made its caller pass in the full remote ref as a new parameter. I also added\n> a unit test.\n>\n> This series is a complete resend of all the patches discussed in these\n> threads, including Jens's submodule recursion fix.\n\nThanks, but I'd prefer to keep unrelated things as separate unless there\nis a compelling reason not to.\n\nAlso I do not think renaming of the existing parameter in the first patch\nis warranted, especially when the new parameter you are adding is more\ndescriptive (i.e. \"remote_ref\" in the context of that function makes it\nclear enough that it is not just a string but is a pointer to a ref\nstructure).\n\nSo let's do this.\n\n-- >8 --\nFrom: Marc Branchaud \nDate: Mon, 16 Apr 2012 18:08:49 -0400\nSubject: [PATCH 1/2] fetch: Give remote_ref to update_local_ref() as well\n\nThis way, the function can look at the remote side to adjust the\ninformational message it gives.\n\nSigned-off-by: Marc Branchaud \nSigned-off-by: Junio C Hamano \n---\n builtin/fetch.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 8ec4eae..06d71e4 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -240,6 +240,7 @@ static int s_update_ref(const char *action,\n \n static int update_local_ref(struct ref *ref,\n \t\t\t const char *remote,\n+\t\t\t const struct ref *remote_ref,\n \t\t\t struct strbuf *display)\n {\n \tstruct commit *current = NULL, *updated;\n@@ -466,7 +467,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n \n \t\t\tstrbuf_reset(¬e);\n \t\t\tif (ref) {\n-\t\t\t\trc |= update_local_ref(ref, what, ¬e);\n+\t\t\t\trc |= update_local_ref(ref, what, rm, ¬e);\n \t\t\t\tfree(ref);\n \t\t\t} else\n \t\t\t\tstrbuf_addf(¬e, \"* %-*s %-*s -> FETCH_HEAD\",\n-- \n1.7.10.332.g1863c\n"},{"id":"189527","messageId":"7v7gxepecd.fsf@alter.siamese.dyndns.org","threadId":"30233","inReplyTo":"7vbomqpef1.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv4 0/3] fetch: Only call a new ref a \"branch\" if it's under refs/heads/.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-17T15:28:34Z","receivedAt":"2012-04-17T15:28:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Marc Branchaud \nDate: Mon, 16 Apr 2012 18:08:50 -0400\nSubject: [PATCH 2/2] fetch: describe new refs based on where it came from\n\nupdate_local_ref() used to say \"[new branch]\" when we stored a new ref\noutside refs/tags/ hierarchy, but the message is more about what we\nfetched, so use the refname at the origin to make that decision.\n\nAlso, only call a new ref a \"branch\" if it's under refs/heads/.\n\nSigned-off-by: Marc Branchaud \nSigned-off-by: Junio C Hamano \n---\n\n * And this is a re-roll of yours on top of the updated 1/2; I also\n fixed the new test here and there.\n\n builtin/fetch.c | 14 +++++++++++---\n t/t5510-fetch.sh | 30 ++++++++++++++++++++++++++++++\n 2 files changed, 41 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 06d71e4..0f80cf8 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -294,16 +294,24 @@ static int update_local_ref(struct ref *ref,\n \t\tconst char *msg;\n \t\tconst char *what;\n \t\tint r;\n-\t\tif (!strncmp(ref->name, \"refs/tags/\", 10)) {\n+\t\t/*\n+\t\t * Nicely describe the new ref we're fetching.\n+\t\t * Base this on the remote's ref name, as it's\n+\t\t * more likely to follow a standard layout.\n+\t\t */\n+\t\tconst char *name = remote_ref ? remote_ref->name : \"\";\n+\t\tif (!prefixcmp(name, \"refs/tags/\")) {\n \t\t\tmsg = \"storing tag\";\n \t\t\twhat = _(\"[new tag]\");\n-\t\t}\n-\t\telse {\n+\t\t} else if (!prefixcmp(name, \"refs/heads/\")) {\n \t\t\tmsg = \"storing head\";\n \t\t\twhat = _(\"[new branch]\");\n \t\t\tif ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&\n \t\t\t (recurse_submodules != RECURSE_SUBMODULES_ON))\n \t\t\t\tcheck_for_new_submodule_commits(ref->new_sha1);\n+\t\t} else {\n+\t\t\tmsg = \"storing ref\";\n+\t\t\twhat = _(\"[new ref]\");\n \t\t}\n \n \t\tr = s_update_ref(msg, ref, 0);\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 308c02e..d7a19a1 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -162,6 +162,36 @@ test_expect_success 'fetch following tags' '\n \n '\n \n+test_expect_success 'fetch uses remote ref names to describe new refs' '\n+\tcd \"$D\" &&\n+\tgit init descriptive &&\n+\t(\n+\t\tcd descriptive &&\n+\t\tgit config remote.o.url .. &&\n+\t\tgit config remote.o.fetch \"refs/heads/*:refs/crazyheads/*\" &&\n+\t\tgit config --add remote.o.fetch \"refs/others/*:refs/heads/*\" &&\n+\t\tgit fetch o\n+\t) &&\n+\tgit tag -a -m \"Descriptive tag\" descriptive-tag &&\n+\tgit branch descriptive-branch &&\n+\tgit checkout descriptive-branch &&\n+\techo \"Nuts\" >crazy &&\n+\tgit add crazy &&\n+\tgit commit -a -m \"descriptive commit\" &&\n+\tgit update-ref refs/others/crazy HEAD &&\n+\t(\n+\t\tcd descriptive &&\n+\t\tgit fetch o 2>actual &&\n+\t\tgrep \" -> refs/crazyheads/descriptive-branch$\" actual |\n+\t\ttest_i18ngrep \"new branch\" &&\n+\t\tgrep \" -> descriptive-tag$\" actual |\n+\t\ttest_i18ngrep \"new tag\" &&\n+\t\tgrep \" -> crazy$\" actual |\n+\t\ttest_i18ngrep \"new ref\"\n+\t) &&\n+\tgit checkout master\n+'\n+\n test_expect_success 'fetch must not resolve short tag name' '\n \n \tcd \"$D\" &&\n-- \n1.7.10.332.g1863c\n"},{"id":"189558","messageId":"4F8DC4CD.9090106@xiplink.com","threadId":"30233","inReplyTo":"7vbomqpef1.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv4 0/3] fetch: Only call a new ref a \"branch\" if it's under refs/heads/.","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2012-04-17T19:30:21Z","receivedAt":"2012-04-17T19:30:21Z","isPatch":false,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 12-04-17 11:26 AM, Junio C Hamano wrote:\n> marcnarc@xiplink.com writes:\n> \n>> It turns out that ref->peer_ref is always NULL in update_local_ref(). So I\n>> made its caller pass in the full remote ref as a new parameter. I also added\n>> a unit test.\n>>\n>> This series is a complete resend of all the patches discussed in these\n>> threads, including Jens's submodule recursion fix.\n> \n> Thanks, but I'd prefer to keep unrelated things as separate unless there\n> is a compelling reason not to.\n> \n> Also I do not think renaming of the existing parameter in the first patch\n> is warranted, especially when the new parameter you are adding is more\n> descriptive (i.e. \"remote_ref\" in the context of that function makes it\n> clear enough that it is not just a string but is a pointer to a ref\n> structure).\n> \n> So let's do this.\n\nI'm good with both your patches. Thanks much for fixing my work!\n\n\t\tM.\n"},{"id":"189585","messageId":"20120417222959.GD10797@sigill.intra.peff.net","threadId":"30233","inReplyTo":"1334614130-31826-1-git-send-email-marcnarc@xiplink.com","subject":"Re: [PATCHv4 0/3] fetch: Only call a new ref a \"branch\" if it's under refs/heads/.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-17T22:29:59Z","receivedAt":"2012-04-17T22:29:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 16, 2012 at 06:08:47PM -0400, marcnarc@xiplink.com wrote:\n\n> \n> It turns out that ref->peer_ref is always NULL in update_local_ref(). So I\n> made its caller pass in the full remote ref as a new parameter. I also added\n> a unit test.\n\nHrm. So yeah, it is because the \"struct ref\" we create is not from the\nref_map, but is a newly created ref based on the local side (and I had\nmy refs backwards before; the remote ref is the \"real\" ref, and the\nlocal version is found in peer_ref, not the other way around).\n\nBut I couldn't help but notice that store_updated_refs already has this\nexact same \"what do we call it to the user\" logic in it, which is what\ngoes into FETCH_HEAD. Shouldn't we just be passing this \"kind\" flag down\nto update_local_ref, and then this copy of the logic can go away\nentirely?\n\n-Peff\n"}]}