Subject: Re: [PATCH v2 2/2] [RFC] push: allow delete one level ref

From: Junio C Hamano <[email-masked]>
Date: Mon, 06 Feb 2023 14:13:26 -0800

"ZheNing Hu via GitGitGadget" <[email-masked]> writes:

> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 13ff9fae3ba..77088f5ba8d 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -1463,7 +1463,9 @@ static const char *update(struct command *cmd, struct shallow_info *si)
>  		find_shared_symref(worktrees, "HEAD", name);
>  
>  	/* only refs/... are allowed */
> -	if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) {
> +	if (!starts_with(name, "refs/") ||
> +	    check_refname_format(name + 5, is_null_oid(new_oid) ?
> +				 REFNAME_ALLOW_ONELEVEL : 0)) {

I am somehow smelling that this is about "refs/stash" and it may be
a good thing to allow removing a leftover refs/stash remotely.  I am
not sure why we need to treat it differently while still blocking
update/modification.

Is it that we are actively discouraging one-level refs like
refs/stash, so removing is fine but once removed we do not allow
creating or updating?  It somewhat smells a hard to explain rule to
the users, at least to me.

> diff --git a/connect.c b/connect.c
> index 63e59641c0d..7a396ad72e9 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -30,7 +30,8 @@ static int check_ref(const char *name, unsigned int flags)
>  		return 0;
>  
>  	/* REF_NORMAL means that we don't want the magic fake tag refs */
> -	if ((flags & REF_NORMAL) && check_refname_format(name, 0))
> +	if ((flags & REF_NORMAL) && check_refname_format(name,
> +							 REFNAME_ALLOW_ONELEVEL))
>  		return 0;

This side of the change does make sense.

Thanks.