Support empty digests in stores

[adam-singer]

Description

remote_execution.proto specification allows for uploading and fetching of empty digests regardless if they have been uploaded before a fetch. Expectation is the requests should always respond successfully. This can be an optimized path in cases but violates expectations in native link underlying storage consistencies.

Fixes #

#498

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Unit tests

Checklist

• Tests added/amended
• bazel test //... passes locally
• PR is contained in a single commit, using git amend see some docs

---

This change is 

[allada]

Reviewed 7 of 12 files at r1.
Reviewable status: 7 of 12 files reviewed, 5 unresolved discussions (waiting on @adam-singer)

a discussion (no related file):
I started reading the tests, but then realized we should just write a single test that tests all stores and just calls .get() and .has() (separate tests) without an update to ensure it returns a success.

By doing it this way we ensure that as we add tests these new tests get tested. If we use a match on the stores enum (in the config), we can get compile time errors if we create a new store but forget to add the test configuration to this file.

---

---

nativelink-store/src/cas_utils.rs line 19 at r2 (raw file):

const ZERO_BYTE_DIGESTS: [DigestInfo; 2] = [
    // Sha256 hash of zero bytes.
    DigestInfo::new(

nit: Lets make this public global consts and then link them in here so we can use them in tests without needing to import hashing functions.

---

nativelink-store/tests/cas_utils_test.rs line 23 at r2 (raw file):

    #[test]
    fn sha256_is_zero_digest() {

nit: Lets write a single test here instead.

match on the DigestHasherFunc's enum and use it's function to do the hashing and ensure they are inside is_zero_digest().

Doing it this way gives us a compile time error if we ever try to add a new digest function but forget to add the corresponding match.

---

nativelink-store/tests/compression_store_test.rs line 447 at r2 (raw file):

    #[tokio::test]
    async fn get_part_is_zero_digest() -> Result<(), Error> {
        let digest = DigestInfo {

nit: Lets import the empty digests from cas_utils.

---

nativelink-store/tests/compression_store_test.rs line 447 at r2 (raw file):

    #[tokio::test]
    async fn get_part_is_zero_digest() -> Result<(), Error> {
        let digest = DigestInfo {

nit: rename empty_digest

[allada]

Reviewed 2 of 12 files at r1, 5 of 5 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @adam-singer)

[allada]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @adam-singer)

[allada]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @adam-singer)

[adam-singer]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @adam-singer)

---

nativelink-store/tests/cas_utils_test.rs line 23 at r2 (raw file):

Previously, allada (Nathan (Blaise) Bruer) wrote…

nit: Lets write a single test here instead.

match on the DigestHasherFunc's enum and use it's function to do the hashing and ensure they are inside is_zero_digest().

Doing it this way gives us a compile time error if we ever try to add a new digest function but forget to add the corresponding match.

I did initially start with that, eventually it took more lines of passing a delegate for the creator, but we can revisit that and shrink it a bit