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

[Dmitry Potapov <dpotapov@gmail.com>]

On Thu, Jan 03, 2008 at 01:27:27PM -0800, Junio C Hamano wrote:
> Dmitry Potapov <dpotapov@gmail.com> writes:
> >  		ref="$(git for-each-ref --format='%(refname)' |
> > -			grep /"$ref")"
> > +			grep '^refs/\([^/]\+/\)\?'"$ref"'$')"
> >  	esac
> 
> Do we assume everybody's grep groks ERE these days?  I had an
> impression that we try to stick to a subset of BRE (namely, no
> \{m,n\}, [::], [==], nor [..]).

I was not aware about this policy, and I am not aware about
existing any grep that does not grok the expressions I used
above. So, I thought they are commonly accepted, but I might
be wrong.

> 
> Also as a general rule when dealing with refname, we use
> fileglob not regex.

Actually, refname was not meant to be used as regex here, and
it was written in the hope that there will be no special regex
symbols in the refname, but, yes, this use looks like hack.

BTW, accordingly to man, git filter-branch has <rev-list options>,
and git rev-list described as <commit(s)>, so fileglob may not
be used here. I look also at git for-each-ref and git show-ref,
and though they could have <pattern> as arguments, they meant
completely different by that. git for-each-ref requires the
full specification starting with refs but allows fileglob, while
git-show-ref does not allow fileglob, but it goes deeper in
refs, so it will match with those refs that are inside origin,
which git ref-list does not do. Here are a few examples:

===
$ git rev-list -1 master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b
$ git rev-list -1 heads/master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b
$ git rev-list -1 refs/heads/master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b
$ git rev-list -1 'refs/heads/maste?'
fatal: ambiguous argument 'refs/heads/maste?': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions
$ git rev-list -1 maint
fatal: ambiguous argument 'maint': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions
===
$ git for-each-ref refs/heads/master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b commit	refs/heads/master
$ git for-each-ref refs/heads/maste?
257f3020f69f3222cdefc1d84b148fb35b2c4f5b commit	refs/heads/master
$ git for-each-ref heads/master
$ git for-each-ref master
$ git for-each-ref maint
===
$ git show-ref refs/heads/master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b refs/heads/master
$ git show-ref refs/heads/maste?
$ git show-ref heads/master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b refs/heads/master
$ git show-ref master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b refs/heads/master
257f3020f69f3222cdefc1d84b148fb35b2c4f5b refs/remotes/origin/master
$ git show-ref maint
4f3d37035a7c735a3b69f962656819f4ff7e4927 refs/remotes/origin/maint
===

> 
> What's the goal here?  Is it to make sure given refname is
> unambiguous by being a unique suffix of tags or heads, as in
> 
> 	test $(git show-ref "$ref" | wc -l) = 1

Because, I am not the author of this script, I can't be sure,
but it seems to me, the goal is to select among all parameters
only those that represents tops of branches, for example,
being given: A..B ^C D, we should choose only B and D and
convert them into the full refname in the same way as rev-list
does that.

> 
> or is there anything more going on?
> 
> Ah, it also wants the full name of the ref.  How about...
> 
> 	ref=$(git show-ref "$ref" | sed -e 's/^.* //')

It works only if the name "unambiguous" for git show-ref, which
interprets refname differently than rev-list as I wrote above.
Nevertheless, I believe we can use 'git show-ref' if we try
something like this:

for prefix in refs refs/tags refs/heads refs/remote; do
  fullref=$(git show-ref "$prefix/$ref" | sed -e 's/^.* //')
  test -n "$fullref" && break
done
ref="$fullref"

If this idea does not raise any objection, I will test it a bit
and then send the patch.

Dmitry