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

From: Junio C Hamano <[email-masked]>
Date: Fri, 31 Mar 2017 11:33:53 -0700

Junio C Hamano <[email-masked]> writes:

> Michael J Gruber <[email-masked]> writes:
>
>>>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.
>
> So it sounds like you meant to do something else, and the
> implementation is wrong for that something else (i.e. it wouldn't do
> the right thing for a tag object outside refs/tags/, with or without
> the "--debug" option passed).

The damage seems worse, but I may be misreading the code.

is_better_name() compares name->from_tag and from_tag numerically,
because it was designed to take a boolean view of that variable.
Now, an artificially bumped 2 gets compared with name->from_tag that
may be 1 and gets different priority.  That artificially inflated
value may be propagated to name->from_tag when the current tip is
judged as a better basis for naming the object.

If this change is only for debugging, perhaps inside if(data->debug)
you added, instead of looking at from_tag, you can look at both
from_tag and deref to choose which prio-nmes to show, without
butchering the value in from_tag variable to affect the existing
code that is exercised with or without --debug?