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

From: phillip.wood123@gmail.com <phillip.wood123@gmail.com>
Date: Sep 18, 2024, 13:18 UTC
Message-ID: <82a08e44-3822-4a26-ad09-4ca1a9b44c11@gmail.com>
In-Reply-To: <xmqqy13vxx3m.fsf@gitster.g>

On 13/09/2024 18:49, Junio C Hamano wrote:

Show 8 quoted lines

> 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