Re: [PATCH] find_unique_abbrev(): honor caller-supplied "len" better

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 11, 2011, 00:40 UTC
Message-ID: <7vzkp21ocm.fsf@alter.siamese.dyndns.org>
In-Reply-To: <AANLkTimH9=OWD4+dPsdYaL2VPdDkTsUG_N3GBf168XqD@mail.gmail.com>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> When you pass in 6 as a len, and that isn't sufficient, it expands it
> to (say) 10. And then you pass in 7 as a length, and now it's
> sufficient, so it keeps it at 7.

Hmph, that is not the way I wanted to do things. If len is 6 and the result needs to be 10 to be sufficient, it should still yield 10 if the len you give is 7 (or 8 or 9 or 10). Of course it would be stupid if you don't do it that way.

I thought the code should work that way already (unless of course there is an implementation bug).

How many characters do we need to name master uniquely today?

    $ ./git -c core.abbrevguard=0 rev-parse --short=1 master
    83c3c

Ok, so there are more than one object with 83c3 and 83c3c is the absolute minimum. We can ask for extra for futureproofing.

    $ ./git -c core.abbrevguard=3 rev-parse --short=1 master
    83c3c622

And we get three extra and the result is 8 characters long. What if we give len=7 that is still short?

    $ ./git -c core.abbrevguard=3 rev-parse --short=7 master
    83c3c622

Of course we allow the limit 7 to be busted to keep the futureproofing.

    $ ./git -c core.abbrevguard=3 rev-parse --short=8 master
    83c3c622

Obeying 8 doesn't hurt the futureproofing so the output remains the same. But if you say len=9, you get 9

    $ ./git -c core.abbrevguard=3 rev-parse --short=9 master
    83c3c6222

even though we know 8 is plenty futureproof, but you get 9 because you asked for 9.

What am I missing to be called "just stupid" and "crazy"?

The earlier code was adding the three extra over what was asked for. With guard=3 and len=7, you would have got 10 for this object that only needs 5 as absolute minimum today, and giving 8 would have been enough to stay unique with room for 3 extra characters to grow. That is what I was fixing.

>  (b) You can't change DEFAULT_ABBREV except with the command line option.

I would agree that would need changing, but I think that is more or less an independent issue.

I thought I took your patch to introduce the configuration when I sent the abbrevguard as a companion patch, but apparently I didn't. That would fix the "grep DEFAULT_ABBREV" issue.