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

Actions that declare directories but don't create those directories fail to build with remote strategy #861

Closed
thii opened this issue Aug 10, 2020 · 9 comments · Fixed by #896

Comments

@thii
Copy link
Member

thii commented Aug 10, 2020

Currently, there are some resource actions that declare output directories but don't create those directories. Instead, they rely on Bazel's unexpected behavior that creates those directories before executing an action. This makes the actions fail to execute with remote execution if the actions require the directories to exist before the execution.

The documentation for declare_directory says that

You must create an action that generates the directory.

The RBE API documentation says that

Directories leading up to the output directories (but not the output directories themselves) are created by the worker prior to execution

Related:

@thii thii changed the title Actions that declare directories should create those directories Actions that declare directories but don't create those directories fail to build with remote strategy Aug 10, 2020
@sergiocampama
Copy link
Contributor

how does that work? how are rules expected to create an action to create the directory if the directory is an output of the action? this seems to result in either having to wrap the tools in a bash script in order to create the output before invoking the tool or having to modify the tool to do the expected thing, which in many cases is not possible... there is no reason why rbe should require this from the action and not have the executor create the output directory for you, since the worst case is that you have an empty directory which gets deduplicated in the CAS.

@thii
Copy link
Member Author

thii commented Aug 10, 2020

You've said the solutions :P

I imagine you'd have to change the output from a directory to a file, if that's possible. If the directory has to be an output of the action, it seems odd to me that the tool does not create the directory for you. I guess I'll file a radar :).

Having the executor create the output directory for you seems to be the best option, but it's against the REv2 specification (even though some of the RE implementations do that).

@thii
Copy link
Member Author

thii commented Aug 12, 2020

Reported to BuildGrid: https://gitlab.com/BuildGrid/buildgrid/-/issues/330.

@edbaunton
Copy link

edbaunton commented Aug 12, 2020

It's worth noting that in v2.1 of RE the distinction between output files and output directories is dropped and it simply becomes an output_path. How do you expect this to behave in that situation?

@thii
Copy link
Member Author

thii commented Aug 12, 2020

Since the output directories would become part of the output_paths, I would expect the remote executor to create the output directories for you.

@edbaunton
Copy link

If the request contains an output_path of /path/to/my/file should it return a directory or a file? The server cannot know.

@thii
Copy link
Member Author

thii commented Aug 14, 2020

Interesting. This seems like a huge unexpected behavior of Bazel that a lot of rules have been depending on.

@sergiocampama
Copy link
Contributor

@edbaunton What was the argument for removing the distinction between files and directories for the outputs? That seems like it limits some of the introspection that could be done on the responses themselves, and it forces this reported issue into a mode that will just make it more difficult to integrate with tools that can not be modified in order to adopt the RE2 expectations.

@edbaunton
Copy link

The only discussion I could find is here.

I guess your rule is creating an empty directory and requiring that to be returned?

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

Successfully merging a pull request may close this issue.

3 participants