Re: [PATCH v2] load_ref_decorations(): fix decoration with tags
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 13, 2021, 21:17 UTC
- Message-ID
- <xmqqpmvl29ry.fsf@gitster.g>
- In-Reply-To
- <YO1GNWjMol8JV8MR@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 6 quoted lines
>> The reason this happens is in the loop where we try to peel the tags, we >> won't necessarily have parsed that first object. If we haven't, its >> `tag` will be NULL, so nothing will be displayed, and its `tagged` will >> also be NULL, so we won't peel any further. > > Thanks, nicely explained.
Yup, nicely explained indeed.
Show 5 quoted lines
>> Note how this commit could have been done as an optimization before >> 88473c8bae: When our peeling hits a non-tag, we won't parse that tagged >> object only to immediately end the loop. > > Yep, thanks for mentioning this, as it's somewhat subtle.
It is too subtle that I am not sure what the paragraph wants to say.
Before 88473c8b, we had a fully parsed object in obj and entered the while() loop iff the outermost object is a tag, then we find the underlying object via obj->tagged. We parse that underlying object to find if it is a tag, and break out if it is not.
By "this commit", I assume that the above mean the change in this fix, i.e. parse 'obj' if it has not been parsed before looking at its tagged field. But I am not sure how that would have been an optimization before 88473c8b that gave a parsed tag object 'obj' upon entry to the loop.
Puzzled.
In any case, let's talk about this patch in the context to which it is designed to be applied, i.e. post 88473c8b3c8b.
When we come here, we have done oid_object_info() plus lookup_object_by_type() to obtain 'obj' and we know its type. Then we enter the loop.
while (obj->type == OBJ_TAG) {
+ if (!obj->parsed)
+ parse_object(the_repository, &obj->oid);And we parse if it hasn't been parsed. THat is why we can ...
obj = ((struct tag *)obj)->tagged; if (!obj) break;
... look at its tagged member.
- if (!obj->parsed) - parse_object(the_repository, &obj->oid); add_name_decoration(DECORATION_REF_TAG, refname, obj);
And the updated 'obj' (i.e. direct referent of the tag object) is fed to add_name_decoration(). And then we move to the next iteration.
Now, do we know what type of object 'obj' is at this point? We did parse the outermost tag upon entry to this loop, we replaced 'obj' variable with the referent by following the .tagged member, but we haven't parsed that object or ran oid_object_info() on it.
Puzzled.
}