From ce7c112ba3d75cf48e4dd6310b3140ab0ec3b486 Mon Sep 17 00:00:00 2001
From: Andrew Bates <andrewbates73@gmail.com>
Date: Fri, 20 Jan 2023 00:24:19 -0800
Subject: [PATCH] Don't override user-set option for storage directory (#171)

---
 NEWS.md                       |  2 ++
 R/caching.R                   | 16 ++++++++++++----
 R/zzz.R                       | 14 +++++++++++---
 README.md                     |  5 +++--
 tests/testthat/test-caching.R | 11 +++++++++++
 tests/testthat/test-zzz.R     | 14 +++++++++++++-
 6 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index 69e4ec3..b3dfd7c 100644
--- a/NEWS.md
+++ b/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
 
diff --git a/R/caching.R b/R/caching.R
index a607067..44ff869 100644
--- a/R/caching.R
+++ b/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
diff --git a/R/zzz.R b/R/zzz.R
index 386a726..298699c 100644
--- a/R/zzz.R
+++ b/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)
 }
diff --git a/README.md b/README.md
index e5a0388..3e6e85d 100644
--- a/README.md
+++ b/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>>)`
diff --git a/tests/testthat/test-caching.R b/tests/testthat/test-caching.R
index a34d9a0..5bd13dc 100644
--- a/tests/testthat/test-caching.R
+++ b/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)
+})
diff --git a/tests/testthat/test-zzz.R b/tests/testthat/test-zzz.R
index 940bb0b..843f8b4 100644
--- a/tests/testthat/test-zzz.R
+++ b/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)
+})