[Mesa-dev] [PATCH] scons: put the generated git_sha1.h file in top-level src/ directory

[Brian Paul]

On 06/16/2016 10:31 AM, Eric Engestrom wrote:
> On Thu, Jun 16, 2016 at 09:48:54AM -0600, Brian Paul wrote:
>> On 06/16/2016 07:35 AM, Eric Engestrom wrote:
>>> That fixed truncation can give non-unique hashes. Switching to rev-parse
>>> (suggested above) fixes this.
>>
>> rev-parse --short produces a 7-char hash.
>
> No it doesn't, not in the general case. You may well have tested it on
> a commit whose hash was unique at that length, but in general it gives
> you the shortest hash that's unique at the point in time when it's run
> (with a minimum, default = 7).

Ah, OK.  I was going from the portion of the man page which says:

"""
        --short, --short=number
            Instead of outputting the full SHA-1 values of object names 
try to
            abbreviate them to a shorter unique name. When no length is
            specified 7 is used. The minimum length is 4.
"""

I had never come across git rev-parse before today.  I feel like I know 
about 1% of git to be honest.


>
> I think I didn't explain my point very well, so I'll say it differently:
> - If a commit hash is meant as a short lived information, the minimal
>    unique hash at that point is good enough. Manually truncating doesn't
>    guarantee that.
> - If the commit hash is meant to be long lived, the full hash should be
>    used. (This is typically the case in a commit message.)
>
> In this case, the hash is meaningless once it becomes old, so the first
> option is good enough.
> It still needs to be unique, at least when generated. For this reason,
> manually truncating it is not a good method.
>
> (This is, of course, my opinion. I know other people have other opinions
> (in both directions), which I respect. I'm just sharing mine.)
>
>
>> The commit[0:7] part above is
>> used to strip the trailing newline from the string (though that could be
>> expressed better).
>
> The original command (log) printed more informations on that line, which
> is what this code was stripping.
> I don't know if python needs newlines to be stripped, but if it does,
> please use an other method (would `commit[0:-1]` do that?).

Yeah, when I was testing without the substring specifier I was getting a 
newline character in my C string and that, of course, broke things.


>
>
>> Yeah, I think I'll bump the hash length to ten.
>
> Please do so using `rev-parse --short=10` :)

Yup, that's what my patch is doing.  I still need to test it on Windows...


>
>>
>> v2 patch coming in a bit...
>>
>> -Brian
>>
>
> Cheers,
>    Eric
>
> PS: I promise I'm not trying to be mean, but I feel like I'm coming
>      across as such :(

Nope, thanks for the helpful feedback!

-Brian