On Mon, Apr 29, 2024 at 09:18:47AM -0700, Junio C Hamano wrote:
> phillip.wood123@xxxxxxxxx writes:
>
> > 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 '/'.
>
> Makes sense.
>
> I wonder if we eventually can get rid of ALLOW_ONELEVEL, though. If
> all callers that use ALLOW_ONELEVEL know under which prefix they
> plan to hang the refname they are checking (if the refname passed
> the check), we can force the check to be performed always on the
> full refname, and it will become easier to make the check more
> consistent---as the check will have full context information.
>
> For example, with ALLOW_ONELEVEL the check may say "HEAD" is OK, but
> if we get rid of ALLOW_ONELEVEL and force the callers to always test
> the full refname, we may say "refs/heads/HEAD" is not acceptable,
> neither is "refs/tags/HEAD", but "refs/remotes/origin/HEAD" is good.
One case I ran into while working on my series is refspec parsing, where
we feed the left and right halves to check_refname_format(), albeit with
a special flag that allows the "*" wildcard character. And there we are
OK with one-level names because they will be passed through the
dwim_ref() lookup.
I don't know if there are other spots that do something similar. Most of
them would, I imagine, just take any input and leave it to the ref code
to enforce syntax after the dwim-ref has happened.
-Peff
