Skip to content
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

[FR]: use sha256 instead of sha386 for integrity? #837

Closed
hunshcn opened this issue May 13, 2024 · 6 comments
Closed

[FR]: use sha256 instead of sha386 for integrity? #837

hunshcn opened this issue May 13, 2024 · 6 comments
Labels
enhancement New feature or request untriaged Requires traige

Comments

@hunshcn
Copy link

hunshcn commented May 13, 2024

What is the current behavior?

now jq/yq binary use sha386 integrity

But mainstream bazel remote cache server can only cache the assets with sha256/blake3 integrity

Describe the feature

use sha256 integrity

@hunshcn hunshcn added the enhancement New feature or request label May 13, 2024
@github-actions github-actions bot added the untriaged Requires traige label May 13, 2024
@hunshcn
Copy link
Author

hunshcn commented Jun 26, 2024

@alexeagle @gregmagolan

Will this be accepted? If I take a pr.

@alexeagle
Copy link
Collaborator

I don't think we care which hash algorithm is used, we just prefer to mirror whatever data is already available rather than download all the binaries and compute checksums ourselves.

@alexeagle
Copy link
Collaborator

Is there an issue for whatever system is failing to understand other hash algorithms? If we change this, it's a workaround for someone else's bug, so I'd at least like to know when they've fixed it.

@hunshcn
Copy link
Author

hunshcn commented Jun 26, 2024

Is there an issue for whatever system is failing to understand other hash algorithms? If we change this, it's a workaround for someone else's bug, so I'd at least like to know when they've fixed it.

see
buildbuddy-io/buildbuddy#6911
https://github.com/buchgr/bazel-remote/blob/fea980bd8f528ccfc38919fb709e9e47d27a1622/server/grpc_asset.go#L79

@alexeagle
Copy link
Collaborator

Oh I see, this is just for the remote asset API. I don't think it helps much to change integrity format on a couple binaries here - the problem is much more widespread (i.e. https://github.com/aspect-build/toolchains_protoc/blob/main/protoc/private/versions.bzl#L54 )

@sluongng
Copy link

sluongng commented Jun 26, 2024

yeah this is for Remote Asset API.

Traditionally the API has always been SHA256 based. Meaning that if you use it with some exotic hasher like SHA512 (rules_js) or SHA386 (jq, protoc etc…), then you get no cache hit from the Remote Cache and your download would always reach upstream URL. That’s functional, but not ideal for folks who rely on Remote Asset API for some speed up.

We recently implemented the changes in the Remote Asset API + Bazel that enables to send over a separate “storage” hasher in addition to the previous “content verify” hasher. This allows us to receive a download request with SHA512 verification, but BLAKE3 storage and download. There would still be no cache hit on this flow, but at least now, you can change the hasher Bazel would use to download/store the blob.

The remaining question is: how do we facilitate for cache hits for cases where the integrity in rctx.download is neither SHA256 or BLAKE3 (the 2 popular hash method Bazel supports)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request untriaged Requires traige
Projects
None yet
Development

No branches or pull requests

3 participants