Am Wed, Jul 05, 2023 at 10:16:17AM -0700 schrieb Junio C Hamano:
> > The one thing that would actually make us a lot faster (by using the
> > packed-refs peels) is to make full peels the only option, and do not
> > bother letting --points-at match "B" in an A->B->C peel. But that would
> > be removing something that is currently matched (even before the patch
> > in this thread), so I stopped short of it in my optimizations.
>
> Interesting. Right now, if I create a 'direct' tag that points
> directly at HEAD, and then create an 'indirect' tag that points at
> 'direct', i.e.
>
> $ git tag -a -m 'a direct tag to HEAD' direct HEAD
> $ git tag -a -m 'an indirect tag' indirect direct
>
> I would get a piece of advice message that encourages to correct the
> mistake with "git tag -f indirect direct^{}". Then I ask for tags
> that point at HEAD, I see only 'direct' and not 'indirect'. Your
> optimization would start showing both 'direct' and 'indirect' if
> they are packed. But you are correct to worry about the opposite
> case. If I ask for tags that point at 'direct', I currently see
> 'indirect', but of course 'indirect' will not appear as the peeled
> value of any ref, and the optimized version will stop saying that
> 'indirect' is a ref that points at 'direct'. That sounds like a
> regression.
I agree. I purposely let the loop run through all the tags in the chain
in my inital patch to keep the existing behaviour. Unless there is a
compelling reason I would suggest do keep it this way. I wouldn't rule
out that someone relies on being able to query for indirect tags by
supplying a tag to the --points-at option.
> > But even
> > if we decide to do that, Jan's patch is not making anything worse there
> > (in fact, it is making it better, because it is matching "C" which we do
> > not currently match).
> > ...
> > So I'd be inclined to proceed with the patches I sent earlier, and then
> > if we choose to later refactor again to drop "B", we can.
>
> We generally avoid taking away anything once we give it to users;
> once the patch under discussion goes in, there is no taking it back,
> i.e. the new _behaviour_ closes the door to certain optimizations.
>
> I do not at all mind to see us decide and declare that it is a good
> thing to say that not just 'direct' but also 'indirect' points at
> HEAD, and that 'indirect' points at 'direct' and the patch under
> discussion makes the world a etter place, and we will not regret
> that decision. But the time to make such a decision is now, before
> we give a go-ahead to the patch.
The reasoning why I came up with the patch in the first place was an odd
behaviour that was reported to me in [1]. The user had a recipe that
checked out a nested tag for a package but the Bob Build Tool thought
the user messed with the working copy. Behind the scenes Bob checked out
the indirect tag (foobar-3.13.1) successfully. But when the user
examined the project state ("bob status"), Bob ran
git tag --points-at HEAD
in the working copy to get the list of tags for the current HEAD. That
did not return the expected tag in the list, so the workspace state was
flagged. OTOH, the tag _is_ visible when manually executing "git log" in
the workspace. Clearly this is confusing and so was I when I tried to
find the root cause. To sum up:
$ git checkout nested-tag --> works
$ git log --> shows nested tag in decoration
$ git tag --points-at HEAD --> does *not* show nested tag
AFAICT the nested tag in the mentioned bug report was not created on
purpose. I haven't come across any sensible use case for them either.
But as most git commands can handle nested tags they should better be
supported consistently IMHO.
-- Jan
[1] https://github.com/BobBuildTool/bob/issues/520
