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

What is the expected behaviour when an action deletes an output directory? #130

Closed
asuffield opened this issue Apr 8, 2020 · 2 comments
Closed

Comments

@asuffield
Copy link

This came up in buildbarn/bb-remote-execution#46 because rules_go does this (accidentally): the output directory is deleted then recreated.

The subtlety here is about whether the output directory is "the exact inode created before the action runs" or "the inode at this path after the action completes". The second one is obviously more convenient, but it's not clearly specified.

@EricBurnett
Copy link
Collaborator

In terms of what the API specifies, I would expect it to be "the inode at this path after the action completes". This will be more obvious as systems migrate to output_paths, where it's not even knowable upfront whether an output will be a directory or a file, and so clearly cannot be a fixed pre-created inode. Though if you're talking about the whole input root, rather than a particular output directory within it, I don't think the API says anything - undefined behaviour.

Note that implementations (including buildbarn) may have limitations that go beyond what the API can specify. E.g. if you delete the whole input root directory and re-create it, I'd also expect some implementations to error. Generally unless something is explicitly required within the API, the minimum requirement is pretty much just clear error messages if/when you hit that boundary condition. Because unfortunately there's always going to be a fuzzy area of things you can do that one implementation will allow but another may not, and it's very unlikely we could set expectations for everything. (If you chmod/chroot one of your parent directories, will it work? How many inode watches can you create? What is the full surface of syscalls you're allowed to call? Etc.)

If this proves to be a capability we care about, adding it to the cross-implementation integration testing project would be the best way to ensure everyone actually can handle it in practice.

@bergsieker
Copy link
Collaborator

Closing based on Eric's response, but please reopen if you feel further discussion is warranted.

santigl pushed a commit to santigl/remote-apis that referenced this issue Aug 26, 2020
Some refactoring of our previous code, including:
- the interface is now defined in filemetadata.Cache
- the actual types are not exported.
- instead, factory functions are used for both the noop cache
  and the single-flight cache
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

3 participants