Skip to content

Commit

Permalink
Make files writable when they are exported.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
plietar committed Jul 8, 2024
1 parent 1fd09ff commit add8d26
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 0 deletions.
5 changes: 5 additions & 0 deletions R/outpack_root.R
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ file_export <- function(root, id, there, here, dest, overwrite, call = NULL) {
}
fs::file_copy(there_full, here_full, overwrite)
}

# Files in the archive and file store are (generally) read-only.
# When exporting for interactive use, it's easier on the user if we make them
# writable again.
fs::file_chmod(here_full, "u+w")
}


Expand Down
30 changes: 30 additions & 0 deletions tests/testthat/test-outpack-helpers.R
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
root <- create_temporary_root(use_file_store = TRUE)

id1 <- create_random_packet(root)
id2 <- create_random_packet(root)

# Just a bit of a sanity check. The two packets are random, so we'd expect
# them to have different contents.
expect_false(identical(
readRDS(file.path(root$path, "archive", "data", id1, "data.rds")),
readRDS(file.path(root$path, "archive", "data", id2, "data.rds"))))

dst <- temp_file()

suppressMessages(
orderly_copy_files(id1, files = "data.rds", dest = dst, root = root))

expect_identical(
readRDS(file.path(dst, "data.rds")),
readRDS(file.path(root$path, "archive", "data", id1, "data.rds")))

suppressMessages(
orderly_copy_files(id2, files = "data.rds", dest = dst, root = root))

expect_identical(
readRDS(file.path(dst, "data.rds")),
readRDS(file.path(root$path, "archive", "data", id2, "data.rds")))
})

0 comments on commit add8d26

Please sign in to comment.