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

From: Patrick Steinhardt <ps@pks.im>
Date: Jan 3, 2024, 08:52 UTC
Message-ID: <ZZUgUUlB8A-rhep5@tanuki>
In-Reply-To: <ZZRaOhK869S1Sg1h@nand.local>

On Tue, Jan 02, 2024 at 01:47:22PM -0500, Taylor Blau wrote:

Show 43 quoted lines

> 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