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

From: ZheNing Hu <adlternative@gmail.com>
Date: Feb 24, 2023, 06:02 UTC
Message-ID: <CAOLTT8TxtysmA_Ug8-+XB02QLigQnp20eBgdCjHkdZHXidnCmQ@mail.gmail.com>
In-Reply-To: <xmqqbkm64f3t.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> 于2023年2月7日周二 06:13写道：

Show 26 quoted lines

>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> 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.
>

As I mentioned before, the problem we encountered was that the refs/master created unexpectedly cannot be deleted. Additionally, some programs only search for references in the command space of refs/heads/ or refs/tags/, so they may ignore one-level references such as refs/master. Therefore, generally speaking, it's better not to let users create/update special one-level references. This can be achieved directly through git receive-pack or implemented by upper-layer services in positions such as pre-receive-hook.

Certainly, I can remove the previous section as you requested.

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

Thanks.