Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify OutputFile.path and OutputDirectory.path relativeness #25

Closed
t-chaik opened this issue Aug 22, 2018 · 3 comments
Closed

Clarify OutputFile.path and OutputDirectory.path relativeness #25

t-chaik opened this issue Aug 22, 2018 · 3 comments

Comments

@t-chaik
Copy link

t-chaik commented Aug 22, 2018

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
Copy link
Contributor

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
Copy link
Author

t-chaik commented Aug 22, 2018

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
Copy link
Contributor

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants