Re: [PATCH v3 4/4] for-each-ref: avoid filtering on empty pattern

From: Karthik Nayak <karthik.188@gmail.com>
Date: Feb 7, 2024, 16:18 UTC
Message-ID: <CAOLa=ZQzz7_L_9cBmK+pgFwd_DFqfWDVRiaZMAxU+54kBq6Pcw@mail.gmail.com>
In-Reply-To: <xmqq1q9oe029.fsf@gitster.g>

On Wed, Feb 7, 2024 at 5:00 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

>
> Karthik Nayak <karthik.188@gmail.com> writes:
>
> > I'm thinking "--all-ref-types" might be a good alternative. Mostly because,
> > "--include-root-refs" seems very specific to the files backend. Also, we don't
> > include other refs which are not HEAD | pseudorefs, but in the $GIT_DIR.
>
> I strongly disagree wiht the "files backend specific" part of the
> comment.  No matter what backend you would use, refs and pseudorefs
> have the full refname, which may look like "HEAD", "FETCH_HEAD",
> "refs/heads/maint", etc., and you can easily see these full refnames
> form a tree structure, with "HEAD", "FETCH_HEAD", "refs/" at the
> root level.

I conceded to this point, I was thinking "root" here refers to $GIT_DIR and this structuring comes from the files backend. But I see the flaw there that irrelevant of the backend, there is a tree hierarchy built up and for refs without prefixes, it can be considered as "root".

Show 7 quoted lines

> I do not understand your "we don't include other refs", either.
> There may be "things" that are ignored by your implementation of
> "for-each-ref ''" even with the files backend in $GIT_DIR directory.
> They are not refs, because the refs are by definition inside "refs/"
> hierarchy, unless they are ones that are specifically included from
> outside the hierarchy ("pseudorefs" is one class of specific
> exception, "HEAD" is another).

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.