From 8573b0d6fa79484d9337854ddd246467538809f5 Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Mon, 5 Aug 2024 14:52:00 -0700 Subject: [PATCH] ensure remote fields from r-universe are included in lockfile --- NEWS.md | 3 +++ R/package.R | 21 ++++++++++++++------- R/snapshot.R | 16 ++++++++++------ man/lockfiles.Rd | 2 +- man/snapshot.Rd | 2 +- tests/testthat/test-install.R | 10 ++++++++++ tests/testthat/test-snapshot.R | 19 +++++++++++++++++-- 7 files changed, 56 insertions(+), 17 deletions(-) diff --git a/NEWS.md b/NEWS.md index 89c09f17b..f07af88f5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,9 @@ # renv (development version) +* `renv` now preserves `Remote` fields present on packages installed from + public package repositories (e.g. ). (#1961) + * Fixed an issue where `renv::load()` could fail to load an alternate project when `options(renv.config.autoloader.enabled = FALSE)` was set. (#1959) diff --git a/R/package.R b/R/package.R index 61ae73d99..a41a89e6f 100644 --- a/R/package.R +++ b/R/package.R @@ -152,11 +152,7 @@ renv_package_pkgtypes <- function() { } -renv_package_augment_standard <- function(record) { - - # only done for repository remotes - if (!identical(record$Source, "Repository")) - return(record) +renv_package_augment_standard <- function(path, record) { # check whether we tagged a url + type for this package url <- attr(record, "url", exact = TRUE) @@ -165,6 +161,17 @@ renv_package_augment_standard <- function(record) { if (is.null(url) || is.null(type)) return(record) + # skip if this isn't a repository remote + if (!identical(record$Source, "Repository")) + return(record) + + # skip if the DESCRIPTION file already has Remote fields + # (mainly relevant for r-universe) + descpath <- file.path(path, "DESCRIPTION") + desc <- renv_description_read(descpath) + if (any(grepl("^Remote", names(desc)))) + return(record) + # figure out base of repository URL pattern <- "/(?:bin|src)/" index <- regexpr(pattern, url, perl = TRUE) @@ -190,8 +197,8 @@ renv_package_augment_standard <- function(record) { renv_package_augment <- function(installpath, record) { - # try to include repository fields - record <- renv_package_augment_standard(record) + # figure out if we should write this as a 'standard' remote + record <- renv_package_augment_standard(installpath, record) # check for remotes fields remotes <- record[grep("^Remote", names(record))] diff --git a/R/snapshot.R b/R/snapshot.R index 7b5ce5ddd..d517a4ce4 100644 --- a/R/snapshot.R +++ b/R/snapshot.R @@ -758,15 +758,19 @@ renv_snapshot_description_impl <- function(dcf, path = NULL) { # get remotes fields git <- grep("^git", names(dcf), value = TRUE) remotes <- grep("^Remote", names(dcf), value = TRUE) - cranlike <- renv_record_cranlike(dcf) + + # don't include 'RemoteRef' if it's a non-informative remote + if (identical(dcf[["RemoteRef"]], "HEAD")) + remotes <- setdiff(remotes, "RemoteRef") + + # drop remote metadata for 'standard' remotes, to avoid spurious + # diffs that could arise from installing a package using 'pak' + # versus 'install.packages()' or an alternate tool + std <- identical(dcf[["RemoteType"]], "standard") # only keep relevant fields extra <- c("Repository", "OS_type") - all <- c( - required, extra, - if (!cranlike) c(remotes, git), - "Requirements", "Hash" - ) + all <- c(required, extra, if (!std) c(remotes, git), "Requirements", "Hash") keep <- renv_vector_intersect(all, names(dcf)) # return as list diff --git a/man/lockfiles.Rd b/man/lockfiles.Rd index 2debf4c2c..83c4cfed4 100644 --- a/man/lockfiles.Rd +++ b/man/lockfiles.Rd @@ -34,7 +34,7 @@ lockfile_modify( \arguments{ \item{type}{The type of snapshot to perform: \itemize{ -\item \code{"implict"}, (the default), uses all packages captured by \code{\link[=dependencies]{dependencies()}}. +\item \code{"implicit"}, (the default), uses all packages captured by \code{\link[=dependencies]{dependencies()}}. \item \code{"explicit"} uses packages recorded in \code{DESCRIPTION}. \item \code{"all"} uses all packages in the project library. \item \code{"custom"} uses a custom filter. diff --git a/man/snapshot.Rd b/man/snapshot.Rd index f9aa6c21c..b21db42b0 100644 --- a/man/snapshot.Rd +++ b/man/snapshot.Rd @@ -38,7 +38,7 @@ directly instead.} \item{type}{The type of snapshot to perform: \itemize{ -\item \code{"implict"}, (the default), uses all packages captured by \code{\link[=dependencies]{dependencies()}}. +\item \code{"implicit"}, (the default), uses all packages captured by \code{\link[=dependencies]{dependencies()}}. \item \code{"explicit"} uses packages recorded in \code{DESCRIPTION}. \item \code{"all"} uses all packages in the project library. \item \code{"custom"} uses a custom filter. diff --git a/tests/testthat/test-install.R b/tests/testthat/test-install.R index 3b384d396..e0e62ca7e 100644 --- a/tests/testthat/test-install.R +++ b/tests/testthat/test-install.R @@ -748,3 +748,13 @@ test_that("installation of package from local sources works", { expect_true(renv_package_installed("bread")) }) + +test_that("packages installed from r-universe preserve their remote metadata", { + skip_on_cran() + skip_on_ci() + + project <- renv_tests_scope(packages = "rlang") + install("rlang", repos = "https://r-lib.r-universe.dev") + record <- renv_snapshot_description(package = "rlang") + expect_true(is.character(record[["RemoteSha"]])) +}) diff --git a/tests/testthat/test-snapshot.R b/tests/testthat/test-snapshot.R index fda0511b4..5c09188b4 100644 --- a/tests/testthat/test-snapshot.R +++ b/tests/testthat/test-snapshot.R @@ -554,7 +554,22 @@ test_that("snapshot() reports missing packages even if renv.verbose is FALSE", { expect_snapshot(. <- snapshot(force = TRUE)) }) -test_that("snapshot() doesn't duplicate packages", { - +test_that("packages installed from r-universe preserve remote metadata", { + + text <- heredoc(" + Package: skeleton + Type: Package + Version: 1.1.0 + Remotes: kevinushey/skeleton + Repository: https://kevinushey.r-universe.dev + RemoteUrl: https://github.com/kevinushey/skeleton + RemoteSha: e4aafb92b86ba7eba3b7036d9d96fdfb6c32761a + ") + + path <- renv_scope_tempfile() + writeLines(text, con = path) + + record <- renv_snapshot_description(path = path) + expect_identical(record[["RemoteSha"]], "e4aafb92b86ba7eba3b7036d9d96fdfb6c32761a") })