Re: [PATCH 4/4] remote: check branch names

[phillip.wood123@gmail.com]

On 13/09/2024 18:49, Junio C Hamano wrote:
> phillip.wood123@gmail.com writes:
> 
>> Thanks for the patch, I'll re-roll based on that. I wonder if we
>> really want to support "@{-N}" when setting remote tracking branches
>> though - should we be using INTERPRET_BRANCH_REMOTE instead when
>> calling strbuf_branchname()?
> 
> Perhaps.  Users try to use "-" in surprising places, though ;-)

strbuf_check_branch_ref() already rejects "-".

INTERPRET_BRANCH_REMOTE supports @{upstream} which might be useful but 
then we will need to check it refers to the correct remote and expand it 
when setting the fetch refspec so a boolean function to check if a name 
is acceptable is insufficient. Given that "git remote set-branches" has 
only ever supported "real" branch names and patterns on the command line 
and no-one has complained I wonder if we're better off doing something like

	if (strbuf_check_branch_ref(&buf, branch_name) ||
	    strcmp(buf.buf + 11, branch_name))
		error(_("invalid branch name '%s'", branch_name));

where the "buf.buf + 11" skips "refs/heads/"

Best Wishes

Phillip