Re: [PATCH 1/2] show-ref --verify: accept pseudorefs

From: phillip.wood123@gmail.com <phillip.wood123@gmail.com>
Date: Feb 8, 2024, 14:34 UTC
Message-ID: <e137c365-13c6-4ae3-aaf8-0b9d7852e508@gmail.com>
In-Reply-To: <xmqq5xz0b3lu.fsf@gitster.g>

Hi Junio

On 07/02/2024 17:12, Junio C Hamano wrote:

Show 6 quoted lines

> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> I think the helper you picked is the most sensible one, modulo a few
> nits.
> 
>   - We would want to teach refname_is_safe() to honor is_pseudoref()
>     from Karthik's series to make rules more consistent.

Yes, I held off sending this series waiting for a while but then got impatient. We may want to split out a helper from is_pseudoref() that just checks the name without trying to read the ref for callers like this which are going to read the ref anyway.

>   - The refname_is_safe() helper is not just about the stuff at the
>     root level.  While starts_with("refs/") is overly lenient, it
>     rejects nonsense like "refs/../trash".  We would want to lose
>     "starts_with() ||" part of the condition from here.

I left the "starts_with()" in as we check the refname when we look it up with read_ref() so it seemed like wasted effort to do it here as well.

> These are minor non-blocking nits that we should keep in mind only
> for longer term maintenance, #leftoverbits after the dust settles.

> Will queue.

Thanks

Phillip