Don't duplicate paths (#1245)

* Don't duplicate paths

Fixes #1095

* Improve tests

* Revert output changes

* WS

DESCRIPTION

@@ -15,7 +15,7 @@ URL: https://rstudio.github.io/renv/
 BugReports: https://github.com/rstudio/renv/issues
 Imports: utils
 Suggests: BiocManager, cli, covr, devtools, jsonlite, knitr, miniUI, packrat, pak,
-    R6, remotes, reticulate, rmarkdown, rstudioapi, shiny, testthat, uuid, yaml
+    R6, remotes, reticulate, rmarkdown, rstudioapi, shiny, testthat, uuid, withr, yaml
 Encoding: UTF-8
 RoxygenNote: 7.2.3
 Roxygen: list(markdown = TRUE)

NEWS.md

@@ -1,6 +1,9 @@
 
 # renv 0.18.0  (UNRELEASED)
 
+* `renv::load()` no longer duplicates entries on the `PATH` environment variable
+  (#1095).
+
 * `renv` gains a new function `renv::checkout()`, for installing the
   latest-available packages from a repository. For example, one can
   use `renv::checkout(date = "2023-02-08")` to install the packages available

R/envvar.R

@@ -22,14 +22,11 @@ renv_envvar_clear <- function(key) {
 
 renv_envvar_modify <- function(envvar, value, prepend) {
 
-  old <- Sys.getenv(envvar, unset = NA)
+  old <- Sys.getenv(envvar, unset = "")
+  old <- strsplit(old, .Platform$path.sep)[[1]]
 
-  parts <- if (prepend)
-    c(value, if (!is.na(old)) old)
-  else
-    c(if (!is.na(old)) old, value)
-
-  new <- paste(unique(parts), collapse = .Platform$path.sep)
+  parts <- if (prepend) union(value, old) else union(old, value)
+  new <- paste(parts, collapse = .Platform$path.sep)
 
   names(new) <- envvar
   do.call(Sys.setenv, as.list(new))

tests/testthat/test-envvar.R

@@ -0,0 +1,22 @@
+context("envvar")
+
+test_that("renv_envvar_prepend doesn't duplicate paths", {
+  path_string <- function(...) {
+    paste(c(...), collapse = .Platform$path.sep)
+  }
+  withr::local_envvar(TESTPATH = path_string("a", "b", "c"))
+
+  renv_envvar_prepend("TESTPATH", "a")
+  expect_equal(Sys.getenv("TESTPATH"), path_string("a", "b", "c"))
+
+  renv_envvar_prepend("TESTPATH", "b")
+  expect_equal(Sys.getenv("TESTPATH"), path_string("b", "a", "c"))
+})
+
+test_that("renv_envvar_prepend works if var isn't set", {
+
+  renv_envvar_prepend("TESTPATH", "a")
+  defer(Sys.unsetenv("TESTPATH"))
+
+  expect_equal(Sys.getenv("TESTPATH"), "a")
+})