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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
