-
Notifications
You must be signed in to change notification settings - Fork 64
digest: promote blake3 to first-class digest #66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
66e27ab
to
1728461
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🍨
oh beans, a rebase is needed |
1728461
to
1367d1e
Compare
The dual module approach for blake3 was slightly awkward. Since it provides similar usability with a massive bump in performance, it's extremely likely to land as a registered algorithm in the image-spec. This PR removes the secondary module, which made it difficult to test as a unit. This may break users who are using HEAD versions of the package. For a new release, this will be backwards compatible. The other drawback is that the zeebo/blake3 will now be a dependency but this can be replaced transparently by the standard libary in the future. In addition to promoting blake3, this makes a few style adjustments to be in line with Go's style guidelines. Signed-off-by: Stephen Day <stephen.day@getcruise.com>
1367d1e
to
b941ada
Compare
I'm not against this, but I'm not sure we have reached the consensus on the algorithm name. Is there any actual implementation that has been already using blake3/b3-256/whatever else? |
Since blake3 supports multiple digests sizes it seems to me that "blake3" without the digest size (e.g., "blake3-256") is a bad choice for a name. Furthermore, it makes sense to register the 512 bit algorithm as well to have the same collision resistance as SHA512 that you already have registered. "blake3-512" or "b3-512" along with either "blake3-256" or "b3-256" seem to make the most sense for names. This is not a binary compact format like multihash so I favor spelling out "blake3" instead of using "b3". That is my two cents for what it is worth. |
since the library you've pulled is is defaulting to 256bit, I think going with @ktarplee is a fine idea. And optionally we could also add "blake3-512" |
Do we want to pick this up again? Do we have consensus in the spec yet? |
Is there any existing user of blake3 in OCI? |
it would be great to see blake3 usage to start permeating the ecosystem - @ktarplee's naming proposal seems logical |
I think we should codify it in the spec before promotion (which I'm now explicitly in favor of: opencontainers/image-spec#819 (comment)), but with the caveat that I'm only an image-spec maintainer, not go-digest. |
FWIW, opencontainers/image-spec#819 is now closed via opencontainers/image-spec#1240 |
Does this need rebasing? |
github.com/zeebo/assert v1.1.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0= | ||
github.com/zeebo/blake3 v0.1.1 h1:Nbsts7DdKThRHHd+YNlqiGlRqGEF2bE2eXN+xQ1hsEs= | ||
github.com/zeebo/blake3 v0.1.1/go.mod h1:G9pM4qQwjRzF1/v7+vabMj/c5mWpGZ2Wzo3Eb4z0pb4= | ||
github.com/zeebo/blake3 v0.2.0 h1:1SGx3IvKWFUU/xl+/7kjdcjjMcvVSm+3dMo/N42afC8= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now at 0.2.4
opencontainers#66 opencontainers/image-spec#1240 Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>
opencontainers#66 opencontainers/image-spec#1240 Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com>
PR #108 is now passing ci...pls take a look. |
* digest: promote blake3 to first-class digest The dual module approach for blake3 was slightly awkward. Since it provides similar usability with a massive bump in performance, it's extremely likely to land as a registered algorithm in the image-spec. This PR removes the secondary module, which made it difficult to test as a unit. This may break users who are using HEAD versions of the package. For a new release, this will be backwards compatible. The other drawback is that the zeebo/blake3 will now be a dependency but this can be replaced transparently by the standard libary in the future. In addition to promoting blake3, this makes a few style adjustments to be in line with Go's style guidelines. Signed-off-by: Stephen Day <stephen.day@getcruise.com> * fix: update stevvooe's blake3 PR opencontainers#66 opencontainers/image-spec#1240 Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * fix: add a length test for blake3 Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * fix: add a Makefile make make build make test Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * fix: merge conflicts Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> * fix: blake3 pulls in golang 1.22.x dep 2025-04-25T19:33:20.2495501Z go: downloading github.com/zeebo/blake3 v0.2.4 2025-04-25T19:33:20.3154128Z go: downloading github.com/klauspost/cpuid/v2 v2.2.10 2025-04-25T19:33:20.3959610Z github.com/klauspost/cpuid/v2: cannot compile Go 1.22 code 2025-04-25T19:33:21.1059807Z FAIL github.com/opencontainers/go-digest [build failed] 2025-04-25T19:33:21.1060449Z FAIL github.com/opencontainers/go-digest/digestset [build failed] 2025-04-25T19:33:21.1219422Z ##[error]Process completed with exit code 1. Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> --------- Signed-off-by: Stephen Day <stephen.day@getcruise.com> Signed-off-by: Ramkumar Chinchani <rchincha.dev@gmail.com> Co-authored-by: Stephen Day <stephen.day@getcruise.com>
The dual module approach for blake3 was slightly awkward. Since it
provides similar usability with a massive bump in performance, it's
extremely likely to land as a registered algorithm in the image-spec.
This PR removes the secondary module, which made it difficult to test as
a unit. This may break users who are using HEAD versions of the package.
For a new release, this will be backwards compatible. The other drawback
is that the zeebo/blake3 will now be a dependency but this can be
replaced transparently by the standard libary in the future.
Signed-off-by: Stephen Day stephen.day@getcruise.com