Thread (8 messages) 8 messages, 4 authors, 2024-01-03

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

From: Patrick Steinhardt <hidden>
Date: 2024-01-03 08:52:39

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:
quoted
quoted
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

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help