On Tue, Jun 12 2018, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
>
>> Change the core.abbrev config variable and the corresponding --abbrev
>> command-line option to support relative values such as +1 or -1.
>>
>> Before Linus's e6c587c733 ("abbrev: auto size the default
>> abbreviation", 2016-09-30) git would default to abbreviating object
>> names to 7-hexdigits, and only picking longer SHA-1s as needed if that
>> was ambiguous.
>>
>> That change instead set the default length as a function of the
>> estimated current count of objects:
>>
>> Based on the expectation that we would see collision in a
>> repository with 2^(2N) objects when using object names shortened
>> to first N bits, use sufficient number of hexdigits to cover the
>> number of objects in the repository. Each hexdigit (4-bits) we
>> add to the shortened name allows us to have four times (2-bits) as
>> many objects in the repository.
>>
>> By supporting relative values for core.abbrev we can allow users to
>> consistently opt-in for either a higher or lower probability of
>> collision, without needing to hardcode a given numeric value like
>> "10", which would be overkill on some repositories, and far to small
>> on others.
>
> Nicely explained and calculated ;-)
>
>> test_expect_success 'describe core.abbrev=[-+]1 and --abbrev=[-+]1' '
>> - test_must_fail git -c core.abbrev=+1 describe | sed_g_tr_d_n >describe &&
>> - test_must_fail git -c core.abbrev=-1 describe | sed_g_tr_d_n >describe &&
>> + git -c core.abbrev=-1 describe | sed_g_tr_d_n >describe &&
>> + test_byte_count = 6 describe &&
>> +
>> + git -c core.abbrev=+1 describe | sed_g_tr_d_n >describe &&
>> + test_byte_count = 8 describe &&
>
> Even though I see the point of supporting absurdly small absolute
> values like 4, I do not quite see the point of supporting negative
> relative values here. What's the expected use case?
I'll add a better explanation for this to the commit message.
Initially I did this for consistency, since it was easy to implement,
and there's no reason to have that arbitrary limitation, but thinking
about it again I think I'll use this for some of my projects.
E.g. here's a breakdown of my dotfiles repo:
$ git -c core.abbrev=4 log --pretty=format:%h|perl -nE 'chomp;say length'|sort|uniq -c|sort -nr
784 4
59 5
7 6
I don't have a single commit that needs 7 characters, yet that's our
default. This is a sane trade-off for the kernel, but for something
that's just a toy or something you're playing around with something
shorter can make sense.
SHA-1s aren't just written down, but also e.g. remembered in wetware
short-term memory.
>> git log --abbrev=+1 --pretty=format:%h -1 | tr_d_n >log &&
>> - test_byte_count = 4 log &&
>> + test_byte_count = 8 log &&
>
> This, together with many many others in the rest of the patch, is
> cute but confusing in that the diff shows change from 4 to 8 due to
> the redefinition of what abbrev=+1 means. I have a feeling that it
> may not be worth doing it "right", but if we were doing it "right",
> we would probably have done it in multiple steps:
>
> - the earlier patches in this series that demonstrates
> --abbrev=+1 is --abbrev=1 and core.abbrev=+1 is an error.
>
> - ensure --abbrev=+1 is rejected as syntax error just like
> core.abbrev=+1 was, without introducing relative values
>
> - introduce relative value.
>
> That way, the last step (which corresponds to this patch) would show
> change from "log --abbrev=+1" failing due to syntax error to showing
> abbreviated value that is slightly longer than the default.
>
> But a I said, it may not be worth doing so. "Is it worth supporting
> negative relative length?" still stands, though.
I'll see what I can do about this value churn.
