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

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

Jeff King [off-list ref] writes:

quoted

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.

quoted

@@ -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