Repair the separators on the staged and backup executable paths

tempfile() joins with a backslash on Windows, so staging beside a WSL
destination produced "//wsl$/distro/path/to/dir\exe-new-1234". The Win32
calls tolerate the mixed separators, but wsl_safe_path() only rewrites
the prefix, so the POSIX chmod inside WSL was handed a path that does not
exist and every real compile failed. The previous code chmod'ed the
destination, which had been through repair_path() already, and ignored
the status besides, so this only surfaced once the staged candidate
became the thing being made executable and its status was checked.

Both temporary paths now go through repair_path(). That also collapses
the duplicated separator withr::local_tempdir() can return, so the
snapshot transforms match every spelling of the fixture directory rather
than the one literal form.

Also clears variables_ in format(overwrite_file = TRUE). The program on
disk is rewritten and stan_code_ reloaded from it, but anything already
parsed stayed cached, so $code() and $variables() could describe
different programs and the fitting methods validate data and initial
values against $variables().

Author: jgabry

R/model.R

@@ -1291,6 +1291,11 @@ format <- function(overwrite_file = FALSE,
   cat(run_log$stdout, file = out_file, sep = "\n")
   if (isTRUE(overwrite_file)) {
     private$stan_code_ <- readLines(self$stan_file())
+    # The program on disk has been rewritten, so anything parsed from it is
+    # stale. $variables() reparses when this is NULL; leaving it would let
+    # $code() and $variables() describe different programs, and the fitting
+    # methods validate data and inits against $variables(). (#1228)
+    private$variables_ <- NULL
   }
 
   invisible(TRUE)

R/utils.R

@@ -302,7 +302,11 @@ copy_temp_files <-
 #'   signalling from here would unwind before the caller could record the state
 #'   describing it.
 install_executable <- function(from, to) {
-  candidate <- tempfile(pattern = "exe-new-", tmpdir = dirname(to))
+  # repair_path() because tempfile() joins with a backslash on Windows, giving
+  # "//wsl$/distro/path/to/dir\\exe-new-1234". The Win32 calls below tolerate the
+  # mixed separators, but wsl_safe_path() only rewrites the prefix, so the POSIX
+  # chmod inside WSL would be handed a path that does not exist.
+  candidate <- repair_path(tempfile(pattern = "exe-new-", tmpdir = dirname(to)))
   # Discarding the staged copy can fail too, so the diagnostics say where it was
   # left rather than implying it is gone and sending the user looking for a file
   # that is still there.
@@ -339,7 +343,7 @@ install_executable <- function(from, to) {
 
   backup <- NULL
   if (file.exists(to)) {
-    backup <- tempfile(pattern = "exe-old-", tmpdir = dirname(to))
+    backup <- repair_path(tempfile(pattern = "exe-old-", tmpdir = dirname(to)))
     if (!isTRUE(suppressWarnings(file.rename(to, backup)))) {
       stop(
         "Could not move the existing executable '", to, "' aside. ",

tests/testthat/test-model-compile.R

@@ -582,7 +582,9 @@ test_that("a leftover backup doesn't unwind a compile when warnings are errors",
       # "<dir>\exe-old-1234", since tempfile() joins with a backslash.
       transform = function(lines) {
         lines <- gsub("\\\\", "/", lines)
-        lines <- gsub(gsub("\\\\", "/", model_dir), "<dir>", lines, fixed = TRUE)
+        for (dir in unique(c(model_dir, repair_path(model_dir)))) {
+          lines <- gsub(dir, "<dir>", lines, fixed = TRUE)
+        }
         gsub("exe-old-[0-9a-f]+", "exe-old-<random>", lines)
       }
     )
@@ -1107,6 +1109,31 @@ test_that("cmdstan_model cpp_options dont captialize cxxflags ", {
   expect_output(print(out), "-Dsomething_not_used")
 })
 
+test_that("format(overwrite_file = TRUE) refreshes cached variables", {
+  model_dir <- withr::local_tempdir()
+  stan_file <- write_stan_file(
+    "parameters { real alpha; } model { alpha ~ std_normal(); }",
+    dir = model_dir,
+    basename = "reformat.stan"
+  )
+  model <- cmdstan_model(stan_file, compile = FALSE)
+  expect_equal(names(model$variables()$parameters), "alpha")
+
+  # The program is edited behind the object's back, then formatted in place.
+  # $format() reloads $code() from disk, so a cached $variables() would go on
+  # describing a different program than $code() does -- and the fitting methods
+  # validate data and initial values against $variables(). (#1228)
+  writeLines(
+    "parameters { real beta; } model { beta ~ std_normal(); }",
+    stan_file
+  )
+  model$format(overwrite_file = TRUE, quiet = TRUE)
+
+  expect_equal(names(model$variables()$parameters), "beta")
+  expect_match(paste(model$code(), collapse = " "), "beta")
+})
+
+
 test_that("format() works", {
   code <- "
   parameters {

tests/testthat/test-utils.R

@@ -232,10 +232,19 @@ local_exe_fixture <- function(destination_exists = TRUE,
 # and tempfile() use backslashes, so tempfile(tmpdir = dirname(to)) yields
 # "C:/a/b\exe-new-1234".
 exe_path_transform <- function(fixture) {
-  dir <- gsub("\\\\", "/", fixture$dir)
+  # Every spelling the directory can appear in: withr::local_tempdir() can
+  # return "/tmp//Rtmpx", file.path() keeps that, and install_executable()
+  # passes its own paths through repair_path(), which collapses it.
+  dirs <- unique(c(
+    fixture$dir,
+    repair_path(fixture$dir),
+    gsub("\\\\", "/", fixture$dir)
+  ))
   function(lines) {
     lines <- gsub("\\\\", "/", lines)
-    lines <- gsub(dir, "<dir>", lines, fixed = TRUE)
+    for (dir in dirs) {
+      lines <- gsub(dir, "<dir>", lines, fixed = TRUE)
+    }
     gsub("exe-(new|old)-[0-9a-f]+", "exe-\\1-<random>", lines)
   }
 }