{"thread":{"id":"11403","subject":"[PATCH] git-filter-branch could be confused by similar names","startedAt":"2007-12-25T14:35:16Z","lastAt":"2008-01-06T09:14:15Z","messageCount":18,"participants":["Dmitry Potapov","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"64095","messageId":"1198593316-7712-1-git-send-email-dpotapov@gmail.com","threadId":"11403","inReplyTo":null,"subject":"[PATCH] git-filter-branch could be confused by similar names","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2007-12-25T14:35:16Z","receivedAt":"2007-12-25T14:35:16Z","isPatch":true,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"'git-filter-branch branch' could fail producing the error:\n\"Which ref do you want to rewrite?\" if existed another branch\nor tag, which name was 'branch-something' or 'something/branch'.\n\nSigned-off-by: Dmitry Potapov <dpotapov@gmail.com>\n---\n git-filter-branch.sh     |    2 +-\n t/t7003-filter-branch.sh |   10 ++++++++++\n 2 files changed, 11 insertions(+), 1 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex dbab1a9..b89a720 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -219,7 +219,7 @@ do\n \t;;\n \t*)\n \t\tref=\"$(git for-each-ref --format='%(refname)' |\n-\t\t\tgrep /\"$ref\")\"\n+\t\t\tgrep '^refs/[^/]\\+/'\"$ref\"'$')\"\n \tesac\n \n \tgit check-ref-format \"$ref\" && echo \"$ref\"\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 5f60b22..c3e5207 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -36,6 +36,16 @@ test_expect_success 'result is really identical' '\n \ttest $H = $(git rev-parse HEAD)\n '\n \n+test_expect_success 'rewrite branch with similar names' '\n+\tgit branch my &&\n+\tgit tag my/orig &&\n+\tgit tag my-orig &&\n+\tgit tag orig/my &&\n+\tgit tag orig-my &&\n+\tgit-filter-branch my &&\n+\ttest $H = $(git rev-parse HEAD)\n+'\n+\n test_expect_success 'rewrite, renaming a specific file' '\n \tgit-filter-branch -f --tree-filter \"mv d doh || :\" HEAD\n '\n-- \n1.5.3.5\n"},{"id":"64194","messageId":"Pine.LNX.4.64.0712292334080.14355@wbgn129.biozentrum.uni-wuerzburg.de","threadId":"11403","inReplyTo":"1198593316-7712-1-git-send-email-dpotapov@gmail.com","subject":"Re: [PATCH] git-filter-branch could be confused by similar names","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-29T22:36:51Z","receivedAt":"2007-12-29T22:36:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 25 Dec 2007, Dmitry Potapov wrote:\n\n> 'git-filter-branch branch' could fail producing the error:\n> \"Which ref do you want to rewrite?\" if existed another branch\n> or tag, which name was 'branch-something' or 'something/branch'.\n> \n> Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>\n> ---\n>  git-filter-branch.sh     |    2 +-\n>  t/t7003-filter-branch.sh |   10 ++++++++++\n>  2 files changed, 11 insertions(+), 1 deletions(-)\n> \n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index dbab1a9..b89a720 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -219,7 +219,7 @@ do\n>  \t;;\n>  \t*)\n>  \t\tref=\"$(git for-each-ref --format='%(refname)' |\n> -\t\t\tgrep /\"$ref\")\"\n> +\t\t\tgrep '^refs/[^/]\\+/'\"$ref\"'$')\"\n\nHmm.  I wonder if this is a proper solution.  It still does not error out \nwhen you have a tag and a branch of the same name.\n\nI kinda hoped that by 1.5.4, rewrite-commits would be finished, but it \nseems that nothing happened in that area after 1.5.3-rcX.\n\nIt would be so much easier to have checks like this -- returning the real \nrefname for short but unique short refnames -- in C.\n\nCiao,\nDscho\n"},{"id":"64203","messageId":"20071230103146.GU13968@dpotapov.dyndns.org","threadId":"11403","inReplyTo":"Pine.LNX.4.64.0712292334080.14355@wbgn129.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-filter-branch could be confused by similar names","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2007-12-30T10:31:46Z","receivedAt":"2007-12-30T10:31:46Z","isPatch":true,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"Hi,\n\nOn Sat, Dec 29, 2007 at 11:36:51PM +0100, Johannes Schindelin wrote:\n> \n> On Tue, 25 Dec 2007, Dmitry Potapov wrote:\n> \n> > 'git-filter-branch branch' could fail producing the error:\n> > \"Which ref do you want to rewrite?\" if existed another branch\n> > or tag, which name was 'branch-something' or 'something/branch'.\n> > \n> > Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>\n> > ---\n> >  git-filter-branch.sh     |    2 +-\n> >  t/t7003-filter-branch.sh |   10 ++++++++++\n> >  2 files changed, 11 insertions(+), 1 deletions(-)\n> > \n> > diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> > index dbab1a9..b89a720 100755\n> > --- a/git-filter-branch.sh\n> > +++ b/git-filter-branch.sh\n> > @@ -219,7 +219,7 @@ do\n> >  \t;;\n> >  \t*)\n> >  \t\tref=\"$(git for-each-ref --format='%(refname)' |\n> > -\t\t\tgrep /\"$ref\")\"\n> > +\t\t\tgrep '^refs/[^/]\\+/'\"$ref\"'$')\"\n> \n> Hmm.  I wonder if this is a proper solution.  It still does not error out \n> when you have a tag and a branch of the same name.\n> \n\nAre you sure? I had created a tag and a branch with the same name, and\nthen tried git filter-branch on it, and it did error out:\n===\nwarning: refname 'test1' is ambiguous.\nWhich ref do you want to rewrite?\n===\n\nMaybe, my fix is not a perfect solution, but it works correctly in all\nknown to me situations, while the original code is clearly broken in\nmost common cases, like when you have created a tag with a name that\nconsists of the name of a branch plus some arbitrary suffix. When you\nrun git-filter-branch on that branch, you only get: \"Which ref do you\nwant to rewrite?\", which is very confusing, because you have only one\nreference with the given name.\n\n\nDmitry\n"},{"id":"64206","messageId":"Pine.LNX.4.64.0712301145360.14355@wbgn129.biozentrum.uni-wuerzburg.de","threadId":"11403","inReplyTo":"20071230103146.GU13968@dpotapov.dyndns.org","subject":"Re: [PATCH] git-filter-branch could be confused by similar names","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-30T10:46:59Z","receivedAt":"2007-12-30T10:46:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 30 Dec 2007, Dmitry Potapov wrote:\n\n> On Sat, Dec 29, 2007 at 11:36:51PM +0100, Johannes Schindelin wrote:\n> > \n> > On Tue, 25 Dec 2007, Dmitry Potapov wrote:\n> > \n> > > 'git-filter-branch branch' could fail producing the error: \"Which \n> > > ref do you want to rewrite?\" if existed another branch or tag, which \n> > > name was 'branch-something' or 'something/branch'.\n> > > \n> > > Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>\n> > > ---\n> > >  git-filter-branch.sh     |    2 +-\n> > >  t/t7003-filter-branch.sh |   10 ++++++++++\n> > >  2 files changed, 11 insertions(+), 1 deletions(-)\n> > > \n> > > diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> > > index dbab1a9..b89a720 100755\n> > > --- a/git-filter-branch.sh\n> > > +++ b/git-filter-branch.sh\n> > > @@ -219,7 +219,7 @@ do\n> > >  \t;;\n> > >  \t*)\n> > >  \t\tref=\"$(git for-each-ref --format='%(refname)' |\n> > > -\t\t\tgrep /\"$ref\")\"\n> > > +\t\t\tgrep '^refs/[^/]\\+/'\"$ref\"'$')\"\n> > \n> > Hmm.  I wonder if this is a proper solution.  It still does not error \n> > out when you have a tag and a branch of the same name.\n> \n> Are you sure? I had created a tag and a branch with the same name, and\n> then tried git filter-branch on it, and it did error out:\n> ===\n> warning: refname 'test1' is ambiguous.\n> Which ref do you want to rewrite?\n> ===\n\nOkay, bad example.  But try \"heads/master\".  Or \"origin\" in a repository \nwhich has \"refs/remotes/origin/HEAD\".\n\nCiao,\nDscho\n"},{"id":"64221","messageId":"20071230135428.GW13968@dpotapov.dyndns.org","threadId":"11403","inReplyTo":"Pine.LNX.4.64.0712301145360.14355@wbgn129.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-filter-branch could be confused by similar names","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2007-12-30T13:54:28Z","receivedAt":"2007-12-30T13:54:28Z","isPatch":true,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"On Sun, Dec 30, 2007 at 11:46:59AM +0100, Johannes Schindelin wrote:\n> \n> On Sun, 30 Dec 2007, Dmitry Potapov wrote:\n> \n> > On Sat, Dec 29, 2007 at 11:36:51PM +0100, Johannes Schindelin wrote:\n> > > \n> > > On Tue, 25 Dec 2007, Dmitry Potapov wrote:\n> > > \n> > > > 'git-filter-branch branch' could fail producing the error: \"Which \n> > > > ref do you want to rewrite?\" if existed another branch or tag, which \n> > > > name was 'branch-something' or 'something/branch'.\n> > > > \n> > > > Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>\n> > > > ---\n> > > >  git-filter-branch.sh     |    2 +-\n> > > >  t/t7003-filter-branch.sh |   10 ++++++++++\n> > > >  2 files changed, 11 insertions(+), 1 deletions(-)\n> > > > \n> > > > diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> > > > index dbab1a9..b89a720 100755\n> > > > --- a/git-filter-branch.sh\n> > > > +++ b/git-filter-branch.sh\n> > > > @@ -219,7 +219,7 @@ do\n> > > >  \t;;\n> > > >  \t*)\n> > > >  \t\tref=\"$(git for-each-ref --format='%(refname)' |\n> > > > -\t\t\tgrep /\"$ref\")\"\n> > > > +\t\t\tgrep '^refs/[^/]\\+/'\"$ref\"'$')\"\n> > > \n> > > Hmm.  I wonder if this is a proper solution.  It still does not error \n> > > out when you have a tag and a branch of the same name.\n> > \n> > Are you sure? I had created a tag and a branch with the same name, and\n> > then tried git filter-branch on it, and it did error out:\n> > ===\n> > warning: refname 'test1' is ambiguous.\n> > Which ref do you want to rewrite?\n> > ===\n> \n> Okay, bad example.  But try \"heads/master\". \n\nYou are right. Somehow, I forgot about this possibility. How about this:\n\n+\t\t\tgrep '^refs/\\([^/]\\+/\\)\\?'\"$ref\"'$')\"\n\n> Or \"origin\" in a repository \n> which has \"refs/remotes/origin/HEAD\".\n\nWell, it does not work, but it would not work before either, because you\nare very likely to have something else in origin. Actually, I doubt that\nanyone will want to filter \"origin\", but if you insist, here is another\ngrep expression, which should accommodate that case too:\n\n+\t\t\tgrep '^refs/\\([^/]\\+/\\)\\?'\"$ref\"'\\(/HEAD\\)\\?$')\"\n\nIn any case, I believe it would be better to have a more strict grep\nexpression than one that is used by git-filter-branch now, because now\nyou either have a very confusing error message, or accidentally you\ncould filter a wrong branch. And as you said before, the proper C\nsolution is not feasible for 1.5.4, so I believe a better grep\nexpression is the right thing to do for now.\n\nIf you have no other objection, I will resent the patch with the\ncorrected version of the grep expression.\n\nDmitry\n"},{"id":"64230","messageId":"Pine.LNX.4.64.0712301700580.14355@wbgn129.biozentrum.uni-wuerzburg.de","threadId":"11403","inReplyTo":"20071230135428.GW13968@dpotapov.dyndns.org","subject":"Re: [PATCH] git-filter-branch could be confused by similar names","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-30T16:03:32Z","receivedAt":"2007-12-30T16:03:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 30 Dec 2007, Dmitry Potapov wrote:\n\n> How about this:\n> \n> +\t\t\tgrep '^refs/\\([^/]\\+/\\)\\?'\"$ref\"'$')\"\n\nMaybe.  I wonder whether just adding a \"$\" (which I obviously forgot) \nwould not be enough...\n\nCiao,\nDscho\n"},{"id":"64233","messageId":"20071230184056.GZ13968@dpotapov.dyndns.org","threadId":"11403","inReplyTo":"Pine.LNX.4.64.0712301700580.14355@wbgn129.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-filter-branch could be confused by similar names","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2007-12-30T18:40:56Z","receivedAt":"2007-12-30T18:40:56Z","isPatch":true,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"On Sun, Dec 30, 2007 at 05:03:32PM +0100, Johannes Schindelin wrote:\n> \n> On Sun, 30 Dec 2007, Dmitry Potapov wrote:\n> \n> > How about this:\n> > \n> > +\t\t\tgrep '^refs/\\([^/]\\+/\\)\\?'\"$ref\"'$')\"\n> \n> Maybe.  I wonder whether just adding a \"$\" (which I obviously forgot) \n> would not be enough...\n\nAdding '$' will certainly make things much better, but you will still\nhave the same problem if you want to filter \"master\", but you have\n\"origin/master\" in your repo.\n\nDmitry\n"},{"id":"64234","messageId":"1199040667-31850-1-git-send-email-dpotapov@gmail.com","threadId":"11403","inReplyTo":"Pine.LNX.4.64.0712301700580.14355@wbgn129.biozentrum.uni-wuerzburg.de","subject":"[PATCH] git-filter-branch could be confused by similar names","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2007-12-30T18:51:07Z","receivedAt":"2007-12-30T18:51:07Z","isPatch":true,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"'git-filter-branch branch' could fail producing the error:\n\"Which ref do you want to rewrite?\" if existed another branch\nor tag, which name was 'branch-something' or 'something/branch'.\n\nSigned-off-by: Dmitry Potapov <dpotapov@gmail.com>\n---\n\nI have corrected my previous patch to allow \"heads\" or \"tags\"\nin the name of a branch or tag, i.e. to write it like this:\n   git filter-branch heads/master\n\n git-filter-branch.sh     |    2 +-\n t/t7003-filter-branch.sh |   10 ++++++++++\n 2 files changed, 11 insertions(+), 1 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex dbab1a9..5de8b12 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -219,7 +219,7 @@ do\n \t;;\n \t*)\n \t\tref=\"$(git for-each-ref --format='%(refname)' |\n-\t\t\tgrep /\"$ref\")\"\n+\t\t\tgrep '^refs/\\([^/]\\+/\\)\\?'\"$ref\"'$')\"\n \tesac\n \n \tgit check-ref-format \"$ref\" && echo \"$ref\"\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 5f60b22..c3e5207 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -36,6 +36,16 @@ test_expect_success 'result is really identical' '\n \ttest $H = $(git rev-parse HEAD)\n '\n \n+test_expect_success 'rewrite branch with similar names' '\n+\tgit branch my &&\n+\tgit tag my/orig &&\n+\tgit tag my-orig &&\n+\tgit tag orig/my &&\n+\tgit tag orig-my &&\n+\tgit-filter-branch my &&\n+\ttest $H = $(git rev-parse HEAD)\n+'\n+\n test_expect_success 'rewrite, renaming a specific file' '\n \tgit-filter-branch -f --tree-filter \"mv d doh || :\" HEAD\n '\n-- \n1.5.3.5\n"},{"id":"64396","messageId":"7v7iiqppkw.fsf@gitster.siamese.dyndns.org","threadId":"11403","inReplyTo":"1199040667-31850-1-git-send-email-dpotapov@gmail.com","subject":"Re: [PATCH] git-filter-branch could be confused by similar names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-03T21:27:27Z","receivedAt":"2008-01-03T21:27:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dmitry Potapov <dpotapov@gmail.com> writes:\n\n> 'git-filter-branch branch' could fail producing the error:\n> \"Which ref do you want to rewrite?\" if existed another branch\n> or tag, which name was 'branch-something' or 'something/branch'.\n>\n> Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>\n> ---\n>\n> I have corrected my previous patch to allow \"heads\" or \"tags\"\n> in the name of a branch or tag, i.e. to write it like this:\n>    git filter-branch heads/master\n>\n>  git-filter-branch.sh     |    2 +-\n>  t/t7003-filter-branch.sh |   10 ++++++++++\n>  2 files changed, 11 insertions(+), 1 deletions(-)\n>\n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index dbab1a9..5de8b12 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -219,7 +219,7 @@ do\n>  \t;;\n>  \t*)\n>  \t\tref=\"$(git for-each-ref --format='%(refname)' |\n> -\t\t\tgrep /\"$ref\")\"\n> +\t\t\tgrep '^refs/\\([^/]\\+/\\)\\?'\"$ref\"'$')\"\n>  \tesac\n\nDo we assume everybody's grep groks ERE these days?  I had an\nimpression that we try to stick to a subset of BRE (namely, no\n\\{m,n\\}, [::], [==], nor [..]).\n\nAlso as a general rule when dealing with refname, we use\nfileglob not regex.\n\nWhat's the goal here?  Is it to make sure given refname is\nunambiguous by being a unique suffix of tags or heads, as in\n\n\ttest $(git show-ref \"$ref\" | wc -l) = 1\n\nor is there anything more going on?\n\nAh, it also wants the full name of the ref.  How about...\n\n\tref=$(git show-ref \"$ref\" | sed -e 's/^.* //')\n\nand have the \"git check-ref-format\" that comes later to issue an\nerror message?        \n\nA better error message would be obtained with perhaps doing\n\n\tLF='\n        '\n        \nat the beginning and then doing:\n\n\tcandidate=$(git show-ref \"$ref\" | sed -e 's/^.* //')\n\tcase \"$candidate\" in\n        '')\n        \tdie \"should not happen -- $ref did not match?\"\n                ;;\n        ?*\"$LF\"?*)\n\t\tdie \"$ref is ambiguous, which one of: $canidate?\"\n\t\t;;\n\tesac\n        ref=$candidate\n"},{"id":"64438","messageId":"20080104155114.GS3373@dpotapov.dyndns.org","threadId":"11403","inReplyTo":"7v7iiqppkw.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-filter-branch could be confused by similar names","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2008-01-04T15:51:15Z","receivedAt":"2008-01-04T15:51:15Z","isPatch":true,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"On Thu, Jan 03, 2008 at 01:27:27PM -0800, Junio C Hamano wrote:\n> Dmitry Potapov <dpotapov@gmail.com> writes:\n> >  \t\tref=\"$(git for-each-ref --format='%(refname)' |\n> > -\t\t\tgrep /\"$ref\")\"\n> > +\t\t\tgrep '^refs/\\([^/]\\+/\\)\\?'\"$ref\"'$')\"\n> >  \tesac\n> \n> Do we assume everybody's grep groks ERE these days?  I had an\n> impression that we try to stick to a subset of BRE (namely, no\n> \\{m,n\\}, [::], [==], nor [..]).\n\nI was not aware about this policy, and I am not aware about\nexisting any grep that does not grok the expressions I used\nabove. So, I thought they are commonly accepted, but I might\nbe wrong.\n\n> \n> Also as a general rule when dealing with refname, we use\n> fileglob not regex.\n\nActually, refname was not meant to be used as regex here, and\nit was written in the hope that there will be no special regex\nsymbols in the refname, but, yes, this use looks like hack.\n\nBTW, accordingly to man, git filter-branch has <rev-list options>,\nand git rev-list described as <commit(s)>, so fileglob may not\nbe used here. I look also at git for-each-ref and git show-ref,\nand though they could have <pattern> as arguments, they meant\ncompletely different by that. git for-each-ref requires the\nfull specification starting with refs but allows fileglob, while\ngit-show-ref does not allow fileglob, but it goes deeper in\nrefs, so it will match with those refs that are inside origin,\nwhich git ref-list does not do. Here are a few examples:\n\n===\n$ git rev-list -1 master\n257f3020f69f3222cdefc1d84b148fb35b2c4f5b\n$ git rev-list -1 heads/master\n257f3020f69f3222cdefc1d84b148fb35b2c4f5b\n$ git rev-list -1 refs/heads/master\n257f3020f69f3222cdefc1d84b148fb35b2c4f5b\n$ git rev-list -1 'refs/heads/maste?'\nfatal: ambiguous argument 'refs/heads/maste?': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions\n$ git rev-list -1 maint\nfatal: ambiguous argument 'maint': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions\n===\n$ git for-each-ref refs/heads/master\n257f3020f69f3222cdefc1d84b148fb35b2c4f5b commit\trefs/heads/master\n$ git for-each-ref refs/heads/maste?\n257f3020f69f3222cdefc1d84b148fb35b2c4f5b commit\trefs/heads/master\n$ git for-each-ref heads/master\n$ git for-each-ref master\n$ git for-each-ref maint\n===\n$ git show-ref refs/heads/master\n257f3020f69f3222cdefc1d84b148fb35b2c4f5b refs/heads/master\n$ git show-ref refs/heads/maste?\n$ git show-ref heads/master\n257f3020f69f3222cdefc1d84b148fb35b2c4f5b refs/heads/master\n$ git show-ref master\n257f3020f69f3222cdefc1d84b148fb35b2c4f5b refs/heads/master\n257f3020f69f3222cdefc1d84b148fb35b2c4f5b refs/remotes/origin/master\n$ git show-ref maint\n4f3d37035a7c735a3b69f962656819f4ff7e4927 refs/remotes/origin/maint\n===\n\n> \n> What's the goal here?  Is it to make sure given refname is\n> unambiguous by being a unique suffix of tags or heads, as in\n> \n> \ttest $(git show-ref \"$ref\" | wc -l) = 1\n\nBecause, I am not the author of this script, I can't be sure,\nbut it seems to me, the goal is to select among all parameters\nonly those that represents tops of branches, for example,\nbeing given: A..B ^C D, we should choose only B and D and\nconvert them into the full refname in the same way as rev-list\ndoes that.\n\n> \n> or is there anything more going on?\n> \n> Ah, it also wants the full name of the ref.  How about...\n> \n> \tref=$(git show-ref \"$ref\" | sed -e 's/^.* //')\n\nIt works only if the name \"unambiguous\" for git show-ref, which\ninterprets refname differently than rev-list as I wrote above.\nNevertheless, I believe we can use 'git show-ref' if we try\nsomething like this:\n\nfor prefix in refs refs/tags refs/heads refs/remote; do\n  fullref=$(git show-ref \"$prefix/$ref\" | sed -e 's/^.* //')\n  test -n \"$fullref\" && break\ndone\nref=\"$fullref\"\n\nIf this idea does not raise any objection, I will test it a bit\nand then send the patch.\n\nDmitry\n"},{"id":"64461","messageId":"7vr6gxjpyn.fsf@gitster.siamese.dyndns.org","threadId":"11403","inReplyTo":"20080104155114.GS3373@dpotapov.dyndns.org","subject":"Re: [PATCH] git-filter-branch could be confused by similar names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-04T20:28:00Z","receivedAt":"2008-01-04T20:28:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dmitry Potapov <dpotapov@gmail.com> writes:\n\n> It works only if the name \"unambiguous\" for git show-ref, which\n> interprets refname differently than rev-list as I wrote above.\n> Nevertheless, I believe we can use 'git show-ref' if we try\n> something like this:\n\nAhh.\n\nBut at that point I would say that exposing the refname dwimming\nlogic to the scripts could be a much cleaner solution.  After\nall, get_sha1_basic() knows what ref it used to come up with the\nanswer, but we are discarding that information.\n\nHow about making \"rev-parse --symbolic-full-name\" give what it\neventually dwimmed the given ref to, perhaps like the attached\npatch?  Then you can do:\n\n  git rev-parse --revs-only --symbolic-full-name \"$@\" >\"$tempdir\"/heads\n\nwithout any need for the loop there in filter-branch.\n\n---\n\n builtin-rev-parse.c |   37 ++++++++++++++++++++++++++++++++++---\n 1 files changed, 34 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-rev-parse.c b/builtin-rev-parse.c\nindex 20d1789..b9af1a5 100644\n--- a/builtin-rev-parse.c\n+++ b/builtin-rev-parse.c\n@@ -21,6 +21,9 @@ static const char *def;\n #define NORMAL 0\n #define REVERSED 1\n static int show_type = NORMAL;\n+\n+#define SHOW_SYMBOLIC_ASIS 1\n+#define SHOW_SYMBOLIC_FULL 2\n static int symbolic;\n static int abbrev;\n static int output_sq;\n@@ -103,8 +106,32 @@ static void show_rev(int type, const unsigned char *sha1, const char *name)\n \n \tif (type != show_type)\n \t\tputchar('^');\n-\tif (symbolic && name)\n-\t\tshow(name);\n+\tif (symbolic && name) {\n+\t\tif (symbolic == SHOW_SYMBOLIC_FULL) {\n+\t\t\tunsigned char discard[20];\n+\t\t\tchar *full;\n+\n+\t\t\tswitch (dwim_ref(name, strlen(name), discard, &full)) {\n+\t\t\tcase 0:\n+\t\t\t\t/*\n+\t\t\t\t * Not found -- not a ref.  We could\n+\t\t\t\t * emit \"name\" here, but symbolic-full\n+\t\t\t\t * users are interested in finding the\n+\t\t\t\t * refs spelled in full, and they would\n+\t\t\t\t * need to filter non-refs if we did so.\n+\t\t\t\t */\n+\t\t\t\tbreak;\n+\t\t\tcase 1: /* happy */\n+\t\t\t\tshow(full);\n+\t\t\t\tbreak;\n+\t\t\tdefault: /* ambiguous */\n+\t\t\t\terror(\"refname '%s' is ambiguous\", name);\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t} else {\n+\t\t\tshow(name);\n+\t\t}\n+\t}\n \telse if (abbrev)\n \t\tshow(find_unique_abbrev(sha1, abbrev));\n \telse\n@@ -421,7 +448,11 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--symbolic\")) {\n-\t\t\t\tsymbolic = 1;\n+\t\t\t\tsymbolic = SHOW_SYMBOLIC_ASIS;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tif (!strcmp(arg, \"--symbolic-full-name\")) {\n+\t\t\t\tsymbolic = SHOW_SYMBOLIC_FULL;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--all\")) {\n"},{"id":"64490","messageId":"7v4pdtgjf0.fsf@gitster.siamese.dyndns.org","threadId":"11403","inReplyTo":"20080104155114.GS3373@dpotapov.dyndns.org","subject":"Re: [PATCH] git-filter-branch could be confused by similar names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-05T01:17:39Z","receivedAt":"2008-01-05T01:17:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dmitry Potapov <dpotapov@gmail.com> writes:\n\n> On Thu, Jan 03, 2008 at 01:27:27PM -0800, Junio C Hamano wrote:\n>> ... I had an\n>> impression that we try to stick to a subset of BRE (namely, no\n>> \\{m,n\\}, [::], [==], nor [..]).\n>\n> I was not aware about this policy, and I am not aware about\n> existing any grep that does not grok the expressions I used\n> above. So, I thought they are commonly accepted, but I might\n> be wrong.\n\nWell I might be wrong too, as I did not vet all the existing use\nof grep in our code.  That's why I said I had \"an impression\".\n\nNow I have (\"git grep 'grep ' -- 'git-*.sh'\"), and it seems to\nbe that we do stick to a narrow subset of BRE.\n\n * We do not use \\{m,n\\};\n\n * We do not use -E;\n\n * We do not use ? nor + (which are \\{0,1\\} and \\{1,\\}\n   respectively in BRE) but that goes without saying as these\n   are ERE elements not BRE (note that \\? and \\+ you wrote are\n   not even part of BRE -- making them accessible from BRE is a\n   GNU extension).\n\nIOW, our scripts' use of grep is very 80'sh ;-)\n\nI do not mind using things that are available in POSIX BRE, but\nlet's not rely on GNU extension that may cause issues to other\npeople.\n\nOther things I noticed while looking at \"t/*.sh\":\n\n * t/t3600-rm.sh and t/5401-update-hooks.sh have unnecessary\n   uses of egrep that can instead be grep;\n\n * t/t3800-mktag.sh uses egrep but I think it can be grep; also,\n   I think the use of temporary file expect.pat is unnecessary.\n\n * t/t5510-fetch.sh does use '^[0-9a-f]\\{40\\} '.\n\n * t/t7001-mv.sh uses -E only to use '.+' when it can just as\n   easily say '..*' instead.\n\n * t/t7600-merge.sh has two occurrences of (GNU extended) \" \\+\"\n   that should be \"  *\" for portability.  An alternative is to\n   use grep -E and say \" +\" instead, but then the other plus\n   sign on the same line needs to be quoted.\n\nOther than the one in 5510 I consider them log hanging fruits\nfor janitors.\n"},{"id":"64535","messageId":"alpine.LSU.1.00.0801051601490.10101@racer.site","threadId":"11403","inReplyTo":"7vr6gxjpyn.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-filter-branch could be confused by similar names","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-01-05T16:03:55Z","receivedAt":"2008-01-05T16:03:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 4 Jan 2008, Junio C Hamano wrote:\n\n> Dmitry Potapov <dpotapov@gmail.com> writes:\n> \n> > It works only if the name \"unambiguous\" for git show-ref, which \n> > interprets refname differently than rev-list as I wrote above. \n> > Nevertheless, I believe we can use 'git show-ref' if we try something \n> > like this:\n> \n> Ahh.\n> \n> But at that point I would say that exposing the refname dwimming\n> logic to the scripts could be a much cleaner solution.\n\nI considered that when ripping the script from cogito, but it seemed to me \nat that time that not requiring an up-to-date git for testing the script \nwould be better.\n\nNow is a different situation, however, so I agree.\n\nCiao,\nDscho\n"},{"id":"64542","messageId":"7vk5mo81in.fsf_-_@gitster.siamese.dyndns.org","threadId":"11403","inReplyTo":"alpine.LSU.1.00.0801051601490.10101@racer.site","subject":"[PATCH 1/2] git-rev-parse --symbolic-full-name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-05T20:23:44Z","receivedAt":"2008-01-05T20:23:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The plumbing level can understand that the user meant\n\"refs/heads/master\" when the user says \"master\" or\n\"heads/master\", but there is no easy way for the scripts to\nfigure it out without duplicating the dwim_ref() logic.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This is the same patch as I showed yesterday but with a doc\n   update.\n\n Documentation/git-rev-parse.txt |    7 +++++++\n builtin-rev-parse.c             |   37 ++++++++++++++++++++++++++++++++++---\n 2 files changed, 41 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt\nindex 329fce0..0cedc13 100644\n--- a/Documentation/git-rev-parse.txt\n+++ b/Documentation/git-rev-parse.txt\n@@ -70,6 +70,13 @@ OPTIONS\n \tpossible '{caret}' prefix); this option makes them output in a\n \tform as close to the original input as possible.\n \n+--symbolic-full-name::\n+\tThis is similar to \\--symbolic, but it omits input that\n+\tare not refs (i.e. branch or tag names; or more\n+\texplicitly disambiguating \"heads/master\" form, when you\n+\twant to name the \"master\" branch when there is an\n+\tunfortunately named tag \"master\"), and show them as full\n+\trefnames (e.g. \"refs/heads/master\").\n \n --all::\n \tShow all refs found in `$GIT_DIR/refs`.\ndiff --git a/builtin-rev-parse.c b/builtin-rev-parse.c\nindex 20d1789..b9af1a5 100644\n--- a/builtin-rev-parse.c\n+++ b/builtin-rev-parse.c\n@@ -21,6 +21,9 @@ static const char *def;\n #define NORMAL 0\n #define REVERSED 1\n static int show_type = NORMAL;\n+\n+#define SHOW_SYMBOLIC_ASIS 1\n+#define SHOW_SYMBOLIC_FULL 2\n static int symbolic;\n static int abbrev;\n static int output_sq;\n@@ -103,8 +106,32 @@ static void show_rev(int type, const unsigned char *sha1, const char *name)\n \n \tif (type != show_type)\n \t\tputchar('^');\n-\tif (symbolic && name)\n-\t\tshow(name);\n+\tif (symbolic && name) {\n+\t\tif (symbolic == SHOW_SYMBOLIC_FULL) {\n+\t\t\tunsigned char discard[20];\n+\t\t\tchar *full;\n+\n+\t\t\tswitch (dwim_ref(name, strlen(name), discard, &full)) {\n+\t\t\tcase 0:\n+\t\t\t\t/*\n+\t\t\t\t * Not found -- not a ref.  We could\n+\t\t\t\t * emit \"name\" here, but symbolic-full\n+\t\t\t\t * users are interested in finding the\n+\t\t\t\t * refs spelled in full, and they would\n+\t\t\t\t * need to filter non-refs if we did so.\n+\t\t\t\t */\n+\t\t\t\tbreak;\n+\t\t\tcase 1: /* happy */\n+\t\t\t\tshow(full);\n+\t\t\t\tbreak;\n+\t\t\tdefault: /* ambiguous */\n+\t\t\t\terror(\"refname '%s' is ambiguous\", name);\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t} else {\n+\t\t\tshow(name);\n+\t\t}\n+\t}\n \telse if (abbrev)\n \t\tshow(find_unique_abbrev(sha1, abbrev));\n \telse\n@@ -421,7 +448,11 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--symbolic\")) {\n-\t\t\t\tsymbolic = 1;\n+\t\t\t\tsymbolic = SHOW_SYMBOLIC_ASIS;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tif (!strcmp(arg, \"--symbolic-full-name\")) {\n+\t\t\t\tsymbolic = SHOW_SYMBOLIC_FULL;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--all\")) {\n-- \n1.5.4.rc2.38.gd6da3\n"},{"id":"64543","messageId":"7vabnk81aw.fsf_-_@gitster.siamese.dyndns.org","threadId":"11403","inReplyTo":"alpine.LSU.1.00.0801051601490.10101@racer.site","subject":"[PATCH 2/2] filter-branch: work correctly with ambiguous refnames","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-05T20:28:23Z","receivedAt":"2008-01-05T20:28:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"'git-filter-branch branch' could fail producing the error:\n\"Which ref do you want to rewrite?\" if existed another branch\nor tag, which name was 'branch-something' or 'something/branch'.\n\n[jc: original report and fix were done between Dmitry Potapov\nand Dscho; I rewrote it using \"rev-parse --symbolic-full-name\"]\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n >> But at that point I would say that exposing the refname dwimming\n >> logic to the scripts could be a much cleaner solution.\n >\n > I considered that when ripping the script from cogito, but it seemed to me \n > at that time that not requiring an up-to-date git for testing the script \n > would be better.\n >\n > Now is a different situation, however, so I agree.\n\n It was already tied to the specific git version when\n git-filter-branch became part of git.git ;-)  \n\n I do not use filter-branch myself very often, but I think this\n is worth fixing.  The additional --no-flags and sed are to deal\n with something like:\n\n\t--topo-order master..next\n\n although I do not offhand know if filter-branch would work with\n things like --topo-order and --first-parent.\n\n git-filter-branch.sh |   22 +++-------------------\n 1 files changed, 3 insertions(+), 19 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex ae29f47..ebf05ca 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -209,25 +209,9 @@ ORIG_GIT_INDEX_FILE=\"$GIT_INDEX_FILE\"\n GIT_WORK_TREE=.\n export GIT_DIR GIT_WORK_TREE\n \n-# These refs should be updated if their heads were rewritten\n-\n-git rev-parse --revs-only --symbolic \"$@\" |\n-while read ref\n-do\n-\t# normalize ref\n-\tcase \"$ref\" in\n-\tHEAD)\n-\t\tref=\"$(git symbolic-ref \"$ref\")\"\n-\t;;\n-\trefs/*)\n-\t;;\n-\t*)\n-\t\tref=\"$(git for-each-ref --format='%(refname)' |\n-\t\t\tgrep /\"$ref\")\"\n-\tesac\n-\n-\tgit check-ref-format \"$ref\" && echo \"$ref\"\n-done > \"$tempdir\"/heads\n+# The refs should be updated if their heads were rewritten\n+git rev-parse --no-flags --revs-only --symbolic-full-name \"$@\" |\n+sed -e '/^^/d' >\"$tempdir\"/heads\n \n test -s \"$tempdir\"/heads ||\n \tdie \"Which ref do you want to rewrite?\"\n-- \n1.5.4.rc2.38.gd6da3\n"},{"id":"64560","messageId":"alpine.LSU.1.00.0801060154150.10101@racer.site","threadId":"11403","inReplyTo":"7vabnk81aw.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] filter-branch: work correctly with ambiguous refnames","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-01-06T01:57:39Z","receivedAt":"2008-01-06T01:57:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 5 Jan 2008, Junio C Hamano wrote:\n\n>  Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n>  > Junio wrote:\n>  >\n>  >> But at that point I would say that exposing the refname dwimming\n>  >> logic to the scripts could be a much cleaner solution.\n>  >\n>  > I considered that when ripping the script from cogito, but it seemed \n>  > to me at that time that not requiring an up-to-date git for testing \n>  > the script would be better.\n>  >\n>  > Now is a different situation, however, so I agree.\n> \n>  It was already tied to the specific git version when\n>  git-filter-branch became part of git.git ;-)\n\nHeh.  But that was not my intention (at least _before_ it was in git.git's \n\"master\"), so that people could test it.\n\n>  I do not use filter-branch myself very often, but I think this\n>  is worth fixing.  The additional --no-flags and sed are to deal\n>  with something like:\n> \n> \t--topo-order master..next\n> \n>  although I do not offhand know if filter-branch would work with\n>  things like --topo-order and --first-parent.\n\nFrankly, I have no idea, but --topo-order _should_ not matter, whereas \n--first-parent _should_ rewrite only commits in the first-parent chain of \nthe given refs.\n\nIn any case, from a cursory look I like the 2 patches (except for the \ncurly brackets around the single-line \"else\" clause, but I know your \nopinion about this, so I will not object).\n\nCiao,\nDscho\n"},{"id":"64565","messageId":"7v8x337jgn.fsf@gitster.siamese.dyndns.org","threadId":"11403","inReplyTo":"alpine.LSU.1.00.0801060154150.10101@racer.site","subject":"Re: [PATCH 2/2] filter-branch: work correctly with ambiguous refnames","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-06T02:53:44Z","receivedAt":"2008-01-06T02:53:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> In any case, from a cursory look I like the 2 patches (except for the \n> curly brackets around the single-line \"else\" clause, but I know your \n> opinion about this, so I will not object).\n\nI care more about consistency across codebase than my own\npreference [*1*].  I just picked the style the kernel folks seem\nto use (see their Documentation/CodingStyle), only because (1)\nthere seem to be people familiar with it, and (2) I am not\nparticularly interested myself in wasting time arguing over\nwhich style is superiour.  I just had to pick one and that was\nthe one I happened to have at hand.\n\nAnd obviously I care more about correctness, so I'd appreciate a\nreview with non cursory look if you have time.\n\n[Footnote]\n\n*1* I favoring shorter code over consistency between when-true\nand when-false clauses.  IOW, I do not like having to have {}\naround a single statement in else clause when if clause needs {}\naround it.\n"},{"id":"64580","messageId":"alpine.LSU.1.00.0801060912310.10101@racer.site","threadId":"11403","inReplyTo":"7v8x337jgn.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] filter-branch: work correctly with ambiguous refnames","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-01-06T09:14:15Z","receivedAt":"2008-01-06T09:14:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Sat, 5 Jan 2008, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > In any case, from a cursory look I like the 2 patches (except for the \n> > curly brackets around the single-line \"else\" clause, but I know your \n> > opinion about this, so I will not object).\n> \n> And obviously I care more about correctness, so I'd appreciate a review \n> with non cursory look if you have time.\n\nI will be in the train for 5.5 hours tomorrow, and hope to do a less \ncursory review then.\n\nCiao,\nDscho\n"}]}