Ensure namedSetOfFiles URIs specify blob type correctly#19044
Ensure namedSetOfFiles URIs specify blob type correctly#19044tylerwilliams wants to merge 1 commit intobazelbuild:masterfrom
Conversation
|
@coeuvre I think this is the last one |
| * might do I/O. | ||
| */ | ||
| private PathMetadata readPathMetadata(Path path, LocalFile file) throws IOException { | ||
| DigestUtil digestUtil = new DigestUtil(xattrProvider, path.getFileSystem().getDigestFunction()); |
There was a problem hiding this comment.
I think we can just use path.getFileSystem().getDigestFunction() instead of creating a DigestUtil.
There was a problem hiding this comment.
DigestUtil exposes the DigestFunction.Value which is distinct from DigestHashFunction which is returned by the method you're suggesting.
So we're using DigestUtil to convert basically.
There was a problem hiding this comment.
Also -- sorry it wasn't clear, DigestUtil was already being created in this function (L217), i just moved it up so I can reference it :)
There was a problem hiding this comment.
Argh, yes. Sorry I didn't read the code carefully.
| Digest digest = metadata.getDigest(); | ||
| DigestFunction.Value digestFunction = metadata.getDigestFunction(); | ||
| String out; | ||
| if (isOldStyleDigestFunction(digestFunction)) { |
There was a problem hiding this comment.
Can we have a unit test for this?
|
Optional: we might also want to make similar changes to Http/Disk cache in a separate PR. |
2b83de5 to
e181273
Compare
|
Thanks! I probably need more time to import this because I need to figure out how to build blake3 internally for remote module so the test can be executed (previously, I skipped that). |
|
The changes in this PR have been included in Bazel 6.4.0 RC1. Please test out the release candidate and report any issues as soon as possible. If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=last_rc. |
I noticed when testing the BLAKE3 digest function that uploaded files were being referenced incorrectly in the BES because they were missing the digest type. This CL fixes that.
Before:
After: