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

Refactor installAs for out-of-workspace copies #1422

Open
ag-eitilt opened this issue Sep 20, 2023 · 0 comments
Open

Refactor installAs for out-of-workspace copies #1422

ag-eitilt opened this issue Sep 20, 2023 · 0 comments
Labels
breaking-change enhancement New feature or request

Comments

@ag-eitilt
Copy link
Collaborator

After #1416 and the surrounding discussion have settled on adding the invariant "within the workspace" to the Path type contract, installAs and installIn in their current forms are (A) potential ways of breaking that contract and (B) not particularly useful if that contract is enforced. If, on the other hand, they were refactored to return Result Unit Error rather than Result Path Error, they could very cleanly fulfil their intended purpose of copying build artifacts to system locations without exposing holes in the API.

This is a breaking change, though, as they have been used to shuffle files around within the workspace in the past. However, claimFileAsPath: String => String => Result Path Error can serve a similar purpose, and we can introduce a copy: String => Path => Result Path Error which does restrict the destination to fill the gap where we already have the file as a Path (and it would finally have the proper semantics unlike every one of the current functions).

installIn becomes much less relevant because we aren't usually going to have a Path directory to install the file into unless it's an intra-workspace copy, but we are going to want a variant which takes an access mode to apply; user/group ownership might be helpful but is likely not going to be used often enough to include, when the alternative is following the install with a simple enough localRunner job.

@ag-eitilt ag-eitilt added enhancement New feature or request breaking-change labels Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant