From 66da51a13cc7738955cd3a5060d7d23fe4cffa58 Mon Sep 17 00:00:00 2001 From: infra-bot Date: Wed, 19 Aug 2026 10:22:23 +0200 Subject: [PATCH] fix(att_amend_desc): preserve pinned versions and accept several dirs in dir.r Keep a version constraint set by hand in DESCRIPTION when the same package is listed under two types (a pinned Imports and a bare Suggests, say). The version is no longer dropped, and an Imports/Suggests conflict resolves to Imports independently of the row order in DESCRIPTION (#140). Let dir.r take several directories, e.g. dir.r = c("R", "inst"), so dependencies used only outside the default scan can be declared. Passing a vector previously raised "the condition has length > 1" (#139). --- DESCRIPTION | 2 +- NEWS.md | 16 ++- R/att_to_description.R | 24 +++- man/att_amend_desc.Rd | 7 +- man/attachment-deprecated.Rd | 7 +- .../test-att_amend_desc_version_pins.R | 116 ++++++++++++++++++ vignettes/a-fill-pkg-description.Rmd | 10 ++ 7 files changed, 174 insertions(+), 8 deletions(-) create mode 100644 tests/testthat/test-att_amend_desc_version_pins.R diff --git a/DESCRIPTION b/DESCRIPTION index b21f892..a78f5cd 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: attachment Title: Deal with Dependencies -Version: 1.0.1 +Version: 1.1.0 Authors@R: c( person("Vincent", "Guyader", , "vincent@thinkr.fr", role = c("cre", "aut"), comment = c(ORCID = "0000-0003-0671-9270")), diff --git a/NEWS.md b/NEWS.md index e26cde5..cdd49e5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,7 +1,21 @@ -# attachment 1.0.1 +# attachment 1.1.0 + +## New features + +- `att_amend_desc()` now accepts several directories in `dir.r`, for example + `att_amend_desc(dir.r = c("R", "inst"))`. This lets you declare dependencies + used only outside the standard package directories, such as a deployment + entry point under `inst/`, which the default scan does not reach. Passing a + vector previously raised `the condition has length > 1` (#139). ## Bug fixes +- `att_amend_desc()` no longer drops a version constraint set by hand in + DESCRIPTION when the same package is listed under two types (a pinned + `Imports` and a bare `Suggests`, for instance). Previously the version that + survived depended on the order of the rows in DESCRIPTION; the hand-set + constraint is now kept, and when both an `Imports` and a `Suggests` row carry + a version the `Imports` one wins (#140). - `att_from_examples()` (and therefore `att_amend_desc()`) no longer chokes on inline R inside roxygen2 markdown tags such as `@param x \`r helper("x")\`` when `helper()` is a package-local function. diff --git a/R/att_to_description.R b/R/att_to_description.R index 960e9c2..704f6d2 100644 --- a/R/att_to_description.R +++ b/R/att_to_description.R @@ -7,7 +7,12 @@ #' #' @param path path to the root of the package directory. Default to current directory. #' @param path.n path to namespace file. -#' @param dir.r path to directory with R scripts. +#' @param dir.r Character vector of one or more directories holding R scripts to +#' parse for dependencies. Defaults to `"R"`. Pass several paths, for example +#' `c("R", "inst")`, to also scan sources that live outside the standard +#' package directories (a deployment entry point under `inst/`, say). Packages +#' called with `library()` or `pkg::fun()` in these scripts are added to +#' Imports, so a version already pinned for them in DESCRIPTION is preserved. #' @param dir.v path to vignettes directory. Set to empty (dir.v = "") to ignore. #' @param dir.t path to tests directory. Set to empty (dir.t = "") to ignore. #' @param extra.suggests vector of other packages that should be added in Suggests (pkgdown, covr for instance) @@ -190,7 +195,7 @@ att_amend_desc <- function(path = ".", if (path.n != "") { imports <- unique(c(imports, att_from_namespace(path.n, document = document))) } - if (dir.r != "") { + if (!identical(dir.r, "")) { # Look for R scripts imports <- unique(c(imports, att_from_rscripts(dir.r))) # Look for Rmd, in case in a bookdown @@ -201,7 +206,7 @@ att_amend_desc <- function(path = ".", suggests <- NULL # Get suggests in examples and remove if already in imports - if (dir.r != "") { + if (!identical(dir.r, "")) { ex <- att_from_examples(dir.r = dir.r) suggests <- c(suggests, ex[!ex %in% imports]) } @@ -364,10 +369,21 @@ att_to_desc_from_is <- function(path.d = "DESCRIPTION", imports = NULL, all_packages <- c(imports, suggests) if (is.null(all_packages)) {all_packages <- character()} + # Collapse the original versions to one row per package before the join. + # A package listed under two types (a pinned Imports and a bare Suggests, say) + # otherwise multiplies rows in the merge, and the later de-duplication would + # keep whichever row happened to come first. Order so that an explicit + # constraint beats "*", and Imports beats Suggests on a tie, so a version set + # by hand in DESCRIPTION is never silently dropped. + orig_versions <- deps_orig[ + order(deps_orig$version == "*", deps_orig$type), + c("package", "version")] + orig_versions <- orig_versions[!duplicated(orig_versions$package), ] + deps_new <- data.frame( type = c(rep("Imports", length(imports)), rep("Suggests", length(suggests))), package = all_packages, stringsAsFactors = FALSE) %>% - merge(deps_orig[,c("package", "version")], + merge(orig_versions, by = "package", sort = TRUE, all.x = TRUE, all.y = FALSE) %>% .[,c("type", "package", "version")] %>% .[order(.$type, .$package), , drop = FALSE] %>% diff --git a/man/att_amend_desc.Rd b/man/att_amend_desc.Rd index a70b320..9181669 100644 --- a/man/att_amend_desc.Rd +++ b/man/att_amend_desc.Rd @@ -50,7 +50,12 @@ att_to_desc_from_pkg( \item{path.d}{path to description file.} -\item{dir.r}{path to directory with R scripts.} +\item{dir.r}{Character vector of one or more directories holding R scripts to +parse for dependencies. Defaults to \code{"R"}. Pass several paths, for example +\code{c("R", "inst")}, to also scan sources that live outside the standard +package directories (a deployment entry point under \verb{inst/}, say). Packages +called with \code{library()} or \code{pkg::fun()} in these scripts are added to +Imports, so a version already pinned for them in DESCRIPTION is preserved.} \item{dir.v}{path to vignettes directory. Set to empty (dir.v = "") to ignore.} diff --git a/man/attachment-deprecated.Rd b/man/attachment-deprecated.Rd index 3b2afcb..91c0e55 100644 --- a/man/attachment-deprecated.Rd +++ b/man/attachment-deprecated.Rd @@ -26,7 +26,12 @@ att_to_description( \item{path.d}{path to description file.} -\item{dir.r}{path to directory with R scripts.} +\item{dir.r}{Character vector of one or more directories holding R scripts to +parse for dependencies. Defaults to \code{"R"}. Pass several paths, for example +\code{c("R", "inst")}, to also scan sources that live outside the standard +package directories (a deployment entry point under \verb{inst/}, say). Packages +called with \code{library()} or \code{pkg::fun()} in these scripts are added to +Imports, so a version already pinned for them in DESCRIPTION is preserved.} \item{dir.v}{path to vignettes directory. Set to empty (dir.v = "") to ignore.} diff --git a/tests/testthat/test-att_amend_desc_version_pins.R b/tests/testthat/test-att_amend_desc_version_pins.R new file mode 100644 index 0000000..7e848c0 --- /dev/null +++ b/tests/testthat/test-att_amend_desc_version_pins.R @@ -0,0 +1,116 @@ +# Version constraints already set in DESCRIPTION must survive att_amend_desc(), +# and dir.r must accept several directories (issues #139 and #140). + +copy_dummy <- function() { + tmpdir <- tempfile("dummypins") + dir.create(tmpdir) + file.copy(system.file("dummypackage", package = "attachment"), tmpdir, recursive = TRUE) + file.path(tmpdir, "dummypackage") +} + +test_that("att_amend_desc keeps a hand-set version when the package also sits under another type (#140)", { + dummypackage <- copy_dummy() + on.exit(unlink(dirname(dummypackage), recursive = TRUE), add = TRUE) + + # `glue` is detected as a Suggests dependency in dummypackage. Pin it in Imports by hand so + # it appears under two types in DESCRIPTION before the amend. + d <- desc::desc(file = file.path(dummypackage, "DESCRIPTION")) + d$set_dep("glue", type = "Imports", version = ">= 1.2.0") + d$write() + + att_amend_desc( + path = dummypackage, + document = FALSE, + check_if_suggests_is_installed = FALSE, + use.config = FALSE + ) + + deps <- desc::desc_get_deps(file.path(dummypackage, "DESCRIPTION")) + glue_row <- deps[deps$package == "glue", ] + + expect_equal(nrow(glue_row), 1L) + # glue is used from a Suggests location, so the scan moves it there; the + # hand-set version must follow it rather than being reset to "*". + expect_equal(glue_row$type, "Suggests") + expect_equal(glue_row$version, ">= 1.2.0") +}) + +test_that("att_amend_desc keeps the Imports-side version when both types carry different pins (#140)", { + dummypackage <- copy_dummy() + on.exit(unlink(dirname(dummypackage), recursive = TRUE), add = TRUE) + + # A different explicit constraint under each type. Collapsing to one row forces + # a choice: the Imports-side constraint wins, whatever the row order. + d <- desc::desc(file = file.path(dummypackage, "DESCRIPTION")) + d$set_dep("glue", type = "Imports", version = ">= 1.2.0") + d$set_dep("glue", type = "Suggests", version = ">= 9.9.9") + d$write() + + att_amend_desc( + path = dummypackage, + document = FALSE, + check_if_suggests_is_installed = FALSE, + use.config = FALSE + ) + + deps <- desc::desc_get_deps(file.path(dummypackage, "DESCRIPTION")) + glue_row <- deps[deps$package == "glue", ] + + expect_equal(nrow(glue_row), 1L) + expect_equal(glue_row$version, ">= 1.2.0") +}) + +test_that("att_amend_desc resolves an Imports/Suggests conflict to Imports, keeping the version (#140)", { + dummypackage <- copy_dummy() + on.exit(unlink(dirname(dummypackage), recursive = TRUE), add = TRUE) + + # `fakepkg` is really used in R/ code, so the scan classifies it as an Imports dependency. + r_file <- file.path(dummypackage, "R", "fun_pin.R") + writeLines( + text = "#' @export\nuse_fake <- function() {\n fakepkg::run()\n}", + con = r_file + ) + # DESCRIPTION carries it under both types: pinned Imports and bare Suggests. + d <- desc::desc(file = file.path(dummypackage, "DESCRIPTION")) + d$set_dep("fakepkg", type = "Imports", version = ">= 2.0.0") + d$set_dep("fakepkg", type = "Suggests", version = "*") + d$write() + + att_amend_desc( + path = dummypackage, + document = FALSE, + must.exist = FALSE, + check_if_suggests_is_installed = FALSE, + use.config = FALSE + ) + + deps <- desc::desc_get_deps(file.path(dummypackage, "DESCRIPTION")) + fake_row <- deps[deps$package == "fakepkg", ] + + expect_equal(nrow(fake_row), 1L) + expect_equal(fake_row$type, "Imports") + expect_equal(fake_row$version, ">= 2.0.0") +}) + +test_that("att_amend_desc accepts several directories in dir.r (#139)", { + dummypackage <- copy_dummy() + on.exit(unlink(dirname(dummypackage), recursive = TRUE), add = TRUE) + + # A runtime dependency used only outside the default scan (here inst/). + dir.create(file.path(dummypackage, "inst"), showWarnings = FALSE) + writeLines("clipr::write_clip('x')", file.path(dummypackage, "inst", "main.R")) + + expect_no_error( + att_amend_desc( + path = dummypackage, + dir.r = c("R", "inst"), + document = FALSE, + must.exist = FALSE, + check_if_suggests_is_installed = FALSE, + use.config = FALSE + ) + ) + + deps <- desc::desc_get_deps(file.path(dummypackage, "DESCRIPTION")) + expect_true("clipr" %in% deps$package) +}) diff --git a/vignettes/a-fill-pkg-description.Rmd b/vignettes/a-fill-pkg-description.Rmd index 3bf26a7..ee6d29c 100644 --- a/vignettes/a-fill-pkg-description.Rmd +++ b/vignettes/a-fill-pkg-description.Rmd @@ -121,6 +121,16 @@ Then I can directly run `att_amend_desc()`. In the _Imports_ case, if for any reason I decide to delete this `my_knit()` function, then the {bookdown} dependency won't be needed anymore, and {attachment} will automatically remove it with the next `att_amend_desc()` +### I have an R script in my "inst/" directory + +If the file under "inst/" is a plain R script rather than a notebook, for instance a deployment entry point "inst/main.R" that calls `pkg::fun()`, you can have {attachment} scan it by adding its directory to `dir.r`: + +```{r, eval=FALSE} +att_amend_desc(dir.r = c("R", "inst")) +``` + +Packages detected this way are added to "Imports". A version already pinned by hand in DESCRIPTION for such a package is preserved. + ## Example on a fake package