From: Patrick Steinhardt <ps@pks.im>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 8/8] refs: check refnames as fully qualified when resolving
Date: Tue, 30 Apr 2024 13:25:32 +0200 [thread overview]
Message-ID: <ZjDVLAKA0_4pTAS7@tanuki> (raw)
In-Reply-To: <20240430104152.GF1279403@coredump.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 2858 bytes --]
On Tue, Apr 30, 2024 at 06:41:52AM -0400, Jeff King wrote:
> On Tue, Apr 30, 2024 at 06:54:12AM +0200, Patrick Steinhardt wrote:
>
> > > diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
> > > index 120e1557d7..5fb780cb08 100755
> > > --- a/t/t1430-bad-ref-name.sh
> > > +++ b/t/t1430-bad-ref-name.sh
> > > @@ -400,4 +400,14 @@ test_expect_success 'update-ref refuses non-underscore outside of refs/' '
> > > test_grep "refusing to update ref with bad name" err
> > > '
> > >
> > > +test_expect_success REFFILES 'rev-parse refuses non-pseudoref outside of refs/' '
> > > + git rev-parse HEAD >.git/bad &&
> > > + test_must_fail git rev-parse --verify bad
> > > +'
> > > +
> > > +test_expect_success REFFILES 'rev-parse recognizes non-pseudoref via worktree' '
> > > + git rev-parse HEAD >.git/bad &&
> > > + test_must_fail git rev-parse --verify main-worktree/bad
> > > +'
> >
> > Are these really specific to the REFFILES backend? I would expect that
> > the reftable backend sohuld fail to parse those, too. The fact that we
> > write into the repository directly during the test setup doesn't change
> > this, because all this patch is about is that we don't want to parse
> > random files in the Git repo. And that is something we should want to
> > enforce for all backends.
>
> So this is where I will show my ignorance of reftables. I assume it
> still has to implement FETCH_HEAD as a file (since it holds extra data).
> But does it do the same for other names outside of "refs/"? I am
> assuming not in the paragraph below.
No, that's why we originally introduced the "special refs" syntax, as
defined in gitglossary(7). There are only two files that behave like
refs, but circumvent the ref backend: FETCH_HEAD and MERGE_HEAD. Both of
these have special syntax and carry additional metadata, and as such
they cannot be stored generically in a ref backend.
All other root refs are stored via the ref backend.
> I would expect the test to succeed after my patches on any ref backend,
> since we'd enforce the syntax outside of the backend-specific code. But
> for a backend which does not look for the root name "foo" directly in
> .git/, it is not an interesting test. The looked-for name does not
> exist for it, so even if we did try the lookup, it would fail. We cannot
> distinguish the two cases from the outcome we see.
>
> So I think dropping REFFILES it would still pass, but we are not really
> testing anything that interesting for reftables. That said, I would be
> OK dropping the REFFILES in the name of simplicity and just documenting
> it in the commit message.
Yeah, I'd prefer to drop it. We should only specify the REFFILES prereq
as sparingly as possible to ensure that behaviour is as consistent as
possible across the implementations.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2024-04-30 11:25 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-29 8:35 [PATCH 8/8] refs: check refnames as fully qualified when resolving Jeff King
2024-04-30 4:54 ` Patrick Steinhardt
2024-04-30 10:41 ` Jeff King
2024-04-30 11:25 ` Patrick Steinhardt [this message]
2024-05-03 17:55 ` Jeff King
2024-05-06 6:22 ` Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZjDVLAKA0_4pTAS7@tanuki \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.