Don't override user-set option for storage directory (#171)

NEWS.md

@@ -1,6 +1,8 @@
 # staged.dependecies 0.2.8
 
 * Updated code for changes in `tidyselect 1.2.0`.
+* Fixed issue with storage directory being overwritten if set by user.
+* Updated copying of config file so it doesn't fail if file already exists.
 
 # staged.dependencies 0.2.7
 

R/caching.R

@@ -36,10 +36,18 @@ clear_cache <- function(pattern = "*") {
 # copies example config file to package settings directory
 # fails if copy did not work
 copy_config_to_storage_dir <- function() {
-  stopifnot(file.copy(
-    system.file("config.yaml", package = "staged.dependencies", mustWork = TRUE),
-    get_storage_dir()
-  ))
+
+  path <- file.path(get_storage_dir(), "config.yaml")
+
+  if (!file.exists(path)) {
+    copied_ok <- file.copy(
+      system.file("config.yaml", package = "staged.dependencies", mustWork = TRUE),
+      get_storage_dir()
+    )
+    if (!copied_ok) {
+      stop("Could not copy config.yaml to storage directory")
+    }
+  }
 }
 
 # directory where repo is cached locally

R/zzz.R

@@ -4,8 +4,7 @@ CONFIG_FILENAME <- "config.yaml" # staged.dependencies._storage_dir
 # Use this function when you want to change the storage directory while the package is already loaded
 # It does not delete the old storage directory
 setup_storage_dir <- function(storage_dir) {
-  options(staged.dependencies._storage_dir = storage_dir)
-
+  set_storage_dir(storage_dir)
   # only copy config if storage dir does not exist
   if (!dir.exists(storage_dir)) {
     dir.create(storage_dir)
@@ -23,7 +22,16 @@ get_storage_dir <- function() {
   options()$staged.dependencies._storage_dir
 }
 
+set_storage_dir <- function(storage_dir) {
+  options("staged.dependencies._storage_dir" = storage_dir)
+}
+
 .onLoad <- function(libname, pkgname) {
+
+  storage_dir <- getOption(
+    "staged.dependencies._storage_dir",
+    path.expand("~/.staged.dependencies")
+  )
   op <- options()
   op.package <- list(
     # mapping from hosts to
@@ -34,5 +42,5 @@ get_storage_dir <- function() {
   )
   toset <- !(names(op.package) %in% names(op))
   if (any(toset)) options(op.package[toset])
-  setup_storage_dir(path.expand("~/.staged.dependencies"))
+  setup_storage_dir(storage_dir)
 }

README.md

@@ -29,8 +29,9 @@ The package also provides:
 
 ## Usage
 
-The directory `~/.staged.dependencies` as well as a dummy config file are created whenever the package is loaded and
-the directory does not exist and this is where checked out repositories are cached.
+### Storage directory
+
+By deault, the directory `~/.staged.dependencies` as well as a dummy config file are created whenever the package is loaded and the directory does not exist. This is where checked out repositories are cached. To use a different location, you can set the option `staged.dependencies._storage_dir` before loading the package. Note that on Windows, the path `~/.staged.dependencies` may be a subdirectory of One Drive and this can lead to [problems](https://github.com/openpharma/staged.dependencies/issues/169).
 
 Note staged.dependencies requires a git signature setup this can be checked with `git2r::default_signature(".")`.
 If this is not setup it can be created using `git2r::config(git2r::repository("."), user.name = <<name>>, user.email = <<email>>)`

tests/testthat/test-caching.R

@@ -242,3 +242,14 @@ test_that("copy_renv_profiles does not copy any files if no /profiles folder", {
     expect_length(fs::dir_ls("to"), 0)
   })
 })
+
+test_that("copy_config_to_storage_dir", {
+  storage_dir <- get_storage_dir()
+  on.exit(set_storage_dir(storage_dir))
+
+  dir <- tempdir()
+  set_storage_dir(dir)
+  expect_error(copy_config_to_storage_dir(), NA)
+  # doesn't fail if config.yaml already exists
+  expect_error(copy_config_to_storage_dir(), NA)
+})

tests/testthat/test-zzz.R

@@ -10,7 +10,8 @@ test_that("setup_storage_dir works", {
 
   # modify storage directory to a new random one
   new_storage_dir <- tempfile("random_storage_dir")
-  on.exit(setup_storage_dir(TESTS_STORAGE_DIR), add = TRUE)
+  set_storage_dir(new_storage_dir)
+  on.exit({setup_storage_dir(TESTS_STORAGE_DIR)}, add = TRUE)
   setup_storage_dir(new_storage_dir)
 
   expect_equal(get_storage_dir(), new_storage_dir)
@@ -23,3 +24,14 @@ test_that("setup_storage_dir works", {
     file.path(new_storage_dir, "packages_cache")
   )
 })
+
+test_that("get/set_storage_dir", {
+  dir <- tempdir()
+
+  withr::with_options(list("staged.dependencies._storage_dir" = dir), {
+    expect_equal(get_storage_dir(), dir)
+  })
+
+  set_storage_dir(dir)
+  expect_equal(get_storage_dir(), dir)
+})