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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 6, 2023, 22:13 UTC
Message-ID: <xmqqbkm64f3t.fsf@gitster.g>
In-Reply-To: <966cb49c388b652861c773ad7430875bf7896c16.1675529298.git.gitgitgadget@gmail.com>

"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 12 quoted lines

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

Show 12 quoted lines

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