diff --git a/NEWS.md b/NEWS.md index c8e269f14..14298b2b1 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,11 @@ # renv 1.1.0 (UNRELEASED) +* Fixed an issue where R package installation could fail if the project + depended on a package whose current version available from the configured + package repositories required on a newer version of R than what was currently + installed, even if that package need not be updated. (#2071) + * Fixed an issue where `RENV_CONFIG_EXTERNAL_LIBRARIES` was incorrectly split when using Windows paths. (#2069) diff --git a/R/project.R b/R/project.R index 8f4dd1ac4..dba05aec4 100644 --- a/R/project.R +++ b/R/project.R @@ -161,15 +161,19 @@ renv_project_remotes <- function(project, filter = NULL, resolve = FALSE) { # check for explicit version requirement explicit <- spec[spec$Require == "==", ] - if (nrow(explicit) == 0) - return(renv_remotes_resolve(package)) - - version <- spec$Version[[1]] - if (!nzchar(version)) - return(renv_remotes_resolve(package)) + if (nrow(explicit)) { + version <- explicit$Version[[1L]] + if (nzchar(version)) { + entry <- paste(package, version, sep = "@") + return(renv_remotes_resolve(entry)) + } + } - entry <- paste(package, version, sep = "@") - renv_remotes_resolve(entry) + # check if we're being invoked during restore or install + # if so, we may want to re-use an already-existing package + # https://github.com/rstudio/renv/issues/2071 + packages <- renv_restore_state(key = "packages") + renv_remotes_resolve(package, infer = !package %in% packages) } diff --git a/R/remotes.R b/R/remotes.R index d0d7dcb71..223cbeba1 100644 --- a/R/remotes.R +++ b/R/remotes.R @@ -15,7 +15,7 @@ remote <- function(spec) { # take a short-form remotes spec, parse that into a remote, # and generate a corresponding package record -renv_remotes_resolve <- function(spec, latest = FALSE) { +renv_remotes_resolve <- function(spec, latest = FALSE, infer = FALSE) { # check for already-resolved specs if (is.null(spec) || is.list(spec)) @@ -30,6 +30,15 @@ renv_remotes_resolve <- function(spec, latest = FALSE) { # https://github.com/rstudio/renv/issues/1135 spec <- gsub("/+$", "", spec, perl = TRUE) + # check if we should infer the package version + infer <- + infer && + grepl(renv_regexps_package_name(), spec) && + renv_package_installed(spec) + + if (infer) + spec <- paste(spec, renv_package_version(spec), sep = "@") + # check for archive URLs -- this is a bit hacky if (grepl("^(?:file|https?)://", spec)) { for (suffix in c(".zip", ".tar.gz", ".tgz", "/tarball")) diff --git a/R/retrieve.R b/R/retrieve.R index e315316ed..83a0c0f9d 100644 --- a/R/retrieve.R +++ b/R/retrieve.R @@ -915,9 +915,15 @@ renv_retrieve_repos_archive <- function(record) { if (is.null(root)) next - # attempt download + # attempt download; report errors via condition handler name <- renv_retrieve_repos_archive_name(record, type = "source") status <- catch(renv_retrieve_repos_impl(record, "source", name, root)) + if (inherits(status, "error")) { + attr(status, "record") <- record + renv_condition_signal("renv.retrieve.error", entry) + } + + # exit now if we had success if (identical(status, TRUE)) return(TRUE) diff --git a/tests/testthat/helper-aaa.R b/tests/testthat/helper-aaa.R index 576a76154..bf4034769 100644 --- a/tests/testthat/helper-aaa.R +++ b/tests/testthat/helper-aaa.R @@ -7,6 +7,7 @@ teardown_env <- function() { } the$tests_repopath <- renv_scope_tempfile("renv-repos-", scope = teardown_env()) + renv_tests_repopath <- function() { the$tests_repopath } diff --git a/tests/testthat/helper-setup.R b/tests/testthat/helper-setup.R index 444ce3cd2..46a6f060c 100644 --- a/tests/testthat/helper-setup.R +++ b/tests/testthat/helper-setup.R @@ -50,7 +50,7 @@ renv_tests_setup_envvars <- function(scope = parent.frame()) { # set up sandbox directory sandbox <- file.path(root, "sandbox") ensure_directory(sandbox) - + renv_scope_envvars( RENV_AUTOLOAD_ENABLED = FALSE, RENV_CONFIG_LOCKING_ENABLED = FALSE, @@ -222,6 +222,7 @@ renv_tests_setup_repos <- function(scope = parent.frame()) { descpath <- file.path(path, "DESCRIPTION") desc <- renv_description_read(descpath) desc$Version <- "0.1.0" + desc$Depends <- gsub("99.99.99", "1.0.0", desc$Depends %||% "", fixed = TRUE) write.dcf(desc, file = descpath) # place these packages into the archive diff --git a/tests/testthat/packages/future/DESCRIPTION b/tests/testthat/packages/future/DESCRIPTION new file mode 100644 index 000000000..c84dec2f0 --- /dev/null +++ b/tests/testthat/packages/future/DESCRIPTION @@ -0,0 +1,10 @@ +Package: future +Type: Package +Version: 1.0.0 +Depends: today +Repository: CRAN +License: GPL +Description: renv test package +Title: renv test package +Author: Anonymous Person +Maintainer: Anonymous Person diff --git a/tests/testthat/packages/today/DESCRIPTION b/tests/testthat/packages/today/DESCRIPTION new file mode 100644 index 000000000..d980a5f14 --- /dev/null +++ b/tests/testthat/packages/today/DESCRIPTION @@ -0,0 +1,10 @@ +Package: today +Type: Package +Version: 1.0.0 +Depends: R (>= 99.99.99) +Repository: CRAN +License: GPL +Description: renv test package +Title: renv test package +Author: Anonymous Person +Maintainer: Anonymous Person diff --git a/tests/testthat/test-install.R b/tests/testthat/test-install.R index e88b33240..01f5690cf 100644 --- a/tests/testthat/test-install.R +++ b/tests/testthat/test-install.R @@ -371,7 +371,7 @@ test_that("packages embedded in the project use a project-local RemoteURL", { skip_if(is.null(usethis$create_package)) renv_scope_options(usethis.quiet = TRUE) unlink("example", recursive = TRUE) - + fields <- list( "Authors@R" = utils::person( "Kevin", "Ushey", @@ -380,7 +380,7 @@ test_that("packages embedded in the project use a project-local RemoteURL", { comment = c(ORCID = "0000-0003-2880-7407") ) ) - + usethis$create_package("example", fields = fields, rstudio = FALSE, open = FALSE) install("./example") @@ -767,3 +767,28 @@ test_that("packages installed from r-universe preserve their remote metadata", { record <- renv_snapshot_description(package = "rlang") expect_true(is.character(record[["RemoteSha"]])) }) + +# https://github.com/rstudio/renv/issues/2071 +test_that("irrelevant R version requirements don't prevent package installation", { + + renv_tests_scope() + init() + + # package in repository not compatible with this version of R + expect_error(install("today")) + + # but older version can be successfully installed + install("today@0.1.0") + expect_true(renv_package_installed("today")) + expect_equal(renv_package_version("today"), "0.1.0") + + # other packages can be installed even if this project depends on it + writeLines("Depends: future, today", con = "DESCRIPTION") + install("future") + expect_true(renv_package_installed("future")) + remove("future") + + # but installing that package should still fail + expect_error(install("today")) + +}) diff --git a/tests/testthat/test-repos.R b/tests/testthat/test-repos.R index d7eb79b1f..b7f324f7c 100644 --- a/tests/testthat/test-repos.R +++ b/tests/testthat/test-repos.R @@ -2,7 +2,7 @@ test_that("we can query our local repository during tests", { expected <- list.files("packages") - drop <- if (.Platform$OS.type == "unix") "windowsonly" else "unixonly" + drop <- c("today", if (.Platform$OS.type == "unix") "windowsonly" else "unixonly") expected <- setdiff(expected, drop) renv_tests_scope()