From f0bc08f452585446c2a0070986813d3a95d3359a Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Mon, 5 Jun 2023 17:08:16 -0500
Subject: [PATCH 01/14] Simplify handling of `Suggested` dependencies

* So a top-level `DESCRIPTION` is treated the same way regardless of whether it's a "Package" description or not.

* Document how "development dependencies" are found in `dependencies()`

* Impute a roxygen2/devtools dependency if the package has a `RoxygenNote` field
---
 NEWS.md                            |  4 ++++
 R/dependencies.R                   | 32 ++++++++++++++++++++----------
 R/project.R                        | 13 ------------
 man/dependencies.Rd                | 12 +++++++----
 tests/testthat/test-dependencies.R | 24 ++++++++++++----------
 5 files changed, 47 insertions(+), 38 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index cbc6738dd..40bc0da84 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,6 +1,10 @@
 
 # renv 0.18.0  (UNRELEASED)
 
+* `dependencies()` now marks `Suggested` packages listed in `DESCRIPTION` files
+  as development dependencies regardless of whether or not they're in 
+  "package" project.
+
 * If `renv::snapshot()` finds missing packages, a new prompt allows you to 
   install them before continuing (#1198).
 
diff --git a/R/dependencies.R b/R/dependencies.R
index 0f789e3dd..c1e8b2d37 100644
--- a/R/dependencies.R
+++ b/R/dependencies.R
@@ -145,10 +145,14 @@
 #'   * `"fatal"`: errors are fatal and stop execution.
 #'   *  `"ignored"`: errors are ignored and not reported to the user.
 #'
-#' @param dev Boolean; include 'development' dependencies as well? That is,
-#'   packages which may be required during development but are unlikely to be
-#'   required during runtime for your project. By default, only runtime
-#'   dependencies are returned.
+#' @param dev Boolean; include development dependencies? These packages are
+#'   typically required when developing the project, but not when running it
+#'   (i.e. you want them installed when humans are working on the project but
+#'   not when computers are deploying it).
+#'
+#'   Development dependencies include packages listed in the `Suggests` field
+#'   of a `DESCRIPTION` found in the project root, and roxygen2 or devtools if
+#'   their use is implied by other project metadata.
 #'
 #' @return An \R `data.frame` of discovered dependencies, mapping inferred
 #'   package names to the files in which they were discovered. Note that the
@@ -536,10 +540,10 @@ renv_dependencies_discover_description <- function(path,
       sprintf("Config/renv/profiles/%s/dependencies", profile)
 
     fields <- c(fields, "Suggests", profile_field)
-    type <- renv_description_type(desc = dcf)
+    proj_desc <- TRUE
 
   } else {
-    type <- "unknown"
+    proj_desc <- FALSE
   }
 
   data <- map(
@@ -547,14 +551,23 @@ renv_dependencies_discover_description <- function(path,
     renv_dependencies_discover_description_impl,
     dcf  = dcf,
     path = path,
-    type = type
+    proj_desc = proj_desc
   )
 
+  # Infer a dependency on roxygen2 and devtools if RoxygenNote present
+  if (!is.null(dcf$RoxygenNote)) {
+    data <- c(
+      data,
+      list(renv_dependencies_list(path, c("roxygen2", "devtools"), dev = TRUE))
+    )
+  }
+
+
   bind(data)
 
 }
 
-renv_dependencies_discover_description_impl <- function(dcf, field, path, type) {
+renv_dependencies_discover_description_impl <- function(dcf, field, path, proj_desc) {
 
   # read field
   contents <- dcf[[field]]
@@ -579,13 +592,12 @@ renv_dependencies_discover_description_impl <- function(dcf, field, path, type)
     return(list())
 
   # create dependency list
-  dev <- field == "Suggests" && type != "package"
   renv_dependencies_list(
     path,
     extract_chr(matches, 2L),
     extract_chr(matches, 3L),
     extract_chr(matches, 4L),
-    dev
+    dev = proj_desc && field == "Suggests"
   )
 
 }
diff --git a/R/project.R b/R/project.R
index 0df1a15eb..6edf82f3f 100644
--- a/R/project.R
+++ b/R/project.R
@@ -126,19 +126,6 @@ renv_project_remotes <- function(project, fields = NULL) {
   ignored <- renv_project_ignored_packages(project = project)
   specs <- specs[setdiff(names(specs), c("R", ignored))]
 
-  # if any Roxygen fields are included,
-  # infer a dependency on roxygen2 and devtools
-  desc <- renv_description_read(descpath)
-  if (any(grepl("^Roxygen", names(desc)))) {
-    for (package in c("devtools", "roxygen2")) {
-      if (!package %in% ignored) {
-        specs[[package]] <-
-          specs[[package]] %||%
-          renv_dependencies_list(descpath, package, dev = TRUE)
-      }
-    }
-  }
-
   # now, try to resolve the packages
   records <- enumerate(specs, function(package, spec) {
 
diff --git a/man/dependencies.Rd b/man/dependencies.Rd
index ff438cc38..a5d75c9a1 100644
--- a/man/dependencies.Rd
+++ b/man/dependencies.Rd
@@ -38,10 +38,14 @@ otherwise ignored.
 \item \code{"ignored"}: errors are ignored and not reported to the user.
 }}
 
-\item{dev}{Boolean; include 'development' dependencies as well? That is,
-packages which may be required during development but are unlikely to be
-required during runtime for your project. By default, only runtime
-dependencies are returned.}
+\item{dev}{Boolean; include development dependencies? These packages are
+typically required when developing the project, but not when running it
+(i.e. you want them installed when humans are working on the project but
+not when computers are deploying it).
+
+Development dependencies include packages listed in the \code{Suggests} field
+of a \code{DESCRIPTION} found in the project root, and roxygen2 or devtools if
+their use is implied by other project metadata.}
 }
 \value{
 An \R \code{data.frame} of discovered dependencies, mapping inferred
diff --git a/tests/testthat/test-dependencies.R b/tests/testthat/test-dependencies.R
index 4b09da1e5..dea6620be 100644
--- a/tests/testthat/test-dependencies.R
+++ b/tests/testthat/test-dependencies.R
@@ -150,22 +150,24 @@ test_that("no warnings are produced when crawling dependencies", {
 
 })
 
-test_that("Suggests are dev. deps for non-package projects", {
+test_that("Suggests are dev. deps for all projects", {
+
   renv_tests_scope()
+
+  expected <- data.frame(
+    Package = "bread",
+    Dev = TRUE,
+    stringsAsFactors = FALSE
+  )
+
   writeLines(c("Type: Project", "Suggests: bread"), con = "DESCRIPTION")
   deps <- dependencies(dev = TRUE)
-  expect_true(nrow(deps) == 1)
-  expect_true(deps$Package == "bread")
-  expect_true(deps$Dev)
-})
+  expect_equal(deps[c("Package", "Dev")], expected)
 
-test_that("Suggests are _not_ dev. deps for package projects", {
-  renv_tests_scope()
   writeLines(c("Type: Package", "Suggests: bread"), con = "DESCRIPTION")
-  deps <- dependencies(dev = FALSE)
-  expect_true(nrow(deps) == 1)
-  expect_true(deps$Package == "bread")
-  expect_false(deps$Dev)
+  deps <- dependencies(dev = TRUE)
+  expect_equal(deps[c("Package", "Dev")], expected)
+
 })
 
 test_that("packages referenced by modules::import() are discovered", {

From 14d6eea11fdeb0ed1679a806dd31b2c6e0ef112b Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Mon, 5 Jun 2023 18:00:16 -0500
Subject: [PATCH 02/14] Don't use renv_dependencies_discover_description() for
 package deps

---
 R/package.R | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/R/package.R b/R/package.R
index c65b3ede4..5e0bdab3c 100644
--- a/R/package.R
+++ b/R/package.R
@@ -276,8 +276,15 @@ renv_package_dependencies_impl <- function(package,
   assign(package, location, envir = visited, inherits = FALSE)
 
   # find its dependencies from the DESCRIPTION file
-  deps <- renv_dependencies_discover_description(location, fields)
-  subpackages <- deps$Package
+  desc <- tryCatch(
+    renv_description_read(location),
+    error = function(err) renv_dependencies_error(path, error = path)
+  )
+
+  fields <- c("Depends", "Imports", "LinkingTo")
+  deps <- bind(map(desc[fields], renv_description_parse_field))
+  subpackages <- setdiff(unique(unlist(deps$Package)), "R")
+
   for (subpackage in subpackages)
     renv_package_dependencies_impl(subpackage, visited, libpaths, fields)
 }

From 28a926126da8bc56fbd0e0c018e00e893e19509a Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Tue, 6 Jun 2023 07:11:40 -0500
Subject: [PATCH 03/14] Make dev dependencies a bit tighter

---
 R/dependencies.R                   |  9 ++-------
 tests/testthat/test-dependencies.R | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/R/dependencies.R b/R/dependencies.R
index c1e8b2d37..380584708 100644
--- a/R/dependencies.R
+++ b/R/dependencies.R
@@ -554,15 +554,11 @@ renv_dependencies_discover_description <- function(path,
     proj_desc = proj_desc
   )
 
-  # Infer a dependency on roxygen2 and devtools if RoxygenNote present
+  # Infer a dependency on roxygen2 if RoxygenNote present
   if (!is.null(dcf$RoxygenNote)) {
-    data <- c(
-      data,
-      list(renv_dependencies_list(path, c("roxygen2", "devtools"), dev = TRUE))
-    )
+    data <- c(data, list(renv_dependencies_list(path, "roxygen2", dev = TRUE)))
   }
 
-
   bind(data)
 
 }
@@ -934,7 +930,6 @@ renv_dependencies_discover_rproj <- function(path) {
   deps <- stack()
   if (identical(props$PackageUseDevtools, "Yes")) {
     deps$push("devtools")
-    deps$push("roxygen2")
   }
 
   renv_dependencies_list(path, deps$data(), dev = TRUE)
diff --git a/tests/testthat/test-dependencies.R b/tests/testthat/test-dependencies.R
index dea6620be..2d16c1d69 100644
--- a/tests/testthat/test-dependencies.R
+++ b/tests/testthat/test-dependencies.R
@@ -4,7 +4,7 @@ test_that(".Rproj files requesting devtools is handled", {
   writeLines("PackageUseDevtools: Yes", "project.Rproj")
   deps <- dependencies(dev = TRUE)
   packages <- deps$Package
-  expect_setequal(packages, c("devtools", "roxygen2"))
+  expect_setequal(packages, "devtools")
 })
 
 test_that("usages of library, etc. are properly handled", {
@@ -170,6 +170,19 @@ test_that("Suggests are dev. deps for all projects", {
 
 })
 
+test_that("roxygen2 is dev dep if needed", {
+
+  renv_tests_scope()
+
+  writeLines(c("RoxygenNote: 7.0.0"), con = "DESCRIPTION")
+  deps <- dependencies(dev = TRUE)
+  expect_equal(
+    deps[c("Package", "Dev")],
+    data.frame(Package = "roxygen2", Dev = TRUE, stringsAsFactors = FALSE)
+  )
+
+})
+
 test_that("packages referenced by modules::import() are discovered", {
   deps <- dependencies("resources/modules.R")
   expect_setequal(deps$Package, c("A", "B", "C", "D", "G", "H", "modules"))

From e8c6a268ed639150d2979bc0686e7abf5f3f20cb Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Tue, 6 Jun 2023 08:23:51 -0500
Subject: [PATCH 04/14] Extract out renv_description_dependencies()

---
 R/description.R | 22 ++++++++++++++++++++++
 R/package.R     | 15 ++-------------
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/R/description.R b/R/description.R
index d6b0f1ffc..36c4ad82d 100644
--- a/R/description.R
+++ b/R/description.R
@@ -112,6 +112,28 @@ renv_description_type <- function(path = NULL, desc = NULL) {
 
 }
 
+renv_description_dependencies <- function(path, subdir = NULL, fields = NULL) {
+
+  fields <- fields %||% c("Depends", "Imports", "LinkingTo")
+
+  desc <- tryCatch(
+    renv_description_read(path = path, subdir = subdir),
+    error = function(err) renv_dependencies_error(path, error = path)
+  )
+
+  out <- bind(map(desc[fields], renv_description_parse_field))
+  if (is.null(out)) {
+    data_frame(
+      Package = character(),
+      Require = character(),
+      Version = character()
+    )
+  } else {
+    out[out$Package != "R", , drop = FALSE]
+  }
+
+}
+
 # parse the dependency requirements normally presented in
 # Depends, Imports, Suggests, and so on
 renv_description_parse_field <- function(field) {
diff --git a/R/package.R b/R/package.R
index 5e0bdab3c..e5fc34432 100644
--- a/R/package.R
+++ b/R/package.R
@@ -251,9 +251,6 @@ renv_package_dependencies_impl <- function(package,
                                            libpaths = NULL,
                                            fields = NULL)
 {
-  # skip the 'R' package
-  if (package == "R")
-    return()
 
   # if we've already visited this package, bail
   if (exists(package, envir = visited, inherits = FALSE))
@@ -275,16 +272,8 @@ renv_package_dependencies_impl <- function(package,
   # we know the path, so set it now
   assign(package, location, envir = visited, inherits = FALSE)
 
-  # find its dependencies from the DESCRIPTION file
-  desc <- tryCatch(
-    renv_description_read(location),
-    error = function(err) renv_dependencies_error(path, error = path)
-  )
-
-  fields <- c("Depends", "Imports", "LinkingTo")
-  deps <- bind(map(desc[fields], renv_description_parse_field))
-  subpackages <- setdiff(unique(unlist(deps$Package)), "R")
-
+  deps <- renv_description_dependencies(location)
+  subpackages <- unique(deps$Package)
   for (subpackage in subpackages)
     renv_package_dependencies_impl(subpackage, visited, libpaths, fields)
 }

From e89c612bdafbeb2eed726e39a7db667b4aac468d Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Tue, 6 Jun 2023 08:43:31 -0500
Subject: [PATCH 05/14] Try a different approach

---
 R/dependencies.R                   | 10 ++++++++--
 R/retrieve.R                       |  6 +++++-
 tests/testthat/test-dependencies.R |  8 --------
 tests/testthat/test-install.R      | 10 ++++++++++
 4 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/R/dependencies.R b/R/dependencies.R
index 380584708..138db4e9c 100644
--- a/R/dependencies.R
+++ b/R/dependencies.R
@@ -516,7 +516,8 @@ renv_dependencies_discover_renv_lock <- function(path) {
 renv_dependencies_discover_description <- function(path,
                                                    fields = NULL,
                                                    subdir = NULL,
-                                                   project = NULL)
+                                                   project = NULL,
+                                                   include_dev = TRUE)
 {
   dcf <- catch(renv_description_read(path = path, subdir = subdir))
   if (inherits(dcf, "error"))
@@ -559,7 +560,12 @@ renv_dependencies_discover_description <- function(path,
     data <- c(data, list(renv_dependencies_list(path, "roxygen2", dev = TRUE)))
   }
 
-  bind(data)
+  out <- bind(data)
+
+  if (!is.null(out) && !include_dev) {
+    out <- out[!out$Dev, , drop = FALSE]
+  }
+  out
 
 }
 
diff --git a/R/retrieve.R b/R/retrieve.R
index 9f0f2ba6f..26a8e9569 100644
--- a/R/retrieve.R
+++ b/R/retrieve.R
@@ -1001,7 +1001,11 @@ renv_retrieve_successful <- function(record, path, install = TRUE) {
   # record this package's requirements
   state <- renv_restore_state()
   requirements <- state$requirements
-  deps <- renv_dependencies_discover_description(path, subdir = subdir)
+  deps <- renv_dependencies_discover_description(
+    path,
+    subdir = subdir,
+    include_dev = FALSE
+  )
   if (length(deps$Source))
     deps$Source <- record$Package
 
diff --git a/tests/testthat/test-dependencies.R b/tests/testthat/test-dependencies.R
index 2d16c1d69..3a75a8b9a 100644
--- a/tests/testthat/test-dependencies.R
+++ b/tests/testthat/test-dependencies.R
@@ -210,14 +210,6 @@ test_that("Suggest dependencies are ignored by default", {
   expect_false(renv_package_installed("egg"))
 })
 
-test_that("Suggest dependencies are used when requested", {
-  renv_tests_scope("breakfast")
-  fields <- c("Imports", "Depends", "LinkingTo", "Suggests")
-  settings$package.dependency.fields(fields)
-  install("breakfast")
-  expect_true(renv_package_installed("egg"))
-})
-
 test_that("a call to geom_hex() implies a dependency on ggplot2", {
 
   file <- renv_test_code({
diff --git a/tests/testthat/test-install.R b/tests/testthat/test-install.R
index 4c940f556..e24e4a9fd 100644
--- a/tests/testthat/test-install.R
+++ b/tests/testthat/test-install.R
@@ -468,6 +468,16 @@ test_that("repositories containing multiple packages can be installed", {
 
 })
 
+test_that("Suggest dependencies are used when requested", {
+
+  renv_tests_scope("breakfast")
+  fields <- c("Imports", "Depends", "LinkingTo", "Suggests")
+  settings$package.dependency.fields(fields)
+  install("breakfast")
+  expect_true(renv_package_installed("egg"))
+
+})
+
 test_that("custom dependency fields in install are supported", {
 
   skip_on_cran()

From 3379997dce0949632b152e9c732f56b3a9041302 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Tue, 6 Jun 2023 09:07:07 -0500
Subject: [PATCH 06/14] Simplify implementation

---
 R/dependencies.R                   |  1 +
 R/description.R                    | 22 ----------------------
 R/package.R                        |  4 ++--
 tests/testthat/test-dependencies.R |  2 +-
 4 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/R/dependencies.R b/R/dependencies.R
index 138db4e9c..88d977e81 100644
--- a/R/dependencies.R
+++ b/R/dependencies.R
@@ -936,6 +936,7 @@ renv_dependencies_discover_rproj <- function(path) {
   deps <- stack()
   if (identical(props$PackageUseDevtools, "Yes")) {
     deps$push("devtools")
+    deps$push("roxygen2")
   }
 
   renv_dependencies_list(path, deps$data(), dev = TRUE)
diff --git a/R/description.R b/R/description.R
index 36c4ad82d..d6b0f1ffc 100644
--- a/R/description.R
+++ b/R/description.R
@@ -112,28 +112,6 @@ renv_description_type <- function(path = NULL, desc = NULL) {
 
 }
 
-renv_description_dependencies <- function(path, subdir = NULL, fields = NULL) {
-
-  fields <- fields %||% c("Depends", "Imports", "LinkingTo")
-
-  desc <- tryCatch(
-    renv_description_read(path = path, subdir = subdir),
-    error = function(err) renv_dependencies_error(path, error = path)
-  )
-
-  out <- bind(map(desc[fields], renv_description_parse_field))
-  if (is.null(out)) {
-    data_frame(
-      Package = character(),
-      Require = character(),
-      Version = character()
-    )
-  } else {
-    out[out$Package != "R", , drop = FALSE]
-  }
-
-}
-
 # parse the dependency requirements normally presented in
 # Depends, Imports, Suggests, and so on
 renv_description_parse_field <- function(field) {
diff --git a/R/package.R b/R/package.R
index e5fc34432..5be9d5b9d 100644
--- a/R/package.R
+++ b/R/package.R
@@ -272,8 +272,8 @@ renv_package_dependencies_impl <- function(package,
   # we know the path, so set it now
   assign(package, location, envir = visited, inherits = FALSE)
 
-  deps <- renv_description_dependencies(location)
-  subpackages <- unique(deps$Package)
+  deps <- renv_dependencies_discover_description(location, include_dev = FALSE)
+  subpackages <- deps$Package
   for (subpackage in subpackages)
     renv_package_dependencies_impl(subpackage, visited, libpaths, fields)
 }
diff --git a/tests/testthat/test-dependencies.R b/tests/testthat/test-dependencies.R
index 3a75a8b9a..32bb55e21 100644
--- a/tests/testthat/test-dependencies.R
+++ b/tests/testthat/test-dependencies.R
@@ -4,7 +4,7 @@ test_that(".Rproj files requesting devtools is handled", {
   writeLines("PackageUseDevtools: Yes", "project.Rproj")
   deps <- dependencies(dev = TRUE)
   packages <- deps$Package
-  expect_setequal(packages, "devtools")
+  expect_setequal(packages, c("devtools", "roxygen2"))
 })
 
 test_that("usages of library, etc. are properly handled", {

From db0da3fedd39a890611302309a3add84fcb5c35a Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Tue, 6 Jun 2023 09:08:09 -0500
Subject: [PATCH 07/14] Revert another change

---
 R/package.R | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/R/package.R b/R/package.R
index 5be9d5b9d..aa7d71e6f 100644
--- a/R/package.R
+++ b/R/package.R
@@ -252,6 +252,10 @@ renv_package_dependencies_impl <- function(package,
                                            fields = NULL)
 {
 
+  # skip the 'R' package
+  if (package == "R")
+    return()
+
   # if we've already visited this package, bail
   if (exists(package, envir = visited, inherits = FALSE))
     return()

From 9e29585a49a7f02b3444ff715e20129795a191e5 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Tue, 6 Jun 2023 09:08:50 -0500
Subject: [PATCH 08/14] Reduce spurious diffs

---
 R/package.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/package.R b/R/package.R
index aa7d71e6f..a8904a1e2 100644
--- a/R/package.R
+++ b/R/package.R
@@ -251,7 +251,6 @@ renv_package_dependencies_impl <- function(package,
                                            libpaths = NULL,
                                            fields = NULL)
 {
-
   # skip the 'R' package
   if (package == "R")
     return()
@@ -276,6 +275,7 @@ renv_package_dependencies_impl <- function(package,
   # we know the path, so set it now
   assign(package, location, envir = visited, inherits = FALSE)
 
+  # find its dependencies from the DESCRIPTION file
   deps <- renv_dependencies_discover_description(location, include_dev = FALSE)
   subpackages <- deps$Package
   for (subpackage in subpackages)

From 9cd37e9ab24520d420e15ab1929c7dc638c54107 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Tue, 6 Jun 2023 13:52:44 -0500
Subject: [PATCH 09/14] Don't install Suggested dependencies of depdendencies

---
 NEWS.md                            |  3 +++
 R/dependencies.R                   | 28 ++++++++--------------------
 R/package.R                        |  2 +-
 R/project.R                        | 13 +++++++++++++
 R/retrieve.R                       |  2 +-
 R/settings.R                       |  9 ++++-----
 man/settings.Rd                    |  9 ++++-----
 tests/testthat/test-dependencies.R | 13 -------------
 8 files changed, 34 insertions(+), 45 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index 40bc0da84..3bef24524 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -5,6 +5,9 @@
   as development dependencies regardless of whether or not they're in 
   "package" project.
 
+* `settings$package.dependency.fields()` now only affects packages installed
+  directly by the user, not downstream dependencies of those packages.
+
 * If `renv::snapshot()` finds missing packages, a new prompt allows you to 
   install them before continuing (#1198).
 
diff --git a/R/dependencies.R b/R/dependencies.R
index 88d977e81..65fe3ef39 100644
--- a/R/dependencies.R
+++ b/R/dependencies.R
@@ -517,7 +517,7 @@ renv_dependencies_discover_description <- function(path,
                                                    fields = NULL,
                                                    subdir = NULL,
                                                    project = NULL,
-                                                   include_dev = TRUE)
+                                                   is_dependency = FALSE)
 {
   dcf <- catch(renv_description_read(path = path, subdir = subdir))
   if (inherits(dcf, "error"))
@@ -541,35 +541,23 @@ renv_dependencies_discover_description <- function(path,
       sprintf("Config/renv/profiles/%s/dependencies", profile)
 
     fields <- c(fields, "Suggests", profile_field)
-    proj_desc <- TRUE
-
-  } else {
-    proj_desc <- FALSE
   }
 
+  if (is_dependency)
+    fields <- setdiff(fields, "Suggests")
+
   data <- map(
     fields,
     renv_dependencies_discover_description_impl,
     dcf  = dcf,
-    path = path,
-    proj_desc = proj_desc
+    path = path
   )
 
-  # Infer a dependency on roxygen2 if RoxygenNote present
-  if (!is.null(dcf$RoxygenNote)) {
-    data <- c(data, list(renv_dependencies_list(path, "roxygen2", dev = TRUE)))
-  }
-
-  out <- bind(data)
-
-  if (!is.null(out) && !include_dev) {
-    out <- out[!out$Dev, , drop = FALSE]
-  }
-  out
+  bind(data)
 
 }
 
-renv_dependencies_discover_description_impl <- function(dcf, field, path, proj_desc) {
+renv_dependencies_discover_description_impl <- function(dcf, field, path) {
 
   # read field
   contents <- dcf[[field]]
@@ -599,7 +587,7 @@ renv_dependencies_discover_description_impl <- function(dcf, field, path, proj_d
     extract_chr(matches, 2L),
     extract_chr(matches, 3L),
     extract_chr(matches, 4L),
-    dev = proj_desc && field == "Suggests"
+    dev = field == "Suggests"
   )
 
 }
diff --git a/R/package.R b/R/package.R
index a8904a1e2..f73aa0993 100644
--- a/R/package.R
+++ b/R/package.R
@@ -276,7 +276,7 @@ renv_package_dependencies_impl <- function(package,
   assign(package, location, envir = visited, inherits = FALSE)
 
   # find its dependencies from the DESCRIPTION file
-  deps <- renv_dependencies_discover_description(location, include_dev = FALSE)
+  deps <- renv_dependencies_discover_description(location, is_dependency = TRUE)
   subpackages <- deps$Package
   for (subpackage in subpackages)
     renv_package_dependencies_impl(subpackage, visited, libpaths, fields)
diff --git a/R/project.R b/R/project.R
index 6edf82f3f..0df1a15eb 100644
--- a/R/project.R
+++ b/R/project.R
@@ -126,6 +126,19 @@ renv_project_remotes <- function(project, fields = NULL) {
   ignored <- renv_project_ignored_packages(project = project)
   specs <- specs[setdiff(names(specs), c("R", ignored))]
 
+  # if any Roxygen fields are included,
+  # infer a dependency on roxygen2 and devtools
+  desc <- renv_description_read(descpath)
+  if (any(grepl("^Roxygen", names(desc)))) {
+    for (package in c("devtools", "roxygen2")) {
+      if (!package %in% ignored) {
+        specs[[package]] <-
+          specs[[package]] %||%
+          renv_dependencies_list(descpath, package, dev = TRUE)
+      }
+    }
+  }
+
   # now, try to resolve the packages
   records <- enumerate(specs, function(package, spec) {
 
diff --git a/R/retrieve.R b/R/retrieve.R
index 26a8e9569..b5af742d4 100644
--- a/R/retrieve.R
+++ b/R/retrieve.R
@@ -1004,7 +1004,7 @@ renv_retrieve_successful <- function(record, path, install = TRUE) {
   deps <- renv_dependencies_discover_description(
     path,
     subdir = subdir,
-    include_dev = FALSE
+    is_dependency = !record$Package %in% state$packages
   )
   if (length(deps$Source))
     deps$Source <- record$Package
diff --git a/R/settings.R b/R/settings.R
index c6eb886c8..0adf9c69a 100644
--- a/R/settings.R
+++ b/R/settings.R
@@ -358,11 +358,10 @@ renv_settings_impl <- function(name, default, scalar, validate, coerce, update)
 #'
 #' ## `package.dependency.fields`
 #'
-#' During dependency discovery, renv uses the fields of an installed
-#' package's `DESCRIPTION` file to determine that package's recursive
-#' dependencies. By default, the `Imports`, `Depends` and `LinkingTo` fields
-#' are used. If you'd prefer that renv also captures the `Suggests`
-#' dependencies for a package, you can set this to
+#' When explicitly installing a package with `install()`, what fields
+#' should be used to determine that packages dependencies? The default
+#' uses `Imports`, `Depends` and `LinkingTo` fields, but you also want
+#' to install `Suggests` dependencies for a package, you can set this to
 #' `c("Imports", "Depends", "LinkingTo", "Suggests")`.
 #'
 #' ## `r.version`
diff --git a/man/settings.Rd b/man/settings.Rd
index 439968643..299a7da66 100644
--- a/man/settings.Rd
+++ b/man/settings.Rd
@@ -50,11 +50,10 @@ added to the lockfile, that entry in the lockfile will not be ignored.
 
 \subsection{\code{package.dependency.fields}}{
 
-During dependency discovery, renv uses the fields of an installed
-package's \code{DESCRIPTION} file to determine that package's recursive
-dependencies. By default, the \code{Imports}, \code{Depends} and \code{LinkingTo} fields
-are used. If you'd prefer that renv also captures the \code{Suggests}
-dependencies for a package, you can set this to
+When explicitly installing a package with \code{install()}, what fields
+should be used to determine that packages dependencies? The default
+uses \code{Imports}, \code{Depends} and \code{LinkingTo} fields, but you also want
+to install \code{Suggests} dependencies for a package, you can set this to
 \code{c("Imports", "Depends", "LinkingTo", "Suggests")}.
 }
 
diff --git a/tests/testthat/test-dependencies.R b/tests/testthat/test-dependencies.R
index 32bb55e21..24291add6 100644
--- a/tests/testthat/test-dependencies.R
+++ b/tests/testthat/test-dependencies.R
@@ -170,19 +170,6 @@ test_that("Suggests are dev. deps for all projects", {
 
 })
 
-test_that("roxygen2 is dev dep if needed", {
-
-  renv_tests_scope()
-
-  writeLines(c("RoxygenNote: 7.0.0"), con = "DESCRIPTION")
-  deps <- dependencies(dev = TRUE)
-  expect_equal(
-    deps[c("Package", "Dev")],
-    data.frame(Package = "roxygen2", Dev = TRUE, stringsAsFactors = FALSE)
-  )
-
-})
-
 test_that("packages referenced by modules::import() are discovered", {
   deps <- dependencies("resources/modules.R")
   expect_setequal(deps$Package, c("A", "B", "C", "D", "G", "H", "modules"))

From 4fad8a247d432a7331a2ae008786f5cd7b719282 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Tue, 6 Jun 2023 13:54:25 -0500
Subject: [PATCH 10/14] Simplify dependency fields

---
 R/dependencies.R | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/R/dependencies.R b/R/dependencies.R
index 65fe3ef39..386b9c669 100644
--- a/R/dependencies.R
+++ b/R/dependencies.R
@@ -523,29 +523,30 @@ renv_dependencies_discover_description <- function(path,
   if (inherits(dcf, "error"))
     return(renv_dependencies_error(path, error = dcf))
 
-  # Most callers don't pass in project so we need to get it from global state
-  project <- project %||%
-    renv_dependencies_state(key = "root") %||%
-    renv_restore_state(key = "root") %||%
-    renv_project_resolve()
+  if (is_dependency) {
+    fields <- c("Imports", "Depends", "LinkingTo")
+  } else {
+    # Most callers don't pass in project so we need to get it from global state
+    project <- project %||%
+      renv_dependencies_state(key = "root") %||%
+      renv_restore_state(key = "root") %||%
+      renv_project_resolve()
 
-  fields <- fields %||% settings$package.dependency.fields(project = project)
+    fields <- fields %||% settings$package.dependency.fields(project = project)
 
-  # if this is the DESCRIPTION file for the active project, include
-  # the dependencies for the active profile (if any) and Suggested fields.
-  if (renv_path_same(file.path(project, "DESCRIPTION"), path)) {
+    # if this is the DESCRIPTION file for the active project, include
+    # the dependencies for the active profile (if any) and Suggested fields.
+    if (renv_path_same(file.path(project, "DESCRIPTION"), path)) {
 
-    # collect profile-specific dependencies as well
-    profile <- renv_profile_get()
-    profile_field <- if (length(profile))
-      sprintf("Config/renv/profiles/%s/dependencies", profile)
+      # collect profile-specific dependencies as well
+      profile <- renv_profile_get()
+      profile_field <- if (length(profile))
+        sprintf("Config/renv/profiles/%s/dependencies", profile)
 
-    fields <- c(fields, "Suggests", profile_field)
+      fields <- c(fields, "Suggests", profile_field)
+    }
   }
 
-  if (is_dependency)
-    fields <- setdiff(fields, "Suggests")
-
   data <- map(
     fields,
     renv_dependencies_discover_description_impl,

From f8691a7c3d38abe4644a9ae1f71dd5d3e1a5a653 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Tue, 6 Jun 2023 16:54:20 -0500
Subject: [PATCH 11/14] Try just using fields

---
 R/dependencies.R | 11 +++++------
 R/package.R      |  2 +-
 R/project.R      |  2 --
 R/retrieve.R     |  2 +-
 4 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/R/dependencies.R b/R/dependencies.R
index 386b9c669..7a93344ab 100644
--- a/R/dependencies.R
+++ b/R/dependencies.R
@@ -516,23 +516,20 @@ renv_dependencies_discover_renv_lock <- function(path) {
 renv_dependencies_discover_description <- function(path,
                                                    fields = NULL,
                                                    subdir = NULL,
-                                                   project = NULL,
-                                                   is_dependency = FALSE)
+                                                   project = NULL)
 {
   dcf <- catch(renv_description_read(path = path, subdir = subdir))
   if (inherits(dcf, "error"))
     return(renv_dependencies_error(path, error = dcf))
 
-  if (is_dependency) {
-    fields <- c("Imports", "Depends", "LinkingTo")
-  } else {
+  if (is.null(fields)) {
     # Most callers don't pass in project so we need to get it from global state
     project <- project %||%
       renv_dependencies_state(key = "root") %||%
       renv_restore_state(key = "root") %||%
       renv_project_resolve()
 
-    fields <- fields %||% settings$package.dependency.fields(project = project)
+    fields <- settings$package.dependency.fields(project = project)
 
     # if this is the DESCRIPTION file for the active project, include
     # the dependencies for the active profile (if any) and Suggested fields.
@@ -545,6 +542,8 @@ renv_dependencies_discover_description <- function(path,
 
       fields <- c(fields, "Suggests", profile_field)
     }
+  } else {
+    fields <- renv_description_dependency_fields(fields)
   }
 
   data <- map(
diff --git a/R/package.R b/R/package.R
index f73aa0993..78c6779db 100644
--- a/R/package.R
+++ b/R/package.R
@@ -276,7 +276,7 @@ renv_package_dependencies_impl <- function(package,
   assign(package, location, envir = visited, inherits = FALSE)
 
   # find its dependencies from the DESCRIPTION file
-  deps <- renv_dependencies_discover_description(location, is_dependency = TRUE)
+  deps <- renv_dependencies_discover_description(location, fields = "strong")
   subpackages <- deps$Package
   for (subpackage in subpackages)
     renv_package_dependencies_impl(subpackage, visited, libpaths, fields)
diff --git a/R/project.R b/R/project.R
index 0df1a15eb..e305dc8b2 100644
--- a/R/project.R
+++ b/R/project.R
@@ -109,10 +109,8 @@ renv_project_remotes <- function(project, fields = NULL) {
   remotes <- renv_project_remotes_field(project, descpath)
 
   # next, find packages mentioned in the DESCRIPTION file
-  fields <- fields %||% c("Depends", "Imports", "LinkingTo", "Suggests")
   deps <- renv_dependencies_discover_description(
     path    = descpath,
-    fields  = fields,
     project = project
   )
 
diff --git a/R/retrieve.R b/R/retrieve.R
index b5af742d4..bda571ae9 100644
--- a/R/retrieve.R
+++ b/R/retrieve.R
@@ -1004,7 +1004,7 @@ renv_retrieve_successful <- function(record, path, install = TRUE) {
   deps <- renv_dependencies_discover_description(
     path,
     subdir = subdir,
-    is_dependency = !record$Package %in% state$packages
+    fields = if (!record$Package %in% state$packages) "most"
   )
   if (length(deps$Source))
     deps$Source <- record$Package

From bcd32396c4896ac51ec278ce880c08d40944bc6f Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Wed, 7 Jun 2023 06:09:03 -0500
Subject: [PATCH 12/14] Strong not most

---
 R/retrieve.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/retrieve.R b/R/retrieve.R
index bda571ae9..2cf9a895d 100644
--- a/R/retrieve.R
+++ b/R/retrieve.R
@@ -1004,7 +1004,7 @@ renv_retrieve_successful <- function(record, path, install = TRUE) {
   deps <- renv_dependencies_discover_description(
     path,
     subdir = subdir,
-    fields = if (!record$Package %in% state$packages) "most"
+    fields = if (!record$Package %in% state$packages) "strong"
   )
   if (length(deps$Source))
     deps$Source <- record$Package

From ddd19e9e1dd3d94f13b24ac567092b451e1cdd07 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Wed, 7 Jun 2023 06:15:44 -0500
Subject: [PATCH 13/14] Tweak news bullet

---
 NEWS.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index 36db29759..0a719153e 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -2,8 +2,8 @@
 # renv 0.18.0  (UNRELEASED)
 
 * `dependencies()` now marks `Suggested` packages listed in `DESCRIPTION` files
-  as development dependencies regardless of whether or not they're in 
-  "package" project.
+  as development dependencies regardless of whether or not they're a "package" 
+  project.
 
 * `settings$package.dependency.fields()` now only affects packages installed
   directly by the user, not downstream dependencies of those packages.

From 80e99a23e9956fc560fcee8ffb8f56818c9141ac Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Wed, 7 Jun 2023 06:27:17 -0500
Subject: [PATCH 14/14] No longer use the DESCRIPTION type

---
 R/description.R | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/R/description.R b/R/description.R
index d6b0f1ffc..664751da1 100644
--- a/R/description.R
+++ b/R/description.R
@@ -83,35 +83,6 @@ renv_description_path <- function(path) {
   path
 }
 
-renv_description_type <- function(path = NULL, desc = NULL) {
-
-  # read DESCRIPTION file when 'desc' not explicitly supplied
-  if (is.null(desc)) {
-
-    # read DESCRIPTION file
-    desc <- catch(renv_description_read(path))
-    if (inherits(desc, "error")) {
-      warning(desc)
-      return("unknown")
-    }
-
-  }
-
-  # check for explicitly recorded type
-  type <- desc$Type
-  if (!is.null(type))
-    return(tolower(type))
-
-  # infer otherwise from 'Package' field otherwise
-  package <- desc$Package
-  if (!is.null(package))
-    return("package")
-
-  # default to unknown
-  "unknown"
-
-}
-
 # parse the dependency requirements normally presented in
 # Depends, Imports, Suggests, and so on
 renv_description_parse_field <- function(field) {