Re: [PATCH 2/2] ref-filter: support filtering of operational refs

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 3338 bytes --]

On Tue, Jan 02, 2024 at 01:47:22PM -0500, Taylor Blau wrote:
> On Tue, Jan 02, 2024 at 07:18:48AM -0800, Karthik Nayak wrote:
> > > As "git for-each-ref" takes pattern that is prefix match, e.g.,
> > >
> > >     $ git for-each-ref refs/remotes/
> > >
> > > shows everything like refs/remotes/origin/main that begins with
> > > refs/remotes/, I wonder if
> > >
> > >     $ git for-each-ref ""
> > >
> > > should mean what you are asking for.  After all, "git for-each-ref"
> > > does *not* take "--branches" and others like "git log" family to
> > > limit its operation to subhierarchy of "refs/" to begin with.
> >
> > But I don't think using an empty pattern is the best way to go forward.
> > This would break the pattern matching feature. For instance, what if the
> > user wanted to print all refs, but pattern match "*_HEAD"?
> >
> > Would that be
> >
> >       $ git for-each-ref "" "*_HEAD"
> >
> > I think this would be confusing, since the first pattern is now acting
> > as an option, since its not really filtering rather its changing the
> > search space.
> >
> > Maybe "--all-refs" or "--all-ref-types" instead?
> 
> I tend to agree that the special empty pattern would be a good shorthand
> for listing all references underneath refs/, including any top-level
> psuedo-refs.
> 
> But I don't think that I quite follow what Karthik is saying here.
> for-each-ref returns the union of references that match the given
> pattern(s), not their intersection. So if you wanted to list just the
> psudo-refs ending in '_HEAD', you'd do:
> 
>   $ git for-each-ref "*_HEAD"
> 
> I think if you wanted to list all pseudo-refs, calling the option
> `--pseudo-refs` seems reasonable. But if you want to list some subset of
> psueod-refs matching a given pattern, you should specify that pattern
> directly.

Where I think this proposal falls short is if you have refs outside of
the "refs/" hierarchy. Granted, this is nothing that should usually
happen nowadays. But I think we should safeguard us for the future:

  - There may be bugs in the reftable backend that allow for such refs
    to be created.

  - We may even eventually end up saying that it's valid for refs to not
    start with "refs/". I consider this to be mostly an artifact of how
    the files backend works, so it is not entirely unreasonable for us
    to eventually lift the restriction for the reftable backend.

I do not want to push for the second bullet point anytime soon, nor do I
have any plans to do so in the future. But regardless of that I would
really love to have a way to ask the ref backend for _any_ reference
that it knows of, regardless of its prefix. Otherwise it becomes next to
impossible for a user to learn about what the reftable binary-format
actually contains. So I think that the current focus on pseudo-refs is
too short-sighted, and would want to aim for a more complete solution to
this problem.

This could be in the form of a `--all-refs` flag that gets translated
into a new `DO_FOR_EACH_REF_ALL_REFS` bit, which would indicate to the
ref backend to also enumerate refs outside of the "refs/" hierarchy.
This is orthogonal to the already existing `--all` pseudo-opt, because
`--all` would only ever enumerate refs inside of the "refs/" hierarchy.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]