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

From: Karthik Nayak <karthik.188@gmail.com>
Date: Jan 2, 2024, 15:23 UTC
Message-ID: <CAOLa=ZSzmmLTNRX_muNGP7tG65mkpKXfUG9sbNZBL3qv930B8g@mail.gmail.com>
In-Reply-To: <ZY1PLtPue4PgbhwU@tanuki>

Patrick Steinhardt <ps@pks.im> writes:

Show 8 quoted lines

> One interesting question is how we should treat files that look like a
> pseudoref, but which really aren't. I'm not aware of any such files
> written by Git itself, but it could certainly be that a user wrote such
> a file into the repository manually. But given that we're adding new
> behaviour that will be opt-in (e.g. via a new switch) I'd rather err on
> the side of caution and mark any such file as broken instead of silently
> ignoring them.
>

This is definitely tricky, especially since something like
`COMMIT_EDITMSG` passes the `is_pseudoref_syntax()` check and could
simply contain a commit-ish object ID. Here identifying that this is not
a pseudoref is hard when it satisfies both:
1. The general pseudoref syntax
2. Contains the expected file content type

Show 21 quoted lines

>
> Yup, for the reftable we don't have the issue of "How do we detect refs
> dynamically" at all. So I would love for there to be a way to print all
> refs in the refdb, regardless of whether they start with `refs/` or look
> like a pseudoref or whatever else. Otherwise it wouldn't be possible for
> a user to delete anything stored in the refdb that may have a funny
> name, be it intentionally, by accident or due to a bug.
>
> In the reftable backend, the ref iterator's `_advance()` function has a
> hardcoded `starts_with(refname, "refs/")` check. If false, then we'd
> skip the ref in order to retain the same behaviour that the files
> backend has. So maybe what we should be doing is to introduce a new flag
> `DO_FOR_EACH_ALL_REFS` and expose it via git-for-each-ref(1) or
> git-show-ref(1). So:
>
>   - For the reftable backend we'd skip the `starts_with()` check and
>     simply return all refs.
>
>   - For the files backend we'd also iterate through all files in
>     $GIT_DIR and check whether they are pseudoref-like.
>

This makes sense to me and is along what I was considering for the dynamic approach, thanks for writing it down, clarifies my thoughts.