pkg: don't depend on dune_digest #11723
Conversation
Leonidas-from-XIV
left a comment
Leonidas-from-XIV
left a comment
There was a problem hiding this comment.
I'm not sure about this PR
- OCaml ships with
Digestwhich supports MD5 so I don't think vendoring another implementation of MD5 is reasonable. - #11720 is vendoring BLAKE3, which in 2025 seems significantly better than MD5
- OCaml's
Digestapparently supports BLAKE2 since OCaml 5.2 (TIL & hooray!), so if we wanted to upgrade to a better hash, maybe BLAKE2 would make more sense?
We are indeed reusing the md5 implementation that is present in OCaml. We're doing so in C rather than OCaml because we don't want to hold the GC lock while digesting.
Right, but we still need MD5 for package management because we are verifying MD5 digests from the opam repository.
BLAKE2 is an improvement over MD5, but not a particularly impressive one. |
[Dune_digest] used md5 but this is an implementation detail we shouldn't rely on for computing md5 checksums in package management. We therefore copy and strip down the important parts, renaming any C stubs. Signed-off-by: Ali Caglayan <alizter@gmail.com>
Leonidas-from-XIV
left a comment
Leonidas-from-XIV
left a comment
There was a problem hiding this comment.
Okay, I see, so this is about holding the GC lock.
Overall the code looks ok, I have one question and one thing that's more of a discussion point…
Signed-off-by: Ali Caglayan <alizter@gmail.com>
Signed-off-by: Ali Caglayan <alizter@gmail.com>
Signed-off-by: Ali Caglayan <alizter@gmail.com>
Signed-off-by: Ali Caglayan <alizter@gmail.com>
You’re talking about speed, there, I guess? |
* pkg: don't depend on dune_digest [Dune_digest] used md5 but this is an implementation detail we shouldn't rely on for computing md5 checksums in package management. We therefore copy and strip down the important parts, renaming any C stubs. * refactor(md5): simplify implementation further * refactor(md5): rename and document md5 api * refactor(md5): rename *hex_string -> *hex * md5: better error message on unix error when opening file for digest Signed-off-by: Ali Caglayan <alizter@gmail.com>
4 participants
Dune_digestused md5 but this is an implementation detail we shouldn't rely on for computing md5 checksums in package management. We therefore copy and strip down the important parts, renaming any C stubs.