Make files writable when they are exported. #149
Conversation
| @@ -157,3 +157,33 @@ test_that("good error message if file not found in packet", { | |||
| i = "For 'd.txt' did you mean 'a.txt', 'b.txt' or 'c.txt'", | |||
| i = "Remember that all orderly paths are case sensitive")) | |||
| }) | |||
|
|
|||
|
|
|||
| test_that("Can overwrite when copying files from packet", { | |||
There was a problem hiding this comment.
can you add a test to verify that orderly_dependency is still copying files over read-only within a packet run? It's not immediately obvious to me from the implementation change that this will the the case. TBH I've entirely forgotten how the various bits of file exporting play together so it would be good to prevent any regression if we change the implementation
There was a problem hiding this comment.
ok, looks like this is due to
outpack_packet_file_mark(packet, result$files$here, "immutable")
in the active context, so that's nice
There was a problem hiding this comment.
file_mark only adds a check the file's hash at the end of the packet run, it doesn't pro-actively prevent files from being modified (albeit the result is basically the same I suppose).
file_export, which I've modified here, does not get called by the orderly_run flow. I'll add a test still.
There was a problem hiding this comment.
Nope, I was wrong about that file_export not being called. With this PR, files in the draft folder become mutable again (as they were before #141).
The outpack_packet_file_mark still does it's job of checking at the end of the packet, so in my opinion that's fine. I've added an explicit test for this.
Since [orderly2#141], orderly marks all files in the archive as read-only. Similarly, files in the file store have always been read-only. This helps prevent accidental corruption by users. However this has had the undesirable side-effect of copying the files with the read-only bit set, even when exporting for interactive use by the user. One scenario where this has come up is a user running `orderly_dependency` interactively twice. They would expect the second call to succeed and overwrite the existing files. However this was not the case, and instead the second call fails as the read-only files cannot be overwritten. The user ends up having to manually delete these files (or mark them writable) everytime they want to update their copy. This change marks all files that are exported as writable. This does not apply to calls to `orderly_dependency` from a job context, in which case the files are still read-only. [orderly2#141]: #141
plietar
force-pushed
the
mrc-5495-writable
branch
from
7cb1d5b to
d872996
Compare
Since orderly2#141, orderly marks all files in the archive as read-only. Similarly, files in the file store have always been read-only. This helps prevent accidental corruption by users.
However this has had the undesirable side-effect of copying the files with the read-only bit set, even when exporting for interactive use by the user.
One scenario where this has come up is a user running
orderly_dependencyinteractively twice. They would expect the second call to succeed and overwrite the existing files. However this was not the case, and instead the second call fails as the read-only files cannot be overwritten. The user ends up having to manually delete these files (or mark them writable) everytime they want to update their copy.This change marks all files that are exported as writable. This does not apply to calls to
orderly_dependencyfrom a job context, in which case the files are still read-only.