From 6aa484a9a3d0bb5917df2faedc174768c3527229 Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Wed, 15 Apr 2026 13:22:09 -0700 Subject: [PATCH 1/2] Better reporting around dependencies needed for task Motivated by build_readme() but also made some improvements in stuff shared with dev_sitrep() --- R/build-readme.R | 14 +++- R/install.R | 32 +++++++- R/sitrep.R | 138 ++++++++++++++++++++++---------- tests/testthat/_snaps/sitrep.md | 23 +++--- tests/testthat/test-sitrep.R | 72 ++++++++++------- 5 files changed, 194 insertions(+), 85 deletions(-) diff --git a/R/build-readme.R b/R/build-readme.R index 0e2351b17..5e0bc4e16 100644 --- a/R/build-readme.R +++ b/R/build-readme.R @@ -40,7 +40,8 @@ build_rmd_impl <- function( path = ".", output_options = list(), ..., - quiet = TRUE + quiet = TRUE, + call = parent.frame() ) { check_dots_used(action = getOption("devtools.ellipsis_action", warn)) @@ -58,7 +59,7 @@ build_rmd_impl <- function( cli::cli_abort("Can't find file{?s}: {.path {files[!ok]}}.") } - local_install(pkg, quiet = TRUE) + local_install(pkg, quiet = quiet, call = call) # Ensure rendering github_document() doesn't generate HTML file output_options$html_preview <- FALSE @@ -130,13 +131,18 @@ build_readme <- function(path = ".", quiet = TRUE, ...) { } } -build_qmd_readme <- function(readme_path, path = ".", quiet = TRUE) { +build_qmd_readme <- function( + readme_path, + path = ".", + quiet = TRUE, + call = parent.frame() +) { pkg <- as.package(path) check_installed("quarto") save_all() - local_install(pkg, quiet = TRUE) + local_install(pkg, quiet = quiet, call = call) # Quarto spawns its own R process for knitr, which won't inherit .libPaths(). diff --git a/R/install.R b/R/install.R index fe884ab75..35f45d65b 100644 --- a/R/install.R +++ b/R/install.R @@ -245,9 +245,39 @@ install_dev_deps <- function( ) } -local_install <- function(pkg = ".", quiet = TRUE, env = parent.frame()) { +abort_for_missing_deps <- function(dep_status, pkg, call = parent.frame()) { + missing <- dep_status[dep_status$status == "missing", ] + if (nrow(missing) == 0) { + return(invisible()) + } + + n <- nrow(missing) + cli::cli_abort( + c( + "{n} {.pkg {pkg$package}} {cli::qty(n)}{?dependency is/dependencies are} not installed.", + "i" = "Install {cli::qty(n)}{?it/them} with {.run pak::local_install_dev_deps()}.", + dep_labels(missing) + ), + call = call + ) +} + +local_install <- function( + pkg = ".", + quiet = TRUE, + env = parent.frame(), + call = parent.frame() +) { pkg <- as.package(pkg) + dep_status <- pkg_dep_status(pkg, dependencies = NA) + abort_for_missing_deps(dep_status, pkg, call = call) + report_deps_ahead_behind( + dep_status, + pkg_name = pkg$package, + update_code = "pak::local_install_dev_deps()" + ) + if (!quiet) { cli::cli_inform(c( i = "Installing {.pkg {pkg$package}} in temporary library" diff --git a/R/sitrep.R b/R/sitrep.R index 3918740dd..8368c5e50 100644 --- a/R/sitrep.R +++ b/R/sitrep.R @@ -109,10 +109,12 @@ dev_sitrep <- function(pkg = ".", debug = FALSE) { has_build_tools = has_build_tools, rtools_path = if (has_build_tools) pkgbuild::rtools_path(), devtools_version = utils::packageVersion("devtools"), - devtools_deps = compare_deps(pak::pkg_deps("devtools", dependencies = NA)), - pkg_deps = if (!is.null(pkg)) { - compare_deps(pak::local_dev_deps(pkg$path, dependencies = TRUE)) - }, + devtools_cran_version = pak::pkg_deps( + "devtools", + dependencies = FALSE + )$version, + devtools_deps = pkg_dep_status("devtools", dependencies = NA), + pkg_deps = if (!is.null(pkg)) pkg_dep_status(pkg, dependencies = TRUE), rstudio_version = if (is_rstudio_running()) rstudioapi::getVersion(), rstudio_msg = if (!is_positron()) check_for_rstudio_updates() ) @@ -127,6 +129,7 @@ new_dev_sitrep <- function( has_build_tools = TRUE, rtools_path = NULL, devtools_version = utils::packageVersion("devtools"), + devtools_cran_version = devtools_version, devtools_deps = NULL, pkg_deps = NULL, rstudio_version = NULL, @@ -142,6 +145,7 @@ new_dev_sitrep <- function( has_build_tools = has_build_tools, rtools_path = rtools_path, devtools_version = devtools_version, + devtools_cran_version = devtools_cran_version, devtools_deps = devtools_deps, pkg_deps = pkg_deps, rstudio_version = rstudio_version, @@ -191,26 +195,28 @@ print.dev_sitrep <- function(x, ...) { cli::cli_rule("devtools") kv_line("version", x$devtools_version) + devtools_version <- package_version(x$devtools_version) + devtools_cran_version <- package_version(x$devtools_cran_version) + if (devtools_version < devtools_cran_version) { + all_ok <- FALSE + cli::cli_bullets(c( + "!" = "{.field devtools} is out of date ({.val {devtools_version}} vs {.val {devtools_cran_version}}).", + " " = "Update it with {.run pak::pak(\"devtools\")}." + )) + } else if (devtools_version > devtools_cran_version) { + cli::cli_bullets(c( + "i" = "{.field devtools} is ahead of CRAN ({.val {devtools_version}} vs {.val {devtools_cran_version}})." + )) + } + devtools_not_ok <- any(x$devtools_deps$status != "ok") if (devtools_not_ok) { all_ok <- FALSE - - behind <- x$devtools_deps[x$devtools_deps$status == "behind", ] - if (nrow(behind) > 0) { - cli::cli_bullets(c( - "!" = "{.field devtools} or its dependencies are out of date.", - " " = "Update them with {.code pak::pak(\"devtools\").}" - )) - cli::cli_verbatim(paste(" ", dep_labels(behind))) - } - - ahead <- x$devtools_deps[x$devtools_deps$status == "ahead", ] - if (nrow(ahead) > 0) { - cli::cli_bullets(c( - "i" = "{.field devtools} or its dependencies are installed from a dev version, FYI:" - )) - cli::cli_verbatim(paste(" ", dep_labels(ahead))) - } + report_deps_ahead_behind( + x$devtools_deps, + pkg_name = "devtools", + update_code = 'pak::pak("devtools")' + ) } cli::cli_rule("dev package") @@ -220,23 +226,11 @@ print.dev_sitrep <- function(x, ...) { dev_pkg_not_ok <- any(x$pkg_deps$status != "ok") if (dev_pkg_not_ok) { all_ok <- FALSE - - behind <- x$pkg_deps[x$pkg_deps$status == "behind", ] - if (nrow(behind) > 0) { - cli::cli_bullets(c( - "!" = "{.field {x$pkg$package}} dependencies are out of date.", - " " = "Update them with {.code pak::local_install_dev_deps()}." - )) - cli::cli_verbatim(paste(" ", dep_labels(behind))) - } - - ahead <- x$pkg_deps[x$pkg_deps$status == "ahead", ] - if (nrow(ahead) > 0) { - cli::cli_bullets(c( - "i" = "{.field {x$pkg$package}} dependencies are installed from a dev version, FYI:" - )) - cli::cli_verbatim(paste(" ", dep_labels(ahead))) - } + report_deps_ahead_behind( + x$pkg_deps, + pkg_name = x$pkg$package, + update_code = "pak::local_install_dev_deps()" + ) } if (all_ok) { @@ -248,7 +242,29 @@ print.dev_sitrep <- function(x, ...) { # Helpers ----------------------------------------------------------------- -compare_deps <- function(deps) { +#' Get dependency status for a package, excluding the package itself +#' +#' @param pkg A package name or a package object, as returned by `as.package()`. +#' @param dependencies Which dependency types to include. Passed along to +#' `pak::pkg_deps()` or `pak::local_dev_deps()`. +#' @returns A data frame with one row per dependency and columns: +#' * package (name) +#' * latest (version) +#' * installed (version) +#' * status (one of: missing, behind, ok, ahead) +#' @noRd +pkg_dep_status <- function(pkg, dependencies = NA) { + if (is_string(pkg)) { + deps <- pak::pkg_deps(pkg, dependencies = dependencies) + } else if (inherits(pkg, "package")) { + deps <- pak::local_dev_deps(pkg$path, dependencies = dependencies) + deps <- deps[deps$package != pkg$package, ] + } else { + cli::cli_abort( + "{.arg pkg} must be a string package name or a package object." + ) + } + installed <- map_chr(deps$package, function(p) { tryCatch( as.character(utils::packageVersion(p)), @@ -257,7 +273,7 @@ compare_deps <- function(deps) { }) status <- map2_chr(installed, deps$version, function(inst, latest) { if (is.na(inst)) { - return("behind") + return("missing") } switch( as.character(utils::compareVersion(inst, latest)), @@ -274,6 +290,46 @@ compare_deps <- function(deps) { ) } +#' Emit cli messages about ahead/behind dependencies +#' +#' @param dep_status A data frame as returned by `compare_deps()`, with +#' columns `package`, `latest`, `installed`, `status`. +#' @param pkg_name Package name to mention in the message. +#' @param update_code Code suggestion for updating behind deps. +#' @return Called for its side effects. +#' @noRd +report_deps_ahead_behind <- function(dep_status, pkg_name, update_code) { + missing <- dep_status[dep_status$status == "missing", ] + if (nrow(missing) > 0) { + n <- nrow(missing) + cli::cli_bullets(c( + "!" = "{n} {.field {pkg_name}} {cli::qty(n)}{?dependency is/dependencies are} not installed.", + " " = "Install {cli::qty(n)}{?it/them} with {.run {update_code}}." + )) + cli::cli_verbatim(paste(" ", dep_labels(missing))) + } + + behind <- dep_status[dep_status$status == "behind", ] + if (nrow(behind) > 0) { + n <- nrow(behind) + cli::cli_bullets(c( + "!" = "{n} {.field {pkg_name}} {cli::qty(n)}{?dependency is/dependencies are} out of date.", + " " = "Update {cli::qty(n)}{?it/them} with {.run {update_code}}." + )) + cli::cli_verbatim(paste(" ", dep_labels(behind))) + } + + ahead <- dep_status[dep_status$status == "ahead", ] + if (nrow(ahead) > 0) { + n <- nrow(ahead) + cli::cli_bullets(c( + "i" = "{n} {.field {pkg_name}} {cli::qty(n)}{?dependency is/dependencies are} ahead of CRAN." + )) + cli::cli_verbatim(paste(" ", dep_labels(ahead))) + } +} + + dep_labels <- function(deps) { # helps with readability deps$package <- format(deps$package, justify = "left") @@ -283,7 +339,7 @@ dep_labels <- function(deps) { format_dep_line <- function(package, installed, latest, status) { if (is.na(installed)) { - paste0(package, " (not installed)") + paste0(package, " (", cli::col_red("missing"), ")") } else { status_styled <- if (status == "behind") { cli::col_red(status) diff --git a/tests/testthat/_snaps/sitrep.md b/tests/testthat/_snaps/sitrep.md index 352d3a870..8a123ebc3 100644 --- a/tests/testthat/_snaps/sitrep.md +++ b/tests/testthat/_snaps/sitrep.md @@ -38,9 +38,9 @@ * path: '/usr/lib/R' -- devtools ------------------------------------------------ * version: 2.4.6 - ! devtools or its dependencies are out of date. - Update them with `pak::pak("devtools").` - cli (behind: 0.5.0 vs 1.0.0) + ! 1 devtools dependency is out of date. + Update it with `pak::pak("devtools")`. + cli (behind: 0.5.0 vs 1.0.0) -- dev package --------------------------------------------- * package: * path: @@ -55,9 +55,9 @@ * path: '/usr/lib/R' -- devtools ------------------------------------------------ * version: 2.4.6 - ! devtools or its dependencies are out of date. - Update them with `pak::pak("devtools").` - somepkg (not installed) + ! 1 devtools dependency is not installed. + Install it with `pak::pak("devtools")`. + somepkg (missing) -- dev package --------------------------------------------- * package: * path: @@ -72,8 +72,7 @@ * path: '/usr/lib/R' -- devtools ------------------------------------------------ * version: 2.4.6 - i devtools or its dependencies are installed from a dev - version, FYI: + i 1 devtools dependency is ahead of CRAN. usethis (ahead: 3.2.1.9000 vs 3.2.1) -- dev package --------------------------------------------- * package: @@ -92,10 +91,10 @@ -- dev package --------------------------------------------- * package: "mypkg" * path: '/tmp/mypkg' - ! mypkg dependencies are out of date. + ! 2 mypkg dependencies are out of date. Update them with `pak::local_install_dev_deps()`. - dplyr (behind: 1.0.0 vs 1.1.0) - tidyr (behind: 1.0.0 vs 1.1.0) + dplyr (behind: 1.0.0 vs 1.1.0) + tidyr (behind: 1.0.0 vs 1.1.0) # print notes dev versions of package deps @@ -110,7 +109,7 @@ -- dev package --------------------------------------------- * package: "mypkg" * path: '/tmp/mypkg' - i mypkg dependencies are installed from a dev version, FYI: + i 1 mypkg dependency is ahead of CRAN. usethis (ahead: 3.2.1.9000 vs 3.2.1) # print shows RStudio update message diff --git a/tests/testthat/test-sitrep.R b/tests/testthat/test-sitrep.R index 33f94191b..7665d6146 100644 --- a/tests/testthat/test-sitrep.R +++ b/tests/testthat/test-sitrep.R @@ -45,7 +45,7 @@ test_that("print warns about missing devtools deps", { package = c("rlang", "somepkg"), latest = c("1.0.0", "1.0.0"), installed = c("1.0.0", NA_character_), - status = c("ok", "behind") + status = c("ok", "missing") ) ) expect_snapshot(print(x)) @@ -101,6 +101,50 @@ test_that("print notes dev versions of package deps", { expect_snapshot(print(x)) }) +test_that("pkg_dep_status detects ahead packages", { + local_mocked_bindings( + pkg_deps = function(...) { + data.frame( + package = c("rlang", "cli"), + version = c("0.0.1", "0.0.1") + ) + }, + .package = "pak" + ) + result <- pkg_dep_status("SOMEPACKAGE") + expect_equal(result$status, c("ahead", "ahead")) + expect_equal(result$latest, c("0.0.1", "0.0.1")) +}) + +test_that("pkg_dep_status detects behind packages", { + local_mocked_bindings( + pkg_deps = function(...) { + data.frame( + package = "rlang", + version = "99999.0.0" + ) + }, + .package = "pak" + ) + result <- pkg_dep_status("SOMEPACKAGE") + expect_equal(result$status, "behind") +}) + +test_that("pkg_dep_status reports missing packages", { + local_mocked_bindings( + pkg_deps = function(...) { + data.frame( + package = c("rlang", "thereIsNoSuchPackage"), + version = c("0.0.1", "1.0.0") + ) + }, + .package = "pak" + ) + result <- pkg_dep_status("SOMEPACKAGE") + expect_equal(result$status, c("ahead", "missing")) + expect_true(is.na(result$installed[[2]])) +}) + test_that("print shows RStudio update message", { local_reproducible_output(width = 60) withr::local_envvar(POSITRON = "") @@ -114,32 +158,6 @@ test_that("print shows RStudio update message", { expect_snapshot(print(x)) }) -test_that("compare_deps detects ahead packages", { - result <- compare_deps(data.frame( - package = c("rlang", "cli"), - version = c("0.0.1", "0.0.1") - )) - expect_equal(result$status, c("ahead", "ahead")) - expect_equal(result$latest, c("0.0.1", "0.0.1")) -}) - -test_that("compare_deps detects behind packages", { - result <- compare_deps(data.frame( - package = "rlang", - version = "99999.0.0" - )) - expect_equal(result$status, "behind") -}) - -test_that("compare_deps reports missing packages", { - result <- compare_deps(data.frame( - package = c("rlang", "thereIsNoSuchPackage"), - version = c("0.0.1", "1.0.0") - )) - expect_equal(result$status, c("ahead", "behind")) - expect_true(is.na(result$installed[[2]])) -}) - test_that("check_for_rstudio_updates", { skip_if_offline() skip_on_cran() From 73f27cd717eeccd9d08e7fe1e71b94176940c1dd Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Wed, 15 Apr 2026 14:20:44 -0700 Subject: [PATCH 2/2] Add NEWS bullets --- NEWS.md | 6 +++ tests/testthat/_snaps/build-readme.md | 10 +++++ tests/testthat/_snaps/sitrep.md | 32 +++++++++++++++ tests/testthat/test-build-readme.R | 24 ++++++++++++ tests/testthat/test-sitrep.R | 56 +++++++++++++++++++++++++++ 5 files changed, 128 insertions(+) diff --git a/NEWS.md b/NEWS.md index 5ef4d93d7..599fc028f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,11 @@ # devtools (development version) +* `build_readme()` no longer installs dependencies into the temporary library + (a regression introduced in 2.5.0). It now exits early if a required + dependency is missing and reports any that are out of date or at a dev + version (#2683). +* `dev_sitrep()` reports if devtools itself is out of date (#2687). + # devtools 2.5.0 Deprecations diff --git a/tests/testthat/_snaps/build-readme.md b/tests/testthat/_snaps/build-readme.md index e37ab3754..8f557e68d 100644 --- a/tests/testthat/_snaps/build-readme.md +++ b/tests/testthat/_snaps/build-readme.md @@ -22,6 +22,16 @@ Error in `build_readme()`: ! Found multiple executable READMEs: 'README.qmd' and 'README.Rmd'. There can only be one. +# build_readme() aborts when a dep is missing + + Code + build_readme(pkg) + Condition + Error in `build_readme()`: + ! 1 {PACKAGE} dependency is not installed. + i Install it with `pak::local_install_dev_deps()`. + missingpkg (missing) + # build_rmd() is deprecated Code diff --git a/tests/testthat/_snaps/sitrep.md b/tests/testthat/_snaps/sitrep.md index 8a123ebc3..3b48c4c0a 100644 --- a/tests/testthat/_snaps/sitrep.md +++ b/tests/testthat/_snaps/sitrep.md @@ -13,6 +13,38 @@ * path: v All checks passed +# print warns when devtools is out of date + + Code + print(x) + Message + -- R ------------------------------------------------------- + * version: 4.4.0 + * path: '/usr/lib/R' + -- devtools ------------------------------------------------ + * version: 2.4.6 + ! devtools is out of date (2.4.6 vs 2.5.0). + Update it with `pak::pak("devtools")`. + -- dev package --------------------------------------------- + * package: + * path: + +# print notes when devtools is ahead of CRAN + + Code + print(x) + Message + -- R ------------------------------------------------------- + * version: 4.4.0 + * path: '/usr/lib/R' + -- devtools ------------------------------------------------ + * version: 2.5.0.9000 + i devtools is ahead of CRAN (2.5.0.9000 vs 2.5.0). + -- dev package --------------------------------------------- + * package: + * path: + v All checks passed + # print warns when R is out of date Code diff --git a/tests/testthat/test-build-readme.R b/tests/testthat/test-build-readme.R index 34c1ad7ac..11a64d439 100644 --- a/tests/testthat/test-build-readme.R +++ b/tests/testthat/test-build-readme.R @@ -112,6 +112,30 @@ test_that("errors if both README.qmd and README.Rmd exist", { expect_snapshot(build_readme(pkg), error = TRUE) }) +test_that("build_readme() aborts when a dep is missing", { + pkg <- local_package_create() + pkg_name <- basename(pkg) + usethis::ui_silence( + usethis::with_project(pkg, use_readme_rmd(open = FALSE)) + ) + + local_mocked_bindings( + pkg_dep_status = function(...) { + data.frame( + package = "missingpkg", + latest = "1.0.0", + installed = NA_character_, + status = "missing" + ) + } + ) + expect_snapshot( + build_readme(pkg), + error = TRUE, + transform = function(x) gsub(pkg_name, "{PACKAGE}", x, fixed = TRUE) + ) +}) + test_that("don't error for README in another directory", { skip_on_cran() diff --git a/tests/testthat/test-sitrep.R b/tests/testthat/test-sitrep.R index 7665d6146..9e6abc6bf 100644 --- a/tests/testthat/test-sitrep.R +++ b/tests/testthat/test-sitrep.R @@ -8,6 +8,28 @@ test_that("print shows all checks passed", { expect_snapshot(print(x)) }) +test_that("print warns when devtools is out of date", { + local_reproducible_output(width = 60) + x <- new_dev_sitrep( + r_version = R_system_version("4.4.0"), + r_path = "/usr/lib/R", + devtools_version = package_version("2.4.6"), + devtools_cran_version = package_version("2.5.0") + ) + expect_snapshot(print(x)) +}) + +test_that("print notes when devtools is ahead of CRAN", { + local_reproducible_output(width = 60) + x <- new_dev_sitrep( + r_version = R_system_version("4.4.0"), + r_path = "/usr/lib/R", + devtools_version = package_version("2.5.0.9000"), + devtools_cran_version = package_version("2.5.0") + ) + expect_snapshot(print(x)) +}) + test_that("print warns when R is out of date", { local_reproducible_output(width = 60) x <- new_dev_sitrep( @@ -116,6 +138,20 @@ test_that("pkg_dep_status detects ahead packages", { expect_equal(result$latest, c("0.0.1", "0.0.1")) }) +test_that("pkg_dep_status detects ok packages", { + local_mocked_bindings( + pkg_deps = function(...) { + data.frame( + package = "rlang", + version = as.character(packageVersion("rlang")) + ) + }, + .package = "pak" + ) + result <- pkg_dep_status("SOMEPACKAGE") + expect_equal(result$status, "ok") +}) + test_that("pkg_dep_status detects behind packages", { local_mocked_bindings( pkg_deps = function(...) { @@ -145,6 +181,26 @@ test_that("pkg_dep_status reports missing packages", { expect_true(is.na(result$installed[[2]])) }) +test_that("pkg_dep_status works with a package object and filters self", { + pkg_path <- local_package_create() + pkg_obj <- as.package(pkg_path) + + local_mocked_bindings( + local_dev_deps = function(...) { + data.frame( + # the package itself typically appears as first row and we want to + # confirm it gets filtered out + package = c(pkg_obj$package, "rlang"), + version = c("0.0.1", "99999.0.0") + ) + }, + .package = "pak" + ) + + result <- pkg_dep_status(pkg_obj) + expect_false(pkg_obj$package %in% result$package) +}) + test_that("print shows RStudio update message", { local_reproducible_output(width = 60) withr::local_envvar(POSITRON = "")