Re: [PATCH 4/8] interpret_branch_name: allow callers to restrict expansions

[Jeff King <peff@peff.net>]

On Tue, Feb 28, 2017 at 12:27:45PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > The original purpose of interpret_branch_name() was to be used by
> > get_sha1() in resolving refs.  As such, some of its expansions may
> > point to refs outside of the local "refs/heads" namespace.
> 
> I am not sure the reference to "get_sha1()" is entirely correct.
> 
> Until it was renamed at 431b1969fc ("Rename interpret/substitute
> nth_last_branch functions", 2009-03-21), the function was called
> interpret_nth_last_branch() which was originally introduced for the
> name, not sha1, at ae5a6c3684 ("checkout: implement "@{-N}" shortcut
> name for N-th last branch", 2009-01-17).  The use of the same syntax
> and function for the object name came a bit later.
> 
> But I think that is an insignificant detail.  Let's read on.

Yeah, sorry, I was lazy about digging up the history. I think the
problem actually started in ae0ba8e20a (Teach @{upstream} syntax to
strbuf_branchanme(), 2010-01-19), when the features were ported over
from get_sha1() to interpret_branch_name().

Since I need to re-roll anyway, I'll tweak this to be more accurate.

> > @@ -405,7 +405,7 @@ int refname_match(const char *abbrev_name, const char *full_name)
> >  static char *substitute_branch_name(const char **string, int *len)
> >  {
> >  	struct strbuf buf = STRBUF_INIT;
> > -	int ret = interpret_branch_name(*string, *len, &buf);
> > +	int ret = interpret_branch_name(*string, *len, &buf, 0);
> >  
> >  	if (ret == *len) {
> >  		size_t size;
> 
> This is the one used by dwim_ref/log, so we'd need to allow it to
> resolve to anything, e.g. "@" -> "HEAD", and pretend that the user
> typed that expansion.  OK.

Yeah. Left them all as "0" here, and then split the updates into their
own commits. So there's no commit that says "and we are leaving this
spot, because it is correct as-is". The other notable one is the
strbuf_branchname() call in merge_name().

I can mention those in the commit message here (I think I did in the
cover letter, but it would be nice to stick it in the history, since
that will be what comes up if you blame those lines).

-Peff