bugs/blake3 hash supportgit-annexhttp://git-annex.branchable.com/bugs/blake3_hash_support/git-annexikiwiki2022-12-26T17:09:45Zblake3 enhancementhttp://git-annex.branchable.com/bugs/blake3_hash_support/comment_1_723a531dad2df722514a7cbf1b8e0d8a/Ilya_Shlyakhter2022-12-11T19:25:27Z2022-12-11T19:25:27Z
Seems like this should be under <a href="http://git-annex.branchable.com/todo/">todo</a> suggestions -- maybe repost there?
comment 2http://git-annex.branchable.com/bugs/blake3_hash_support/comment_2_7be0932328ba4f49c33b0ab14429a283/joey2022-12-12T16:52:03Z2022-12-12T16:22:17Z
<p>Thanks for the patch.</p>
<p>I do think it would make sense to include "256" in the name of the backend,
unless BLAKE3 recommends against using other lengths for some good reason.</p>
<p>As for accepting this patch, the added dependency makes it harder. This
would require distributions like Debian add the blake3 package. If
cryptonite included it, that would be much easier -- though it would still
need ifdefs to support the old versions of crytonite for some time.</p>
<p>Going the current route with adding a blake3 depdenency would need to start
with it as a build flag..</p>
comment 3http://git-annex.branchable.com/bugs/blake3_hash_support/comment_3_a28c55a1d6158e301cc63d6087d8ab0e/edef2022-12-16T14:53:00Z2022-12-16T13:53:04Z
<blockquote><p>I do think it would make sense to include "256" in the name of the backend,
unless BLAKE3 recommends against using other lengths for some good reason.</p></blockquote>
<p>Done.</p>
<blockquote><p>Going the current route with adding a blake3 depdenency would need to start
with it as a build flag..</p></blockquote>
<p>I've added a followup patch that makes blake3 into a build flag. I'm
happy to roll with that for now, if that gets code merged. I'm not
particularly satisfied by the existing BLAKE3 libraries for Haskell,
so I might opt to roll a nicer one with proper parallelism when I have
some time to spare.</p>
<p>It appears that some flags switch between a vendored library copy and
an external dependency (namely, the git-lfs flag), maybe that's an
alternative option for the interim?</p>
comment 4http://git-annex.branchable.com/bugs/blake3_hash_support/comment_4_3ef2af8eb62a81299e49a3b7dccd337e/joey2022-12-26T17:09:45Z2022-12-26T16:26:45Z
<p>I've only vendored libraries that I developed myself,
that made sense to split out of git-annex. I don't want to get into
vendoring other stuff here. But I think a build flag is ok, it does not
need vendoring.</p>
<p>Looking at the code, I suspect that updateIncrementalVerifier
might have a space leak. It does not force the S.length to be evaluated.
And BLAKE3.update might also be lazy, I am not sure?
Compare with the very similar code in mkIncrementalHasher,
which is careful to force the length and also the hash update.</p>
<p>Also I would prefer not to use Control.Arrow just because I'm not
idiomatically familiar with it. Not that <code>***</code> is very complicated,
but it obfuscates the code slightly for me.</p>
<p>So, I would be happier if that part of the code were changed to just
spell it all out and force everything like mkIncrementalHasher does.
(Or if mkIncrementalHasher were generalized so that it could reuse its
code. Looks doable by parameterizing hashUpdate and hashFinalize.)</p>
<p>Also doc/backends.mdwn will need to be updated.</p>
<p>The rest of the code looks good and I'm ready to merge it once the above
are addressed.</p>