search

bazelbuild / remote-apis

An API for caching and execution of actions on a remote system.
Apache License 2.0
339 stars 118 forks source link

Clarify OutputFile.path and OutputDirectory.path relativeness #25

Closed t-chaik closed 6 years ago

t-chaik commented 6 years ago

In REAPI v1 OutputFile.path and OutputDirectory.path where both documented as:

message OutputFile { // The full path of the file relative to the input root, [...]

message OutputDirectory { // The full path of the directory relative to the input root, [...]

But now, in REAPI v2, OutputDirectory.path documentation changed for :

message OutputDirectory { // The full path of the file relative to the working directory. [...]

There doesn't seem to be any good reason for OutputFile.path and OutputDirectory.path to be relative to different root, neither there is to depend on a root (working directory) that is define outside of the current message scope.

Any reasons why OutputDirectory.path relativeness had been changed?

I think that this modification has been introduced by mistake.

ola-rozenfeld commented 6 years ago

Oh, it was changed on purpose, because we now allow modifying the working directory: https://github.com/bazelbuild/remote-apis/blob/master/build/bazel/remote/execution/v2/remote_execution.proto#L478 The feature was introduced because some existing clients prefer to run their actions in the root directory of a VM, and rely on some absolute paths (IIRC). The working directory is equal to the input root by default, though -- maybe we should have mentioned this again in the output files/dirs documentation?

t-chaik commented 6 years ago

Thanks for the details @ola-rozenfeld!

Isn't it misleading for OutputFile.path and OutputDirectory.path not to be relative from the same root? I'm fine either input root or working directory, but I find the inconsistency quite error prone...

ola-rozenfeld commented 6 years ago

Oh! Haha, yes, sorry, I only now understood you fully -- yes, it's supposed to say working directory everywhere, of course. Fixing now. Thank you!