On Wed, Feb 07, 2024 at 06:02:34PM +0100, Karthik Nayak wrote:
> On Wed, Feb 7, 2024 at 5:46 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> >
> > Karthik Nayak <karthik.188@xxxxxxxxx> writes:
> >
> > > This is a bit of a grey area, what I mean is that we do allow users to create
> > > non "refs/" prefixed refs:
> > >
> > > $ git update-ref foo @~1
> > >
> > > $ cat .git/foo
> > > 2b52187cd2930931c6d34436371f470bb26eef4f
> > >
> > > What I mean to say is that, by saying "--include-root-refs" it seems to imply
> > > that any such refs should be included too, but this simply is not the case.
> >
> > But isn't that a quality of implementation issue? I'd consider it a
> > bug once we have and enforce the definition of what pseudorefs are.
>
> Yeah, that makes sense. I'll use "--include-root-refs" then.
I'd still argue that this is too specific to the files backend. The fact
that we end up only adding root refs to the files backend to me is an
implementation-specific limitation and not a feature. What I really want
is to ask the backend to give me all refs that it knows of for the
current worktree.
The "reftable" backend has this snippet in its iterator:
```
/*
* The files backend only lists references contained in
* "refs/". We emulate the same behaviour here and thus skip
* all references that don't start with this prefix.
*/
if (!starts_with(iter->ref.refname, "refs/"))
continue;
```
If we add the new `--include-root-refs` flag then we would have to
extend it to query whether the ref either starts with "refs/" or whether
it might look like a pseudo-ref:
```
if (!starts_with(iter->ref.refname, "refs/") &&
!(flags & INCLUDE_ROOT_REFS || is_pseudoref(iter->ref.refname)))
continue;
```
The problem I have is that it still wouldn't end up surfacing all refs
which exist in the ref backend while being computationally more
expensive. So the original usecase I had in mind when pitching this
topic isn't actually addressed.
I know that in theory, the reftable backend shouldn't contain refs other
than "refs/" or pseudo-refs anyway. But regardless of that, I think that
formulating this in the form of "root refs" is too restrictive and too
much focussed on the "files" backend. I wouldn't be surprised if there
are ways to have git-update-ref(1) or other tools write refs with
unexpected names, and not being able to learn about those is a
limitation.
Putting this in the context of "Give me all refs which you know of for
the current worktree" is a lot simpler. It would still be equivalent to
"--include-root-refs" for the "files" backend, because the files backend
doesn't have a way to properly track all refs it has ever created. And
for the "reftable" backend it's as simple as this:
```
if (!(iter->flags & DO_FOR_EACH_INCLUDE_ALL_REFS) &&
!starts_with(iter->ref.refname, "refs/"))
```
I'm not sure about better terminology though. Something like
`--include-all-worktree-refs` could easily lead to confusion because it
might make the user think that they also want to list refs from other
worktrees. The best I can come up with is `--include-non-refs` -- pseudo
refs aren't real refs, and neither are refs which don't start with
"refs/".
Patrick
Attachment:
signature.asc
Description: PGP signature
