[Mesa-dev] [PATCH] automake: increase the MESA_GIT_SHA1 hash id length from 7 to 10 digits

[Brian Paul]

On 06/15/2017 07:10 AM, Emil Velikov wrote:
> On 15 June 2017 at 13:54, Brian Paul <brianp at vmware.com> wrote:
>> On 06/15/2017 03:38 AM, Emil Velikov wrote:
>>>
>>> On 15 June 2017 at 10:12, Eric Engestrom <eric.engestrom at imgtec.com>
>>> wrote:
>>>>
>>>> On Thursday, 2017-06-15 09:56:55 +0100, Emil Velikov wrote:
>>>>>
>>>>> Hi Brian,
>>>>>
>>>>> On 15 June 2017 at 04:46, Brian Paul <brianp at vmware.com> wrote:
>>>>>>
>>>>>> The SCons build has been using 10 digits of the git hash id for the
>>>>>> MESA_GIT_SHA1 string in git_sha1.h for about a year now.  I bumped it
>>>>>> up after running into a case where a 7-digit hash ID was ambiguous.
>>>>>>
>>>>>> This patch makes the same change for the autotools build.
>>>>>>
>>>>>> The command "git log | grep "^commit" | cut -b 8-14 | sort | uniq -d"
>>>>>> shows there are currently 17 cases where 7 digits of hash id are
>>>>>> ambiguous on master (probably quite a few more if we'd consider other
>>>>>> branches).
>>>>>>
>>>>>> Instead of using "git log -n 1 --oneline" use
>>>>>> "git rev-parse --short=10 HEAD" to get the HEAD hash id.
>>>>>
>>>>>
>>>>> The current command produces 11 digit SHA
>>>>
>>>>
>>>> That would depend on your git version (it used to be 7 until early 2.x
>>>> and is 10 on my 2.13.1), and your core.abbrev config :)
>>>>
>>> Hmm that may explain some of it - using 2.13.0 here.
>>> Local/global/system core.abbrev on the other hand is empty.
>>
>>
>> I'm still on git 1.9.1 (Mint 17) and I only get 7 digits.
>>
>>
>>>>> and size will grow automatically as needed.
>>>>
>>>>
>>>> Ack, but only after the collision happens, I think the point is to be
>>>> big enough that the hash stored will be unique compared to (reasonably)
>>>> all the future versions. Being explicit about the min length does that.
>>>>
>>> Having it future proof sounds reasonable - thanks did not think of that.
>>>
>>> Not sure about the growth algo. See the following:
>>>
>>> $ for i in `seq 14 20`; do \
>>>      echo -n "SHA digits $[$i-8+1] -- conflicts "; \
>>>      git log | grep "^commit" | cut -b 8-$i | sort | uniq -d | wc -l; \
>>> done
>>>
>>> SHA digits 7 -- conflicts 17
>>> SHA digits 8 -- conflicts 1
>>> SHA digits 9 -- conflicts 0
>>> SHA digits 10 -- conflicts 0
>>> SHA digits 11 -- conflicts 0
>>> SHA digits 12 -- conflicts 0
>>> SHA digits 13 -- conflicts 0
>>>
>>> $ git log -n 1 --oneline
>>> 18d4a6f964c (HEAD -> master) i965: gen4_blorp_exec.h to the sources list
>>
>>
>> So, what do you propose with regard to the patch?  git rev-parse with 11
>> digits (or 12)?
>>
> I agree with Eric here - land as-is and polish (digit range and/or
> separate script) at a later stage.
> Really wanted to see if there's anything busted (on my end?) since
> things look rather different here.
>
> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

Thanks.  I'm going to post a v2 which uses printf instead of sed, per 
Eric's suggestion.

-Brian