From 31ef58e111617c1378ca015e8ca8005803b4de62 Mon Sep 17 00:00:00 2001 From: jgabry Date: Mon, 11 May 2026 11:47:43 +0200 Subject: [PATCH 1/3] Use proper numeric ordering when selecting the latest installation and checking version-specific behavior --- R/install.R | 9 +++--- R/model.R | 4 +-- R/path.R | 45 +++++++++++++++++++++++++----- R/utils.R | 7 ++++- R/zzz.R | 2 +- tests/testthat/test-install.R | 14 ++++++++++ tests/testthat/test-model-sample.R | 2 +- tests/testthat/test-path.R | 19 +++++++++++++ 8 files changed, 86 insertions(+), 16 deletions(-) diff --git a/R/install.R b/R/install.R index ee91fd1cb..947640017 100644 --- a/R/install.R +++ b/R/install.R @@ -804,14 +804,15 @@ rtools4x_toolchain_path <- function() { rtools4x_version <- function() { rtools_ver <- NULL + r_version <- current_r_version() - if (R.version$minor < "2.0") { + if (r_version < "4.2.0") { rtools_ver <- "40" - } else if (R.version$minor < "3.0") { + } else if (r_version < "4.3.0") { rtools_ver <- "42" - } else if (R.version$minor < "4.0") { + } else if (r_version < "4.4.0") { rtools_ver <- "43" - } else if (R.version$minor < "5.0") { + } else if (r_version < "4.5.0") { rtools_ver <- "44" } else { rtools_ver <- "45" diff --git a/R/model.R b/R/model.R index 0359678af..25b49ba41 100644 --- a/R/model.R +++ b/R/model.R @@ -1138,7 +1138,8 @@ sample <- function(data = NULL, save_metric = getOption("cmdstanr_save_metric", FALSE), save_cmdstan_config = getOption("cmdstanr_save_config", FALSE)) { - if (self$cmdstan_version() < "2.36.0" && !fixed_param) { + if (cmdstan_version_compare(self$cmdstan_version(), "2.36.0") < 0 && + !fixed_param) { if (self$has_stan_file() && file.exists(self$stan_file())) { if (!is.null(self$variables()) && length(self$variables()$parameters) == 0) { stop("Model contains no parameters. Please use 'fixed_param = TRUE'.", call. = FALSE) @@ -2570,4 +2571,3 @@ map_cmdstan_to_cmdstanr <- function(method) { character(0) ) } - diff --git a/R/path.R b/R/path.R index a62c233ef..94ab8b595 100644 --- a/R/path.R +++ b/R/path.R @@ -134,12 +134,34 @@ cmdstan_min_version <- function() { "2.35.0" } -is_supported_cmdstan_version <- function(version) { - if (is.null(version)) { - return(FALSE) +cmdstan_version_base <- function(version) { + # During path discovery we need to compare install dir names before + # cmdstan_version() can read the makefile to get the version number, + # so we need to be able to parse the version number from the dir name + version <- as.character(version) + version <- sub("[/\\\\]+$", "", version) + version <- basename(version) + version <- sub("^cmdstan-", "", version) + sub("-rc[0-9]+$", "", version) +} + +cmdstan_version_compare <- function(version, other) { + # Empty strings are used when no native or WSL install was found + if (length(version) != 1 || is.na(version) || !nzchar(version)) { + return(-1L) } + if (length(other) != 1 || is.na(other) || !nzchar(other)) { + return(1L) + } + utils::compareVersion( + cmdstan_version_base(version), + cmdstan_version_base(other) + ) +} + +is_supported_cmdstan_version <- function(version) { cmp <- tryCatch( - suppressWarnings(utils::compareVersion(as.character(version), cmdstan_min_version())), + suppressWarnings(cmdstan_version_compare(version, cmdstan_min_version())), error = function(e) NA_integer_ ) isTRUE(cmp >= 0) @@ -241,7 +263,7 @@ cmdstan_default_path <- function(dir = NULL) { if (!nzchar(latest_cmdstan) && !nzchar(latest_wsl_cmdstan)) { return(NULL) } - if (latest_wsl_cmdstan >= latest_cmdstan) { + if (cmdstan_version_compare(latest_wsl_cmdstan, latest_cmdstan) >= 0) { return(file.path(wsl_installs_path, latest_wsl_cmdstan)) } else { return(file.path(installs_path, latest_cmdstan)) @@ -254,11 +276,20 @@ latest_cmdstan_installed <- function(installs_path) { cmdstan_installs <- list.dirs(path = installs_path, recursive = FALSE, full.names = FALSE) latest_cmdstan <- "" if (length(cmdstan_installs) > 0) { - cmdstan_installs <- grep("^cmdstan-", cmdstan_installs, value = TRUE) + cmdstan_installs <- grep( + "^cmdstan-[0-9]+\\.[0-9]+\\.[0-9]+(-rc[0-9]+)?$", + cmdstan_installs, + value = TRUE + ) if (length(cmdstan_installs) == 0) { return(latest_cmdstan) } - latest_cmdstan <- sort(cmdstan_installs, decreasing = TRUE)[1] + # list.dirs() returns dir names so sort by parsed version first instead of dir name + latest_cmdstan <- cmdstan_installs[order( + numeric_version(cmdstan_version_base(cmdstan_installs)), + cmdstan_installs, + decreasing = TRUE + )[1]] if (is_release_candidate(latest_cmdstan)) { non_rc_path <- strsplit(latest_cmdstan, "-rc")[[1]][1] if (dir.exists(file.path(installs_path, non_rc_path))) { diff --git a/R/utils.R b/R/utils.R index 40fa62c3e..4b925549a 100644 --- a/R/utils.R +++ b/R/utils.R @@ -58,8 +58,13 @@ os_is_linux <- function() { isTRUE(Sys.info()[["sysname"]] == "Linux") } +current_r_version <- function() { + getRversion() +} + is_ucrt_toolchain <- function() { - os_is_windows() && R.version$major == "4" && R.version$minor >= "2.0" + r_version <- current_r_version() + os_is_windows() && r_version >= "4.2.0" && r_version < "5.0.0" } # Check if running R in Rosetta 2 translation environment, which is an diff --git a/R/zzz.R b/R/zzz.R index bea367db0..0ea383133 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -25,7 +25,7 @@ startup_messages <- function() { current_version <- try(cmdstan_version(), silent = TRUE) if (!inherits(latest_version, "try-error") && !inherits(current_version, "try-error") - && latest_version > current_version) { + && cmdstan_version_compare(latest_version, current_version) > 0) { packageStartupMessage( "\nA newer version of CmdStan is available. See ?install_cmdstan() to install it.", "\nTo disable this check set option or environment variable cmdstanr_no_ver_check=TRUE." diff --git a/tests/testthat/test-install.R b/tests/testthat/test-install.R index 243b803fd..cfa21031e 100644 --- a/tests/testthat/test-install.R +++ b/tests/testthat/test-install.R @@ -304,6 +304,20 @@ test_that("deprecated CMDSTANR_USE_MSYS_TOOLCHAIN is ignored with warning", { }) }) +test_that("Rtools helpers compare R versions numerically", { + local({ + local_mocked_bindings(current_r_version = function() numeric_version("4.10.0")) + expect_equal(rtools4x_version(), "45") + }) + local({ + local_mocked_bindings( + os_is_windows = function() TRUE, + current_r_version = function() numeric_version("4.10.0") + ) + expect_equal(is_ucrt_toolchain(), TRUE) + }) +}) + test_that("rtools4x_toolchain_path prefers static-posix when available", { skip_if(arch_is_aarch64()) env_var <- paste0( diff --git a/tests/testthat/test-model-sample.R b/tests/testthat/test-model-sample.R index 85c5dbb85..81186ed08 100644 --- a/tests/testthat/test-model-sample.R +++ b/tests/testthat/test-model-sample.R @@ -321,7 +321,7 @@ test_that("Correct behavior if fixed_param not set when the model has no paramet ) reset_cmdstan_version(m) - if (cmdstan_version() >= "2.36.0") { + if (cmdstan_version_compare(cmdstan_version(), "2.36.0") >= 0) { # as of 2.36.0 we don't need fixed_param if no parameters expect_no_error( utils::capture.output( diff --git a/tests/testthat/test-path.R b/tests/testthat/test-path.R index 5b862a25c..93f2d6c4f 100644 --- a/tests/testthat/test-path.R +++ b/tests/testthat/test-path.R @@ -212,6 +212,18 @@ test_that("cmdstan_default_path() respects custom install directories", { ) }) +test_that("cmdstan_default_path() orders install directories by CmdStan version", { + installs <- withr::local_tempdir(pattern = "cmdstan-version-installs") + dir.create(file.path(installs, "cmdstan-2.9.0"), recursive = TRUE, showWarnings = FALSE) + dir.create(file.path(installs, "cmdstan-2.35.0"), recursive = TRUE, showWarnings = FALSE) + + expect_equal(latest_cmdstan_installed(installs), "cmdstan-2.35.0") + expect_equal( + cmdstan_default_path(dir = installs), + file.path(installs, "cmdstan-2.35.0") + ) +}) + test_that("cmdstan_default_path() returns NULL for empty custom install directories", { installs <- withr::local_tempdir(pattern = "cmdstan-empty-installs") @@ -246,6 +258,13 @@ test_that("CmdStan version helpers handle invalid inputs", { expect_false(is_supported_cmdstan_version("not-a-version")) }) +test_that("CmdStan version helpers use numeric ordering", { + expect_equal(cmdstan_version_compare("cmdstan-2.35.0", "cmdstan-2.9.0"), 1) + expect_equal(cmdstan_version_compare("cmdstan-2.9.0", "cmdstan-2.35.0"), -1) + expect_equal(cmdstan_version_compare("2.36.0-rc1", "2.36.0"), 0) + expect_equal(cmdstan_version_compare("2.100.0", "2.36.0"), 1) +}) + test_that("CmdStan version can be recovered from WSL UNC install path", { wsl_path <- "//wsl$/Ubuntu-22.04/root/.cmdstan/cmdstan-2.38.0" From 171d2f4889901cec3b15a564f62b571a8b19f7d8 Mon Sep 17 00:00:00 2001 From: jgabry Date: Mon, 11 May 2026 11:57:44 +0200 Subject: [PATCH 2/3] Add a few more comments --- R/path.R | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/R/path.R b/R/path.R index 94ab8b595..b06ccfd1e 100644 --- a/R/path.R +++ b/R/path.R @@ -134,10 +134,10 @@ cmdstan_min_version <- function() { "2.35.0" } -cmdstan_version_base <- function(version) { - # During path discovery we need to compare install dir names before - # cmdstan_version() can read the makefile to get the version number, - # so we need to be able to parse the version number from the dir name +# Normalize versions for comparison. This is intentionally looser than +# cmdstan_version_from_path(): it accepts version strings, paths, and install +# dir names, and strips release-candidate suffixes. +cmdstan_version_for_comparison <- function(version) { version <- as.character(version) version <- sub("[/\\\\]+$", "", version) version <- basename(version) @@ -145,8 +145,9 @@ cmdstan_version_base <- function(version) { sub("-rc[0-9]+$", "", version) } +# Scalar comparison of versions numbers. Returns -1, 0, or 1. +# Empty strings are used when no native or WSL install was found during path discovery. cmdstan_version_compare <- function(version, other) { - # Empty strings are used when no native or WSL install was found if (length(version) != 1 || is.na(version) || !nzchar(version)) { return(-1L) } @@ -154,8 +155,8 @@ cmdstan_version_compare <- function(version, other) { return(1L) } utils::compareVersion( - cmdstan_version_base(version), - cmdstan_version_base(other) + cmdstan_version_for_comparison(version), + cmdstan_version_for_comparison(other) ) } @@ -272,6 +273,7 @@ cmdstan_default_path <- function(dir = NULL) { NULL } +# Return the newest CmdStan install directory name under an install root latest_cmdstan_installed <- function(installs_path) { cmdstan_installs <- list.dirs(path = installs_path, recursive = FALSE, full.names = FALSE) latest_cmdstan <- "" @@ -284,9 +286,10 @@ latest_cmdstan_installed <- function(installs_path) { if (length(cmdstan_installs) == 0) { return(latest_cmdstan) } - # list.dirs() returns dir names so sort by parsed version first instead of dir name + # list.dirs() returns dir names, so sort by parsed versions instead of + # lexicographic names latest_cmdstan <- cmdstan_installs[order( - numeric_version(cmdstan_version_base(cmdstan_installs)), + numeric_version(cmdstan_version_for_comparison(cmdstan_installs)), cmdstan_installs, decreasing = TRUE )[1]] @@ -300,6 +303,7 @@ latest_cmdstan_installed <- function(installs_path) { latest_cmdstan } +# Detect WSL UNC paths, which may need path-based version fallback below is_wsl_unc_path <- function(path) { is.character(path) && length(path) == 1 && @@ -307,6 +311,9 @@ is_wsl_unc_path <- function(path) { startsWith(repair_path(path), "//wsl$/") } +# Strict extractor for CmdStan install paths. Unlike +# cmdstan_version_for_comparison(), it returns NULL for non-CmdStan paths and +# preserves release-candidate suffixes. cmdstan_version_from_path <- function(path) { path <- repair_path(path) match <- regmatches( From ba2fe1fdca497615402e9de77ef3237ba81381bd Mon Sep 17 00:00:00 2001 From: jgabry Date: Wed, 3 Jun 2026 13:23:31 +0200 Subject: [PATCH 3/3] fix outdated comment --- R/run.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/run.R b/R/run.R index eefa6d7ce..8a15bf495 100644 --- a/R/run.R +++ b/R/run.R @@ -67,11 +67,11 @@ CmdStanRun <- R6::R6Class( self$args$new_files(type = "profile") }, new_config_files = function() { - # because CmdStan 2.34 uses the output_file name as the base for the config file + # the output_file name is used as the base for the config file paste0(tools::file_path_sans_ext(private$output_files_), "_config.json") }, new_metric_files = function() { - # because CmdStan 2.34 uses the output_file name as the base for the metric file + # the output_file name is used as the base for the metric file paste0(tools::file_path_sans_ext(private$output_files_), "_metric.json") }, config_files = function(include_failed = FALSE) {