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

From: phillip.wood123@gmail.com <phillip.wood123@gmail.com>
Date: Apr 29, 2024, 09:32 UTC
Message-ID: <b2977495-3b4a-4f2a-85dc-d0568f34f2e9@gmail.com>
In-Reply-To: <20240426211529.GD13703@coredump.intra.peff.net>

Hi Peff

On 26/04/2024 22:15, Jeff King wrote:

Show 10 quoted lines

> 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 '/'.

Best Wishes

Phillip