Merge pull request #1886 from rstudio/fix-cran-load-hook-py-require-warning

Use startup messages for py_require checks during package load

Author: t-kalinowski

R/package.R

@@ -370,10 +370,10 @@ check_virtualenv_required_packages <- function(config) {
   # - numba==0.60.0
   # + numba==0.61.0
   if (any(grepl("downloading", pip_output, ignore.case = TRUE))) {
-    warning(paste(
+    warning_or_startup_message(paste(
       pip_output[grepl("downloading", pip_output, ignore.case = TRUE)],
       collapse = "\n"
-    ))
+    ), call. = FALSE)
   }
 
   # uv pip doesn't have an option to produce structured output,
@@ -399,7 +399,7 @@ check_virtualenv_required_packages <- function(config) {
       vapply(packages, function(pkg) any(startsWith(pkg, would_install_pkgs)), TRUE,
              USE.NAMES = FALSE)
     ]
-    warning(
+    warning_or_startup_message(
       "Some Python package requirements declared via `py_require()` are not",
       " installed in the selected Python environment: (", config$python, ")\n",
       "  ", paste0(would_install_pkgs, collapse=" "),
@@ -433,14 +433,7 @@ check_forbidden_initialization <- function() {
 
     call <- calls[[i]]
     frame <- frames[[i]]
-    if (!identical(call[[1]], as.name("runHook")))
-      next
-
-    bad <-
-      identical(call[[2]], ".onLoad") ||
-      identical(call[[2]], ".onAttach")
-
-    if (!bad)
+    if (!is_package_loading(list(call)))
       next
 
     pkgname <- tryCatch(

R/use_python.R

@@ -278,20 +278,10 @@ use_miniconda <- function(condaenv = NULL, required = NULL) {
 
 use_python_required <- function() {
 
-  # for now, assume that calls from within a package's .onLoad() are
+  # for now, assume that calls from within package load hooks are
   # advisory, but we may consider relaxing this in the future
-  calls <- sys.calls()
-  for (call in calls) {
-
-    match <-
-      length(call) >= 2 &&
-      identical(call[[1L]], as.name("runHook")) &&
-      identical(call[[2L]], ".onLoad")
-
-    if (match)
-      return(FALSE)
-
-  }
+  if (is_package_loading())
+    return(FALSE)
 
   # default to TRUE
   TRUE

R/utils.R

@@ -742,6 +742,36 @@ parent.pkg <- function(env = parent.frame(2)) {
     NULL # print visible
 }
 
+is_package_loading <- function(calls = sys.calls()) {
+  for (call in calls) {
+    if (length(call) < 2L)
+      next
+
+    hook <- call[[2L]]
+    if (is.symbol(hook))
+      hook <- as.character(hook)
+
+    match <-
+      identical(call[[1L]], as.name("runHook")) &&
+      is.character(hook) &&
+      hook %in% c(".onLoad", ".onAttach")
+
+    if (match)
+      return(TRUE)
+  }
+
+  FALSE
+}
+
+warning_or_startup_message <- function(..., call. = TRUE) {
+  msg <- .makeMessage(...)
+  if (is_package_loading()) {
+    packageStartupMessage(msg)
+  } else {
+    warning(msg, call. = call.)
+  }
+}
+
 warn_and_return <- function(..., call. = TRUE) {
   cond <- if (inherits(..1, "condition")) {
     ..1

tests/testthat/test-py_require.R

@@ -244,3 +244,39 @@ test_that("py_require() warns missing packages in a virtual env", {
     }
   )
 })
+
+test_that("package load hooks get startup messages, not warnings", {
+  expect_true(is_package_loading(list(quote(runHook(".onLoad", foo)))))
+  expect_true(is_package_loading(list(quote(runHook(.onAttach, foo)))))
+  expect_false(is_package_loading(list(quote(runHook(".onUnload", foo)))))
+
+  msg <- paste(
+    "Some Python package requirements declared via `py_require()` are not",
+    "installed in the selected Python environment: (/path/to/python)\n",
+    "  numpy"
+  )
+
+  expect_warning(
+    warning_or_startup_message(msg, call. = FALSE),
+    "Some Python package requirements declared via `py_require()` are not",
+    fixed = TRUE
+  )
+
+  runHook <- function(name, expr) expr()
+  expect_message(
+    expect_warning(
+      runHook(".onLoad", function() warning_or_startup_message(msg, call. = FALSE)),
+      NA
+    ),
+    "Some Python package requirements declared via `py_require()` are not",
+    fixed = TRUE
+  )
+  expect_message(
+    expect_warning(
+      runHook(".onAttach", function() warning_or_startup_message(msg, call. = FALSE)),
+      NA
+    ),
+    "Some Python package requirements declared via `py_require()` are not",
+    fixed = TRUE
+  )
+})