Re: [PATCH v3 3/4] name-rev: provide debug output

[Michael J Gruber <git@grubix.eu>]

Am 31. März 2017 18:52:16 MESZ schrieb Junio C Hamano <gitster@pobox.com>:
>Michael J Gruber <git@grubix.eu> writes:
>
>> Currently, `git describe --contains --debug` does not create any
>debug
>> output because it does not pass the flag down to `git name-rev`,
>which
>> does not know that flag.
>>
>> Teach the latter that flag, so that the former can pass it down (in
>> the following commit).
>>
>> The output is patterned after that of `git describe --debug`, with
>the
>> following differences:
>>
>> describe loops over all args to describe, then over all possible
>> descriptions; name-rev does it the other way round. Therefore, we
>need
>> to amend each possible description by the arg that it is for (and we
>> leave out the "searching to describe" header).
>>
>> The date cut-off for name-rev kicks in way more often than the
>candidate
>> number cut-off of describe, so we do not clutter the output with the
>> cut-off.
>>
>> Signed-off-by: Michael J Gruber <git@grubix.eu>
>> ---
>
>>  static void name_rev(struct commit *commit,
>>  		const char *tip_name, unsigned long taggerdate,
>>  		int generation, int distance, int from_tag,
>> -		int deref)
>> +		int deref, struct name_ref_data *data)
>>  {
>>  	struct rev_name *name = (struct rev_name *)commit->util;
>>  	struct commit_list *parents;
>> @@ -75,6 +88,7 @@ static void name_rev(struct commit *commit,
>>  
>>  	if (deref) {
>>  		tip_name = xstrfmt("%s^0", tip_name);
>> +		from_tag += 1;
>
>Why this change?  I didn't see it explained in the proposed log
>message.  "deref" is true only when our immediate caller is the one
>that inspected the object at the tip and found it to be a tag object
>(i.e. not a lightweight tag or a branch).  from_tag is about "is the
>tip within refs/tags/ hierarchy?  Yes/No?" and such a caller will
>set it appropriately when calling us.  This function just passes it
>down when it recursively calls itself.  
>
>We shouldn't be mucking with that value ourselves here, should we?
>
>The only case that this change may make a difference I can think of
>is when you have a tag object pointed at from outside refs/tags
>(e.g. refs/heads/foo is a tag object); if you are trying to change
>the definition of "from_tag" from the current "Is the tip inside
>refs/tags/?" to "Is the tip either inside refs/tags/ or is it a tag
>object anywhere?", that may be a good change (I didn't think things
>through, though), but that shouldn't be hidden inside a commit that
>claims to only add support for debugging.
>
>What problem are you solving?  

Sorry, I forgot about that change and failed to mention it.

It makes no difference in the non-debug case which cares about the Boolean only. In the debug case, I want to distinguish between annotated and lightweight tags, just like describe --debug does. By adding 1 via deref and passing this down, I know that an annotated tag gets the value 2, a lightweight tag 1 and everything else 0, just like describe --tags.

>> @@ -236,7 +273,6 @@ static int name_ref(const char *path, const
>struct object_id *oid, int flags, vo
>>  	}
>>  
>>  	add_to_tip_table(oid->hash, path, can_abbreviate_output);
>> -
>>  	while (o && o->type == OBJ_TAG) {
>>  		struct tag *t = (struct tag *) o;
>>  		if (!t->tagged)
>
>This is a patch noise we can do without.
>
>Thanks.