From de28b10f46580ad679b2d756722bcb1c8cae480b Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Thu, 11 Jun 2026 09:17:07 -0400 Subject: [PATCH 1/5] Preserve Windows PATH when installing launchers --- R/install.R | 61 ++++++++++++++++++++++++++--------- inst/add-path-entry.ps1 | 26 +++++++++++++-- tests/testthat/test-install.R | 30 +++++++++++++++++ 3 files changed, 100 insertions(+), 17 deletions(-) diff --git a/R/install.R b/R/install.R index 15ae403..a15d7b7 100644 --- a/R/install.R +++ b/R/install.R @@ -485,23 +485,23 @@ ensure_path_windows <- function(destdir = rapp_install_dir()) { stopifnot(.Platform$OS.type == "windows") destdir <- normalizePath(destdir, winslash = "\\", mustWork = TRUE) - # Check if we're already on PATH. If we are, do nothing - # Read current PATH from HKCU\Environment - path <- get_env_win_registry("Path") - path <- strsplit(path, ";", fixed = TRUE)[[1L]] - path <- path[nzchar(path)] - path <- unique(path) - - path_norm <- function(x) tolower(normalizePath(x, mustWork = FALSE)) - present <- path_norm(destdir) %in% path_norm(path) + if (path_has_dir_windows(destdir)) { + return(FALSE) + } + + # Check if we're already on the user-level PATH. If we are, do nothing. + # Read current PATH from HKCU\Environment. + path <- unique(raw_path_entries(get_env_win_registry("Path"))) + present <- path_norm_windows(destdir) %in% path_norm_windows(path) if (present) { return(FALSE) } # We are not on the PATH yet, we have to add it # Pass the new path entry via envvar to avoid quoting and encoding shenanigans - # also, propogate the new updated PATH from the registry to this R session + # and also propagate the new entry to this R session. old <- Sys.getenv("RAPP_NEW_PATH_ENTRY", NA_character_) + old_path <- Sys.getenv("PATH") Sys.setenv("RAPP_NEW_PATH_ENTRY" = destdir) on.exit({ if (is.na(old)) { @@ -509,14 +509,27 @@ ensure_path_windows <- function(destdir = rapp_install_dir()) { } else { Sys.setenv("RAPP_NEW_PATH_ENTRY" = old) } - Sys.setenv("PATH" = get_env_win_registry("Path")) }) script <- shQuote(utils::shortPathName( system.file("add-path-entry.ps1", package = "Rapp") )) args <- c("-NoProfile", "-ExecutionPolicy", "Bypass", "-File", script) - system2("powershell", args) + status <- system2("powershell", args) + if (!identical(status, 0L)) { + warning( + "Could not add ", + destdir, + " to the user-level PATH; powershell exited with status ", + status, + ".", + call. = FALSE + ) + return(FALSE) + } + + Sys.setenv("PATH" = prepend_path_entry(destdir, old_path)) + TRUE } get_env_win_registry <- function(name) { @@ -633,10 +646,28 @@ path_has_dir <- function(dir, env_path = Sys.getenv("PATH")) { normalizePath(dir, mustWork = FALSE) %in% path_entries(env_path) } -path_entries <- function(env_path = Sys.getenv("PATH")) { +path_has_dir_windows <- function(dir, env_path = Sys.getenv("PATH")) { + path_norm_windows(dir) %in% path_norm_windows(raw_path_entries(env_path)) +} + +path_norm_windows <- function(x) { + tolower(normalizePath(x, winslash = "\\", mustWork = FALSE)) +} + +prepend_path_entry <- function(dir, env_path = Sys.getenv("PATH")) { + paste(c(dir, raw_path_entries(env_path)), collapse = .Platform$path.sep) +} + +raw_path_entries <- function(env_path = Sys.getenv("PATH")) { + if (is.null(env_path) || !length(env_path) || is.na(env_path)) { + return(character()) + } entries <- strsplit(env_path, .Platform$path.sep, fixed = TRUE)[[1L]] - entries <- entries[nzchar(entries)] - normalizePath(entries, mustWork = FALSE) + entries[nzchar(entries)] +} + +path_entries <- function(env_path = Sys.getenv("PATH")) { + normalizePath(raw_path_entries(env_path), mustWork = FALSE) } is_macos <- function() { diff --git a/inst/add-path-entry.ps1 b/inst/add-path-entry.ps1 index de85c13..7cf4b45 100644 --- a/inst/add-path-entry.ps1 +++ b/inst/add-path-entry.ps1 @@ -10,18 +10,40 @@ $NewPathEntry = if ($env:RAPP_NEW_PATH_ENTRY) { $env:RAPP_NEW_PATH_ENTRY } elsei Write-Verbose "Adding $NewPathEntry to your user-level PATH" +function Normalize-PathEntry([string]$PathEntry) { + try { + $PathEntry = (Resolve-Path -LiteralPath $PathEntry -ErrorAction Stop).ProviderPath + } catch { + } + + try { + $PathEntry = [System.IO.Path]::GetFullPath($PathEntry) + } catch { + } + + return $PathEntry.TrimEnd('\').ToLowerInvariant() +} + # Read unexpanded PATH components $PathEntries = (Get-Item -LiteralPath $RegistryPath).GetValue( 'Path', '', 'DoNotExpandEnvironmentNames') -split ';' -ne '' -if ($NewPathEntry -in $PathEntries) { +$NewPathEntryNorm = Normalize-PathEntry $NewPathEntry +$PathEntryNorms = $PathEntries | ForEach-Object { Normalize-PathEntry $_ } + +if ($NewPathEntryNorm -in $PathEntryNorms) { Write-Verbose "Install directory $NewPathEntry already on PATH!" - exit 1 + exit 0 } # Prepend to PATH $NewPath = (,$NewPathEntry + $PathEntries) -join ';' +if ($NewPath.Length -gt 32767) { + Write-Error "Adding $NewPathEntry would make your user-level PATH $($NewPath.Length) characters, exceeding the Windows environment variable limit of 32767." + exit 3 +} + # Update registry as REG_EXPAND_SZ Set-ItemProperty -Type ExpandString -LiteralPath $RegistryPath Path -Value $NewPath diff --git a/tests/testthat/test-install.R b/tests/testthat/test-install.R index 7268a85..80a0c35 100644 --- a/tests/testthat/test-install.R +++ b/tests/testthat/test-install.R @@ -213,6 +213,36 @@ test_that("non-Rapp executables respect overwrite flag", { }) +test_that("install_pkg_cli_apps leaves Windows PATH alone when destdir is already available", { + skip_if_not(is_windows()) + + destdir <- tempfile("rapp-bin-win-path") + dir.create(destdir, recursive = TRUE) + on.exit(unlink(destdir, recursive = TRUE), add = TRUE) + + current_path <- Sys.getenv("PATH") + path_with_destdir <- paste(destdir, current_path, sep = .Platform$path.sep) + withr::local_envvar( + RAPP_NO_MODIFY_PATH = NA, + PATH = path_with_destdir + ) + + expect_false(Rapp:::ensure_path_windows(destdir)) + expect_identical(Sys.getenv("PATH"), path_with_destdir) +}) + + +test_that("prepend_path_entry keeps the existing process PATH", { + env_path <- paste("first", "second", sep = .Platform$path.sep) + + expect_identical( + Rapp:::prepend_path_entry("new", env_path), + paste("new", "first", "second", sep = .Platform$path.sep) + ) + expect_identical(Rapp:::prepend_path_entry("new", ""), "new") +}) + + test_that("install_pkg_cli_apps adds default macOS install dir to PATH setup", { skip_if_not(identical(Sys.info()[["sysname"]], "Darwin")) skip_on_cran() From d1a538a66cfb34ca5b4d6a0d3456a3cf83b58100 Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Thu, 11 Jun 2026 09:23:27 -0400 Subject: [PATCH 2/5] Shorten Windows PATH handling for launcher install --- R/install.R | 74 ++++++++++++++++++++++------------- inst/add-path-entry.ps1 | 12 +++++- tests/testthat/test-install.R | 8 ++-- 3 files changed, 62 insertions(+), 32 deletions(-) diff --git a/R/install.R b/R/install.R index a15d7b7..1b0f767 100644 --- a/R/install.R +++ b/R/install.R @@ -483,17 +483,30 @@ ensure_path_windows <- function(destdir = rapp_install_dir()) { return(FALSE) } stopifnot(.Platform$OS.type == "windows") - destdir <- normalizePath(destdir, winslash = "\\", mustWork = TRUE) - - if (path_has_dir_windows(destdir)) { + destdir <- normalizePath(destdir, mustWork = TRUE) + path_entry <- utils::shortPathName(destdir) + if ( + is.na(path_entry) || + !nzchar(path_entry) || + nchar(path_entry) >= nchar(destdir) + ) { + path_entry <- destdir + } + + if ( + path_has_dir(destdir, ignore_case = TRUE) || + path_has_dir(path_entry, ignore_case = TRUE) + ) { return(FALSE) } # Check if we're already on the user-level PATH. If we are, do nothing. # Read current PATH from HKCU\Environment. - path <- unique(raw_path_entries(get_env_win_registry("Path"))) - present <- path_norm_windows(destdir) %in% path_norm_windows(path) - if (present) { + user_path <- get_env_win_registry("Path") + if ( + path_has_dir(destdir, user_path, ignore_case = TRUE) || + path_has_dir(path_entry, user_path, ignore_case = TRUE) + ) { return(FALSE) } @@ -502,7 +515,7 @@ ensure_path_windows <- function(destdir = rapp_install_dir()) { # and also propagate the new entry to this R session. old <- Sys.getenv("RAPP_NEW_PATH_ENTRY", NA_character_) old_path <- Sys.getenv("PATH") - Sys.setenv("RAPP_NEW_PATH_ENTRY" = destdir) + Sys.setenv("RAPP_NEW_PATH_ENTRY" = path_entry) on.exit({ if (is.na(old)) { Sys.unsetenv("RAPP_NEW_PATH_ENTRY") @@ -519,7 +532,7 @@ ensure_path_windows <- function(destdir = rapp_install_dir()) { if (!identical(status, 0L)) { warning( "Could not add ", - destdir, + path_entry, " to the user-level PATH; powershell exited with status ", status, ".", @@ -528,7 +541,12 @@ ensure_path_windows <- function(destdir = rapp_install_dir()) { return(FALSE) } - Sys.setenv("PATH" = prepend_path_entry(destdir, old_path)) + Sys.setenv( + "PATH" = paste( + c(path_entry, path_entries(old_path, normalize = FALSE)), + collapse = .Platform$path.sep + ) + ) TRUE } @@ -642,32 +660,34 @@ write_macos_path_lines <- function(profile, lines, profile_display) { ) } -path_has_dir <- function(dir, env_path = Sys.getenv("PATH")) { - normalizePath(dir, mustWork = FALSE) %in% path_entries(env_path) -} - -path_has_dir_windows <- function(dir, env_path = Sys.getenv("PATH")) { - path_norm_windows(dir) %in% path_norm_windows(raw_path_entries(env_path)) -} +path_has_dir <- function( + dir, + env_path = Sys.getenv("PATH"), + ignore_case = is_windows() +) { + dir <- normalizePath(dir, mustWork = FALSE) + entries <- path_entries(env_path) -path_norm_windows <- function(x) { - tolower(normalizePath(x, winslash = "\\", mustWork = FALSE)) -} + if (ignore_case) { + dir <- tolower(dir) + entries <- tolower(entries) + } -prepend_path_entry <- function(dir, env_path = Sys.getenv("PATH")) { - paste(c(dir, raw_path_entries(env_path)), collapse = .Platform$path.sep) + dir %in% entries } -raw_path_entries <- function(env_path = Sys.getenv("PATH")) { +path_entries <- function(env_path = Sys.getenv("PATH"), normalize = TRUE) { if (is.null(env_path) || !length(env_path) || is.na(env_path)) { return(character()) } - entries <- strsplit(env_path, .Platform$path.sep, fixed = TRUE)[[1L]] - entries[nzchar(entries)] -} -path_entries <- function(env_path = Sys.getenv("PATH")) { - normalizePath(raw_path_entries(env_path), mustWork = FALSE) + entries <- strsplit(env_path, .Platform$path.sep, fixed = TRUE)[[1L]] + entries <- entries[nzchar(entries)] + if (normalize) { + normalizePath(entries, mustWork = FALSE) + } else { + entries + } } is_macos <- function() { diff --git a/inst/add-path-entry.ps1 b/inst/add-path-entry.ps1 index 7cf4b45..d61d3ed 100644 --- a/inst/add-path-entry.ps1 +++ b/inst/add-path-entry.ps1 @@ -40,7 +40,17 @@ if ($NewPathEntryNorm -in $PathEntryNorms) { $NewPath = (,$NewPathEntry + $PathEntries) -join ';' if ($NewPath.Length -gt 32767) { - Write-Error "Adding $NewPathEntry would make your user-level PATH $($NewPath.Length) characters, exceeding the Windows environment variable limit of 32767." + $Hint = @( + "Rapp uses a Windows short path when one is available." + "Remove stale entries from your user-level PATH or choose a shorter launcher directory with RAPP_BIN_DIR." + "Windows long-path support does not increase this environment-variable limit." + ) -join ' ' + $Message = @( + "Adding $NewPathEntry would make your user-level PATH $($NewPath.Length) characters," + "exceeding the Windows environment variable limit of 32767." + $Hint + ) -join ' ' + Write-Error $Message exit 3 } diff --git a/tests/testthat/test-install.R b/tests/testthat/test-install.R index 80a0c35..6b3c6e2 100644 --- a/tests/testthat/test-install.R +++ b/tests/testthat/test-install.R @@ -232,14 +232,14 @@ test_that("install_pkg_cli_apps leaves Windows PATH alone when destdir is alread }) -test_that("prepend_path_entry keeps the existing process PATH", { +test_that("path_entries can keep the existing process PATH raw", { env_path <- paste("first", "second", sep = .Platform$path.sep) expect_identical( - Rapp:::prepend_path_entry("new", env_path), - paste("new", "first", "second", sep = .Platform$path.sep) + Rapp:::path_entries(env_path, normalize = FALSE), + c("first", "second") ) - expect_identical(Rapp:::prepend_path_entry("new", ""), "new") + expect_identical(Rapp:::path_entries("", normalize = FALSE), character()) }) From 92f3831199a953a694c41e3855a186ee180ca50a Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Thu, 11 Jun 2026 09:32:24 -0400 Subject: [PATCH 3/5] Fix Windows PATH handling for launcher install --- .Rbuildignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.Rbuildignore b/.Rbuildignore index 5f19960..bb578bb 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -9,3 +9,4 @@ ^\.vscode$ ^deps$ ^\.git-blame-ignore-revs$ +^\.git$ From 39d8b70ac45891bf15ea75d746b9ef60b3bc9a2a Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Thu, 11 Jun 2026 09:39:29 -0400 Subject: [PATCH 4/5] Add NEWS entry for Windows PATH fix --- NEWS.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS.md b/NEWS.md index 6c22466..23d0137 100644 --- a/NEWS.md +++ b/NEWS.md @@ -45,6 +45,11 @@ directory to the user's zsh profile on macOS when it is not already on `PATH`. It respects `ZDOTDIR` and warns if the profile cannot be updated (#35). +- On Windows, `install_pkg_cli_apps()` no longer replaces the current + process `PATH` with only the user-level `Path` after adding launcher + directories. It now avoids duplicate entries, uses a short path when + available, and reports too-long user-level `Path` values with a + remediation hint (#38). # Rapp 0.3.0 From 1f453ac66530a6f7af4f345b339c2a83e90e44a8 Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Thu, 11 Jun 2026 09:46:48 -0400 Subject: [PATCH 5/5] Persist Windows PATH additions when PATH is transient --- R/install.R | 39 +++++++++++++++++++++-------------- tests/testthat/test-install.R | 22 ++++++++++++++++++-- 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/R/install.R b/R/install.R index 1b0f767..b7c9575 100644 --- a/R/install.R +++ b/R/install.R @@ -493,20 +493,28 @@ ensure_path_windows <- function(destdir = rapp_install_dir()) { path_entry <- destdir } - if ( - path_has_dir(destdir, ignore_case = TRUE) || - path_has_dir(path_entry, ignore_case = TRUE) - ) { - return(FALSE) + is_on_path <- function(env_path = Sys.getenv("PATH")) { + path_has_dir(destdir, env_path, ignore_case = TRUE) || + path_has_dir(path_entry, env_path, ignore_case = TRUE) + } + update_process_path <- function(env_path = Sys.getenv("PATH")) { + if (is_on_path(env_path)) { + return(FALSE) + } + Sys.setenv( + "PATH" = paste( + c(path_entry, path_entries(env_path, normalize = FALSE)), + collapse = .Platform$path.sep + ) + ) + TRUE } # Check if we're already on the user-level PATH. If we are, do nothing. # Read current PATH from HKCU\Environment. user_path <- get_env_win_registry("Path") - if ( - path_has_dir(destdir, user_path, ignore_case = TRUE) || - path_has_dir(path_entry, user_path, ignore_case = TRUE) - ) { + if (is_on_path(user_path)) { + update_process_path() return(FALSE) } @@ -528,7 +536,7 @@ ensure_path_windows <- function(destdir = rapp_install_dir()) { system.file("add-path-entry.ps1", package = "Rapp") )) args <- c("-NoProfile", "-ExecutionPolicy", "Bypass", "-File", script) - status <- system2("powershell", args) + status <- run_powershell(args) if (!identical(status, 0L)) { warning( "Could not add ", @@ -541,12 +549,7 @@ ensure_path_windows <- function(destdir = rapp_install_dir()) { return(FALSE) } - Sys.setenv( - "PATH" = paste( - c(path_entry, path_entries(old_path, normalize = FALSE)), - collapse = .Platform$path.sep - ) - ) + update_process_path(old_path) TRUE } @@ -554,6 +557,10 @@ get_env_win_registry <- function(name) { utils::readRegistry("Environment", hive = "HCU", view = "default")[[name]] } +run_powershell <- function(args) { + system2("powershell", args) +} + ensure_path_macos <- function(destdir = rapp_install_dir()) { if (Sys.getenv("RAPP_NO_MODIFY_PATH") != "") { return(FALSE) diff --git a/tests/testthat/test-install.R b/tests/testthat/test-install.R index 6b3c6e2..1ee8938 100644 --- a/tests/testthat/test-install.R +++ b/tests/testthat/test-install.R @@ -213,7 +213,7 @@ test_that("non-Rapp executables respect overwrite flag", { }) -test_that("install_pkg_cli_apps leaves Windows PATH alone when destdir is already available", { +test_that("install_pkg_cli_apps persists Windows PATH when destdir is only transient", { skip_if_not(is_windows()) destdir <- tempfile("rapp-bin-win-path") @@ -227,8 +227,26 @@ test_that("install_pkg_cli_apps leaves Windows PATH alone when destdir is alread PATH = path_with_destdir ) - expect_false(Rapp:::ensure_path_windows(destdir)) + powershell_calls <- list() + testthat::local_mocked_bindings( + get_env_win_registry = function(name) { + expect_identical(name, "Path") + "C:\\Windows\\system32" + }, + run_powershell = function(args) { + powershell_calls <<- append(powershell_calls, list(list( + args = args, + path_entry = Sys.getenv("RAPP_NEW_PATH_ENTRY") + ))) + 0L + }, + .package = "Rapp" + ) + + expect_true(Rapp:::ensure_path_windows(destdir)) expect_identical(Sys.getenv("PATH"), path_with_destdir) + expect_length(powershell_calls, 1L) + expect_same_path(powershell_calls[[1L]][["path_entry"]], destdir) })