Merge pull request #1743 from rstudio/misc-windows-fixes

Misc Windows fixes

R/py_require.R

@@ -33,9 +33,9 @@
 #'   dependencies. Many `uv` options can be customized via environment
 #'   variables, as described
 #'   [here](https://docs.astral.sh/uv/configuration/environment/). For example:
-#'   - If temporarily offline, set `Sys.setenv(UV_OFFLINE=1)`.
+#'   - If temporarily offline, set `Sys.setenv(UV_OFFLINE = "1")`.
 #'   - To use a different index: `Sys.setenv(UV_INDEX = "https://download.pytorch.org/whl/cpu")`.
-#'   - To allow resolving a prerelease dependency: `Sys.setenv(UV_PRERELEASE="allow")`.
+#'   - To allow resolving a prerelease dependency: `Sys.setenv(UV_PRERELEASE = "allow")`.
 #'
 #'   ## Installing from alternate sources
 #'
@@ -98,18 +98,19 @@
 #' @param action Determines how `py_require()` processes the provided
 #'   requirements. Options are:
 #'   - `add`: Adds the entries to the current set of requirements.
-#'   - `remove`: Removes _exact_ matches from the requirements list. For example,
-#'   if `"numpy==2.2.2"` is in the list, passing `"numpy"` with `action =
-#'   "remove"` will not remove it. Requests to remove nonexistent entries are
-#'   ignored.
+#'   - `remove`: Removes _exact_ matches from the requirements list. Requests to remove nonexistent entries are
+#'   ignored. For example, if `"numpy==2.2.2"` is in the list, passing `"numpy"`
+#'   with `action = "remove"` will not remove it.
 #'   - `set`: Clears all existing requirements and replaces them with the
 #'   provided ones. Packages and the Python version can be set independently.
 #'
-#' @param exclude_newer Restricts package versions to those published before a
+#' @param exclude_newer Limit package versions to those published before a
 #'   specified date. This offers a lightweight alternative to freezing package
 #'   versions, helping guard against Python package updates that break a
-#'   workflow. Once `exclude_newer` is set, only the `set` action can override
-#'   it.
+#'   workflow. Accepts strings formatted as RFC 3339 timestamps (e.g.,
+#'   "2006-12-02T02:07:43Z") and local dates in the same format (e.g.,
+#'   "2006-12-02") in your system's configured time zone. Once `exclude_newer`
+#'   is set, only the `set` action can override it.
 #'
 #' @returns `py_require()` is primarily called for its side effect of modifying
 #'   the manifest of "Python requirements" for the current R session  that
@@ -549,6 +550,7 @@ uv_binary <- function(bootstrap_install = TRUE) {
   if (bootstrap_install) {
     # Install 'uv' in the 'r-reticulate' sub-folder inside the user's cache directory
     # https://github.com/astral-sh/uv/blob/main/docs/configuration/installer.md
+    dir.create(dirname(uv), showWarnings = FALSE, recursive = TRUE)
     file_ext <- if (is_windows()) ".ps1" else ".sh"
     url <- paste0("https://astral.sh/uv/install", file_ext)
     install_uv <- tempfile("install-uv-", fileext = file_ext)
@@ -557,25 +559,27 @@ uv_binary <- function(bootstrap_install = TRUE) {
       return(NULL)
       # stop("Unable to download Python dependencies. Please install `uv` manually.")
     }
+    if(debug <- Sys.getenv("_RETICULATE_DEBUG_UV_") == "1")
+      system2 <- system2t
 
     if (is_windows()) {
-      system2("powershell", c(
-        "-ExecutionPolicy", "ByPass",
-        "-c",
-        paste0(
-          "$env:UV_UNMANAGED_INSTALL='", dirname(uv), "';", # shQuote()?
-          # 'Out-Null' makes installation silent
-          "irm ", install_uv, " | iex *> Out-Null"
+
+      withr::with_envvar(c("UV_UNMANAGED_INSTALL" = shortPathName(dirname(uv))), {
+        system2("powershell", c(
+          "-ExecutionPolicy", "ByPass", "-c",
+          sprintf("irm %s | iex", shortPathName(install_uv))),
+          stdout = if (debug) "" else FALSE,
+          stderr = if (debug) "" else FALSE
         )
-      ))
-    } else if (is_macos() || is_linux()) {
+      })
+
+    } else {
+
       Sys.chmod(install_uv, mode = "0755")
-      dir.create(dirname(uv), showWarnings = FALSE, recursive = TRUE)
-      system2(install_uv, c("--quiet"),
-        env = c(
-          paste0("UV_UNMANAGED_INSTALL=", maybe_shQuote(dirname(uv)))
-        )
-      )
+      withr::with_envvar(c("UV_UNMANAGED_INSTALL" = dirname(uv)), {
+        system2(install_uv, c(if (!debug) "--quiet"))
+      })
+
     }
   }
 
@@ -617,6 +621,8 @@ uv_get_or_create_env <- function(packages = py_reqs_get("packages"),
     if (is_positron())
       c(RUST_LOG = NA)
   ))
+  uv_output_file <- tempfile()
+  on.exit(unlink(uv_output_file), add = TRUE)
 
   uv_args <- c(
     "run",
@@ -627,17 +633,20 @@ uv_get_or_create_env <- function(packages = py_reqs_get("packages"),
     exclude_newer,
     packages,
     "--",
-    "python", "-c", "import sys; print(sys.executable);"
+    "python", "-c",
+    # chr(119) == "w", but avoiding a string literal to minimize the need for
+    # shell quoting shenanigans
+    "import sys; f=open(sys.argv[-1], chr(119)); f.write(sys.executable); f.close();",
+    uv_output_file
   )
 
   # debug print system call
-  if (Sys.getenv("_RETICULATE_DEBUG_UV_") == "1")
+  if (debug <- Sys.getenv("_RETICULATE_DEBUG_UV_") == "1")
     message(paste0(c(shQuote(uv), maybe_shQuote(uv_args)), collapse = " "))
 
-  env_python <- suppressWarnings(system2(uv, maybe_shQuote(uv_args), stdout = TRUE))
-  error_code <- attr(env_python, "status", TRUE)
+  error_code <- suppressWarnings(system2(uv, maybe_shQuote(uv_args)))
 
-  if (!is.null(error_code)) {
+  if (error_code) {
     cat("uv error code: ", error_code, "\n", sep = "", file = stderr())
     msg <- do.call(py_reqs_format, call_args)
     writeLines(c(msg, strrep("-", 73L)), con = stderr())
@@ -650,7 +659,10 @@ uv_get_or_create_env <- function(packages = py_reqs_get("packages"),
     stop("Call `py_require()` to remove or replace conflicting requirements.")
   }
 
-  env_python
+  ephemeral_python <- readLines(uv_output_file, warn = FALSE)
+  if (debug)
+    message("resolved ephemeral python: ", ephemeral_python)
+  ephemeral_python
 }
 
 #' uv run tool
@@ -741,6 +753,15 @@ uv_python_list <- function(uv = uv_binary()) {
   x
 }
 
+uvx_binary <- function(...) {
+  uv <- uv_binary(...)
+  if(is.null(uv)) {
+    return()
+  }
+  uvx <- file.path(dirname(uv), if (is_windows()) "uvx.exe" else "uvx")
+  if (file.exists(uvx)) uvx else NULL # print visible
+}
+
 resolve_python_version <- function(constraints = NULL, uv = uv_binary()) {
   constraints <- as.character(constraints %||% "")
   constraints <- trimws(unlist(strsplit(constraints, ",", fixed = TRUE)))

R/utils.R

@@ -647,15 +647,15 @@ maybe_shQuote <- function(x) {
 rm_all_reticulate_state <- function(external = FALSE) {
 
   rm_rf <- function(...)
-    unlink(path.expand(c(...)), recursive = TRUE, force = TRUE)
+    try(unlink(path.expand(c(...)), recursive = TRUE, force = TRUE))
 
   if (external) {
     if (!is.null(uv <- uv_binary(FALSE))) {
       system2(uv, c("cache", "clean"))
-      rm_rf(system2(uv, c("python", "dir"),
-                    env = "NO_COLOR=1", stdout = TRUE))
-      rm_rf(system2(uv, c("tool", "dir"),
-                    env = "NO_COLOR=1", stdout = TRUE))
+      withr::with_envvar(c("NO_COLOR"="1"), {
+        rm_rf(system2(uv, c("python", "dir"), stdout = TRUE))
+        rm_rf(system2(uv, c("tool", "dir"), stdout = TRUE))
+      })
     }
 
     if (nzchar(Sys.which("pip3")))
@@ -670,12 +670,12 @@ rm_all_reticulate_state <- function(external = FALSE) {
   rm_rf(virtualenv_path("r-reticulate"))
   for (venv in virtualenv_list()) {
     if (startsWith(venv, "r-"))
-      virtualenv_remove(venv, confirm = FALSE)
+      rm_rf(virtualenv_path(venv))
   }
   rm_rf(reticulate_cache_dir())
-  try(tools::R_user_dir("reticulate", "cache"))
-  try(tools::R_user_dir("reticulate", "data"))
-  try(tools::R_user_dir("reticulate", "config"))
+  rm_rf(tools::R_user_dir("reticulate", "cache"))
+  rm_rf(tools::R_user_dir("reticulate", "data"))
+  rm_rf(tools::R_user_dir("reticulate", "config"))
   invisible()
 }
 
@@ -687,7 +687,7 @@ reticulate_cache_dir <- function(...) {
     path.expand(rappdirs::user_cache_dir("r-reticulate", NULL))
   }
 
-  file.path(root, ...)
+  normalizePath(file.path(root, ...), mustWork = FALSE)
 }
 
 

src/python.cpp

@@ -2832,6 +2832,7 @@ PyOS_sighandler_t install_interrupt_handlers_() {
   // the correct handler.
 
   // First, install the Python handler
+  GILScope _gil;
   PyObject *main = PyImport_AddModule("__main__"); // borrowed ref
   PyObject *main_dict = PyModule_GetDict(main); // borrowed ref
   PyObjectPtr locals(PyDict_New());
@@ -2867,7 +2868,7 @@ PyObject* python_interrupt_handler(PyObject *module, PyObject *args)
   // it sees that trip_signals() had been called.
 
   // args will be (signalnum, frame), but we ignore them
-
+  GILScope _gil;
   if (R_interrupts_pending == 0) {
     // R won the race to handle the interrupt. The interrupt has already been
     // signaled as an R condition. There is nothing for this handler to do.

tests/testthat/test-finalize.R

@@ -14,12 +14,12 @@ class Foo:
     weakref.finalize(self, self.on_finalize)
 
   def on_finalize(self):
-    with open('%s', 'a') as f:
+    with open(r'%s', 'a') as f:
       f.write('Foo.finalize ran\\n')
 
 import atexit
 def on_exit():
-  with open('%s', 'a') as f:
+  with open(r'%s', 'a') as f:
     f.write('on_exit finalizer ran\\n')
 
 atexit.register(on_exit)

tests/testthat/test-interrupts.R

@@ -6,7 +6,11 @@ test_that("Running Python scripts can be interrupted", {
   time <- import("time", convert = TRUE)
 
   # interrupt this process shortly
-  system(paste("sleep 1 && kill -s INT", Sys.getpid()), wait = FALSE)
+  interruptor <- callr::r_bg(args = list(pid = Sys.getpid()), function(pid) {
+    Sys.sleep(1)
+    system2("kill", c("-s", "INT", pid))
+    # ps::ps_interrupt(ps::ps_handle(pid))
+  })
 
   # tell Python to sleep
   before <- Sys.time()
@@ -70,7 +74,7 @@ test_that("interrupts work when Python is running", {
   p$wait()
 
   expect_identical(p$get_exit_status(), 0L)
-  expect_identical(p$read_output(), "Caught interrupt; Finished!")
+  expect_identical(p$read_all_output(), "Caught interrupt; Finished!")
 
 })
 
@@ -124,7 +128,7 @@ test_that("interrupts can be caught by Python", {
 
   expect_identical(p$get_exit_status(), 0L)
   expect_identical(
-    p$read_output(),
+    p$read_all_output(),
     "Caught interrupt; Running finally; Python finished; R Finished!")
 
 })
@@ -186,7 +190,7 @@ test_that("interrupts can be caught by Python while calling R", {
 
   expect_identical(p$get_exit_status(), 0L)
   expect_identical(
-    p$read_output(),
+    p$read_all_output(),
     "Caught interrupt; Running finally; Python finished; R Finished!")
 
 })

tests/testthat/test-python-arrays.R

@@ -76,6 +76,8 @@ def apply_mask(x, mask):
     py_logical_array <- r_to_py(r_logical_array)
     py_index_array <- r_to_py(r_index_array)
 
+    if (is_windows()) # not sure why ints cast to doubles on windows 
+       storage.mode(r_index_array) <- "double"
     # check that round-triping gives an identical array
     expect_identical(r_logical_array, py_to_r(py_logical_array))
     expect_identical(r_index_array, py_to_r(py_index_array))