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

Eric Engestrom eric.engestrom at imgtec.com
Thu Jun 16 16:31:08 UTC 2016


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).

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, I think I'll bump the hash length to ten.

Please do so using `rev-parse --short=10` :)

> 
> 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 :(


More information about the mesa-dev mailing list