6 ms·

Someone please remind me why the hash is not a type definition so the representation would only have to be changed in one place.

by zoren 10y ago

csense 10y ago
They want the new version to be backward compatible with existing sha1 repos and remotes. Also, sha256 hashes are longer.
jffry 10y ago
If you have a repo with a lot of GPG signed commits, or you just don't want to change all your commit IDs (because you reference them in other places), then it'd be very valuable to be able to have a repo that's mixed old and new hashes. Also your Git binary, if compiled with only the One True Hash™, wouldn't be able to work with older repos at all because the hashes it's calculating are now different. (Edit: Another benefit of generalizing this is so that if/when, in the future, the new hash algorithm must be abandoned due to weaknesses, Git tooling will have been already introduced to the notion that hashes can be different and should hopefully be a less involved migration the next time around)
- loeg 10y ago
  The one typedef could have just been changed from char[20] to 'struct objectid' to support multiple hash types.
  - hn_throwaway_99 10y ago
    It was, see comment from bk2204 above: > Yes, this is correct. The struct object_id changes don't actually change the hash. What they do, however, is allow us to remove a lot of the hard-coded instances of 20 and 40 (SHA-1 length in bytes and hex, respectively) in the codebase.
    - loeg 10y ago
      No. We're talking about zoren's hypothetical case where git used a typedef from the beginning, instead of littering char[20]s all over the tree. My prior comment was explaining why jffry's complaint is nonsensical (a typedef does not prevent moving from a single hash model to a multiple simultaneous hash model).
      - hn_throwaway_99 10y ago
        Hmm, not sure where there disagreement is, that's exactly what I'm saying. Obviously it wasn't done from the beginning, but the code is now changing from the char[20]s everywhere to the typedef precisely to be able support multiple hash functions.
nuntius 10y ago
Backwards compat requires that both old and new hashes work at the same time. A simple typedef is unlikely to handle all the semantics and space needed for such a change... It is often hard to generalize when N=1. Now that the N=1 use case is established and we are moving towards N=2, it is painfully obvious to all that a better abstraction is needed. Typedef or no, we would still need a full audit of the code to find spots where people "inlined" the expansion. IMO, Linus should have done better here -- no crypto hash lasts forever, but this code is far cleaner than useless layers of abstraction.
- jlgaddis 10y ago
  Perhaps you haven't read Linus' comments where he stated (more than a decade ago) that the usage of SHA1 here isn't for "security"? (Hint: that's why GPG signing commits is an option.)
  - scrollaway 10y ago
    When you GPG sign a commit, you just GPG sign its hash, you're not signing its diff alongside it.
    - cormacrelf 10y ago
      That's what comes to mind every time someone brings up Linus' comments from way back when. If SHA-1 is insecure, then there is no way to have security. Forge an object, and GPG sign its commit, and you have broken the apparent security GPG signing was meant to bring. If SHA-1 was not meant for security, then security must have been a non-goal of Git. The comments are brought up usually to explain why Linus didn't think much of it at the time, whereas they actually demonstrate the shift of thinking around what Git is meant to provide. Security is definitely a goal now, and the hash function is the critical piece of security infrastructure.
    - glandium 10y ago
      GPG signatures actually sign the hash digest of the text they're given. Fun fact, which I think (hope) changed in recent versions of GPG: the hash, by default, is (was?) SHA-1. One can check what is used with e.g. $ git cat-file -p $some_tag | gpg --list-packets | grep "digest algo" The output is of the form digest algo n, begin of digest xx yy Where n can be: 1: MD5 2: SHA1 8: SHA256 10: SHA512 (See RFC 4880, 9.4 for all values)
deleted 10y ago
[deleted]
dahart 10y ago
That's exactly what this change is. You mean why wasn't it that way before the change? Maybe because it wasn't ever needed before? Git's been good with only sha-1 for 12 years. Think about the flip side of your question... what were the alternatives 12 years ago, or 5 years ago? And why would someone write code for alternatives that aren't expected to be used and maybe don't exist? In my experience, generalizing ahead of need more often than not causes problems, and I've watched over-engineering result in far more effort to fix when the need it was anticipating does arrive than just waiting until the need is there.
- digi_owl 10y ago
  Not really adding much, but damn it i feel old reading that. I still recall freshly the hoopla over Bitkeeper licensing that lead to Torvalds creating Git.
- loeg 10y ago
  > what were the alternatives 12 years ago, or 5 years ago? SHA-2. > And why would someone write code for alternatives that aren't expected to be used and maybe don't exist? Well, the real question is why someone picked SHA-1 over SHA-2 in 2005 when attacks that reduced its strength were already being demonstrated.
  - dahart 10y ago
    Linus has explained why he picked SHA-1. I'm not Linus, and I'm not defending his choice, but he has said repeatedly that git's hash is primarily for indexing and error correction, and not primarily for security. Clearly he felt like SHA-1 was "good enough". And if you have something that's "good enough" there are reasons not to write code for alternatives you're not going to use.
    - snakeanus 10y ago
      >but he has said repeatedly that git's hash is primarily for indexing and error correction, and not primarily for security And he was wrong as openpgp signatures on commits and tags are a thing. Not sure when that feature was introduced however, I doubt that it existed in the first version of git. That being said he should have changed the hash function the moment that feature was introduced.
asveikau 10y ago
How to say this without being rude.. You didn't read the diff. To derisively say "remind me why not X" at a diff that does X ... I am amused.

Someone please remind me why the hash is not a type definition so the representation would only have to be changed in one place.

by zoren 10y ago

Someone please remind me why the hash is not a type definition so the representation would only have to be changed in one place.

csense 10y ago

They want the new version to be backward compatible with existing sha1 repos and remotes. Also, sha256 hashes are longer.

jffry 10y ago

If you have a repo with a lot of GPG signed commits, or you just don't want to change all your commit IDs (because you reference them in other places), then it'd be very valuable to be able to have a repo that's mixed old and new hashes. Also your Git binary, if compiled with only the One True Hash™, wouldn't be able to work with older repos at all because the hashes it's calculating are now different. (Edit: Another benefit of generalizing this is so that if/when, in the future, the new hash algorithm must be abandoned due to weaknesses, Git tooling will have been already introduced to the notion that hashes can be different and should hopefully be a less involved migration the next time around)

loeg 10y ago
The one typedef could have just been changed from char[20] to 'struct objectid' to support multiple hash types.
- hn_throwaway_99 10y ago
  It was, see comment from bk2204 above: > Yes, this is correct. The struct object_id changes don't actually change the hash. What they do, however, is allow us to remove a lot of the hard-coded instances of 20 and 40 (SHA-1 length in bytes and hex, respectively) in the codebase.
  - loeg 10y ago
    No. We're talking about zoren's hypothetical case where git used a typedef from the beginning, instead of littering char[20]s all over the tree. My prior comment was explaining why jffry's complaint is nonsensical (a typedef does not prevent moving from a single hash model to a multiple simultaneous hash model).
    - hn_throwaway_99 10y ago
      Hmm, not sure where there disagreement is, that's exactly what I'm saying. Obviously it wasn't done from the beginning, but the code is now changing from the char[20]s everywhere to the typedef precisely to be able support multiple hash functions.

nuntius 10y ago

Backwards compat requires that both old and new hashes work at the same time. A simple typedef is unlikely to handle all the semantics and space needed for such a change... It is often hard to generalize when N=1. Now that the N=1 use case is established and we are moving towards N=2, it is painfully obvious to all that a better abstraction is needed. Typedef or no, we would still need a full audit of the code to find spots where people "inlined" the expansion. IMO, Linus should have done better here -- no crypto hash lasts forever, but this code is far cleaner than useless layers of abstraction.

jlgaddis 10y ago
Perhaps you haven't read Linus' comments where he stated (more than a decade ago) that the usage of SHA1 here isn't for "security"? (Hint: that's why GPG signing commits is an option.)
- scrollaway 10y ago
  When you GPG sign a commit, you just GPG sign its hash, you're not signing its diff alongside it.
  - cormacrelf 10y ago
    That's what comes to mind every time someone brings up Linus' comments from way back when. If SHA-1 is insecure, then there is no way to have security. Forge an object, and GPG sign its commit, and you have broken the apparent security GPG signing was meant to bring. If SHA-1 was not meant for security, then security must have been a non-goal of Git. The comments are brought up usually to explain why Linus didn't think much of it at the time, whereas they actually demonstrate the shift of thinking around what Git is meant to provide. Security is definitely a goal now, and the hash function is the critical piece of security infrastructure.
  - glandium 10y ago
    GPG signatures actually sign the hash digest of the text they're given. Fun fact, which I think (hope) changed in recent versions of GPG: the hash, by default, is (was?) SHA-1. One can check what is used with e.g. $ git cat-file -p $some_tag | gpg --list-packets | grep "digest algo" The output is of the form digest algo n, begin of digest xx yy Where n can be: 1: MD5 2: SHA1 8: SHA256 10: SHA512 (See RFC 4880, 9.4 for all values)

deleted 10y ago

[deleted]

dahart 10y ago

That's exactly what this change is. You mean why wasn't it that way before the change? Maybe because it wasn't ever needed before? Git's been good with only sha-1 for 12 years. Think about the flip side of your question... what were the alternatives 12 years ago, or 5 years ago? And why would someone write code for alternatives that aren't expected to be used and maybe don't exist? In my experience, generalizing ahead of need more often than not causes problems, and I've watched over-engineering result in far more effort to fix when the need it was anticipating does arrive than just waiting until the need is there.

digi_owl 10y ago
Not really adding much, but damn it i feel old reading that. I still recall freshly the hoopla over Bitkeeper licensing that lead to Torvalds creating Git.
loeg 10y ago
> what were the alternatives 12 years ago, or 5 years ago? SHA-2. > And why would someone write code for alternatives that aren't expected to be used and maybe don't exist? Well, the real question is why someone picked SHA-1 over SHA-2 in 2005 when attacks that reduced its strength were already being demonstrated.
- dahart 10y ago
  Linus has explained why he picked SHA-1. I'm not Linus, and I'm not defending his choice, but he has said repeatedly that git's hash is primarily for indexing and error correction, and not primarily for security. Clearly he felt like SHA-1 was "good enough". And if you have something that's "good enough" there are reasons not to write code for alternatives you're not going to use.
  - snakeanus 10y ago
    >but he has said repeatedly that git's hash is primarily for indexing and error correction, and not primarily for security And he was wrong as openpgp signatures on commits and tags are a thing. Not sure when that feature was introduced however, I doubt that it existed in the first version of git. That being said he should have changed the hash function the moment that feature was introduced.

asveikau 10y ago

How to say this without being rude.. You didn't read the diff. To derisively say "remind me why not X" at a diff that does X ... I am amused.