Subject: Re: [PATCH v4 1/7] refs: accept symref values in `ref_transaction[_add]_update`

From: Jeff King <[email-masked]>
Date: Tue, 30 Apr 2024 06:30:02 -0400

On Mon, Apr 29, 2024 at 10:32:58AM +0100, [email-masked] wrote:

> On 26/04/2024 22:15, Jeff King wrote:
> > On Fri, Apr 26, 2024 at 12:31:36PM -0700, Junio C Hamano wrote:
> > 
> > But the more interesting one is a call to refname_is_safe(), added
> > recently by Phillip's 1dbe401563 (show-ref --verify: accept pseudorefs,
> > 2024-02-07). Looking at that commit, the intent was to allow pseudo-refs
> > by loosening the conditional that checked "HEAD" to allow "FOO_BAR" but
> > not "foobar" outside of "refs/". We enforce the all-caps pseudoref
> > syntax in is_refname_safe().
> > 
> > The proper API there is I think check_ref_format() with ALLOW_ONELEVEL.
> 
> ALLOW_ONELEVEL just disables the check that the refname contains a '/' and I
> think it is aimed at checking branch and tag names without a
> refs/{heads,tags} prefix. If we want to move away from using
> refname_is_safe() perhaps we could add an ALLOW_PSEUDOREF flag that only
> allows the name to contain '[A-Z_]' if there is no '/'.

Right, I think that was the original reason for ALLOW_ONELEVEL. But then
lots of callers ended up using it because it was the only way to allow
"HEAD" or "FETCH_HEAD" to work alongside regular refs.

See the series I posted elsewhere which adds a new flag to cover that
case.

-Peff