Do not need to store zero-length filesystem files

[palfrey]

Description

Ran into issues during #2024 and I think they're due to filesystem not handling zero-length file storage correctly. Namely, it happily retrieves zero-length files, but it shouldn't need to store zero-length ones. Previously there was a partial implementation of zero-length special casing, this just expands it to all the other upload/retrieve cases.

Type of change

Please delete options that aren't relevant.

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

How Has This Been Tested?

Please also list any relevant details for your test configuration

Checklist

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

---

This change is 

[MarcusSorealheis]

I think this is mostly good. My high level thoughts are:

Skip populate_fast_store on zero digest, right? Seems like a wasted network call because there will be no data. However, I'm not sure offhand if that would break the protocol.

Also probably need to verify no paths try to hardlink/open the synthesized zero-entry,

Is there a reason why we don't want to extend other store impls with zero-digest fast-path?

[MarcusSorealheis]

@amankrx could you take a look at this, maybe test some existing workloads with this branch.

[palfrey]

Answering the last one first :)

Is there a reason why we don't want to extend other store impls with zero-digest fast-path?

Other than it's a chunk of work, no, not really. AFAIK, I think all stores should implement zero-digest fast-path the same way, which had me wondering if it should eventually go in StoreLike not the actual stores TBH. Except that there's things like RunningActionImpl and get_file_entry_for_digest that mean we need do some things in a store-specific way. I've just written #2034 about doing this in general.

Skip populate_fast_store on zero digest, right? Seems like a wasted network call because there will be no data. However, I'm not sure offhand if that would break the protocol.

Yeah, I'm not totally sure. If the Fast store implemented zero-digest fast-path, it's fine, but otherwise not so much. I think we should leave it as is.

Also probably need to verify no paths try to hardlink/open the synthesized zero-entry,

The only other code AFAIK that does hardlinks is the DirectoryCache work, which I think is safe from this.

[MarcusSorealheis]

SGTM!