On Fri, Nov 22, 2024 at 09:39:07AM +0900, Junio C Hamano wrote:
> Taylor Blau <me@xxxxxxxxxxxx> writes:
>
> > Fix that by correctly assigning the value of 'revs->tag_objects' to the
> > 'tmp_tags' field.
>
> Makes sense. This breakage would make no difference in practice
> right now, as {type}_objects members of the rev_info structure have
> always been all flipped on together since their inception at
> 3c90f03d (Prepare git-rev-list for tracking tag objects too,
> 2005-06-29), so the original value of the tag_objects member is
> always the same as that of the blob_objects member.
Yep, I concur.
> A possible alternative "fix" for this typo could be to unify these
> {type}_objects members into a single .non_commit_objects member in
> the rev_info structure; given that we (as far as I remember) never
> utilized the "feature" that, say, we can ask only for trees but not
> blobs for the past ~20 years, nobody knows if the apparent "support"
> for that feature is subtly broken in multiple ways (and one of them
> you just fixed with this patch) and the "feature" may not be worth
> keeping.
I have been tempted to do that, too. But FWIW, I do remember
implementing something that set some but not all of the fields in a
series. Digging in my old branches, I think it may have been for a
reachability check for remote git-archive, where we want to dig only as
deep as the object we are looking for (so if we are finding out if a
tree is reachable, we do not need to ask about blobs, and looking for a
tag needs neither trees nor blobs).
If no such code made it into the project in those 20 years, it may not
worth worrying about too much. But I wanted to point it out as a
plausible use case.
> But neverhteless, this is a correct thing to do unless we decide to
> rip out the support for individual types. Will queue.
Agreed.
-Peff
