Re: [PATCH] git-filter-branch could be confused by similar names

[Dmitry Potapov <dpotapov@gmail.com>]

On Sun, Dec 30, 2007 at 11:46:59AM +0100, Johannes Schindelin wrote:
> 
> On Sun, 30 Dec 2007, Dmitry Potapov wrote:
> 
> > On Sat, Dec 29, 2007 at 11:36:51PM +0100, Johannes Schindelin wrote:
> > > 
> > > On Tue, 25 Dec 2007, Dmitry Potapov wrote:
> > > 
> > > > 'git-filter-branch branch' could fail producing the error: "Which 
> > > > ref do you want to rewrite?" if existed another branch or tag, which 
> > > > name was 'branch-something' or 'something/branch'.
> > > > 
> > > > Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
> > > > ---
> > > >  git-filter-branch.sh     |    2 +-
> > > >  t/t7003-filter-branch.sh |   10 ++++++++++
> > > >  2 files changed, 11 insertions(+), 1 deletions(-)
> > > > 
> > > > diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> > > > index dbab1a9..b89a720 100755
> > > > --- a/git-filter-branch.sh
> > > > +++ b/git-filter-branch.sh
> > > > @@ -219,7 +219,7 @@ do
> > > >  	;;
> > > >  	*)
> > > >  		ref="$(git for-each-ref --format='%(refname)' |
> > > > -			grep /"$ref")"
> > > > +			grep '^refs/[^/]\+/'"$ref"'$')"
> > > 
> > > Hmm.  I wonder if this is a proper solution.  It still does not error 
> > > out when you have a tag and a branch of the same name.
> > 
> > Are you sure? I had created a tag and a branch with the same name, and
> > then tried git filter-branch on it, and it did error out:
> > ===
> > warning: refname 'test1' is ambiguous.
> > Which ref do you want to rewrite?
> > ===
> 
> Okay, bad example.  But try "heads/master". 

You are right. Somehow, I forgot about this possibility. How about this:

+			grep '^refs/\([^/]\+/\)\?'"$ref"'$')"

> Or "origin" in a repository 
> which has "refs/remotes/origin/HEAD".

Well, it does not work, but it would not work before either, because you
are very likely to have something else in origin. Actually, I doubt that
anyone will want to filter "origin", but if you insist, here is another
grep expression, which should accommodate that case too:

+			grep '^refs/\([^/]\+/\)\?'"$ref"'\(/HEAD\)\?$')"

In any case, I believe it would be better to have a more strict grep
expression than one that is used by git-filter-branch now, because now
you either have a very confusing error message, or accidentally you
could filter a wrong branch. And as you said before, the proper C
solution is not feasible for 1.5.4, so I believe a better grep
expression is the right thing to do for now.

If you have no other objection, I will resent the patch with the
corrected version of the grep expression.

Dmitry