Directory canonicalization does not require non-empty names

[stuhood]

Hey folks! The definition of "canonical" for a Directory does not currently (https://github.com/bazelbuild/remote-apis/blob/f54876595da9f2c2d66c98c318d00b60fd64900b/build/bazel/remote/execution/v2/remote_execution.proto#L634-L653) require that the names of children are non-empty, but in practice this seems like a useful constraint.

As an example, the violation of one constraint on inputs lead to a server implementation returning a non-sensical output with an empty directory name (roughly: components like ["out", "", "etc", "file.txt"]). Including "non-empty" in the definition of canonical would have caught this earlier: immediately after the server's response.

Thoughts?

[EdSchouten]

FYI: These are the restrictions that Buildbarn applies: https://github.com/buildbarn/bb-storage/blob/d02061266d4c2f1bb7ecb52efad56e0a7f28c397/pkg/filesystem/path/component.go#L17

[EricBurnett]

It does say "Every child in the directory must have a path of exactly one segment", and you could argue that "" is zero segments rather than one. But either way, I'm fine with it being made more explicit that the empty string is not valid.

On Sun, Feb 7, 2021 at 7:43 AM Ed Schouten [email protected] wrote:

FYI: These are the restrictions that Buildbarn applies: https://github.com/buildbarn/bb-storage/blob/d02061266d4c2f1bb7ecb52efad56e0a7f28c397/pkg/filesystem/path/component.go#L17

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/remote-apis/issues/175#issuecomment-774668273, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABREW23F3VPUBXYO67ZUXDS52DGNANCNFSM4RXUC67Q .