r-lib / devtools

Tools to make an R developer's life easier
https://devtools.r-lib.org
Other
2.37k stars 755 forks source link

check_built() changes default for 'error_on' in undocumented way #2506

Closed kenahoo closed 8 months ago

kenahoo commented 1 year ago

In our build process, I recently replaced some code that shelled out to R CMD check with devtools::check_built(). Errors started happening, but I couldn't reproduce them when I fired up RStudio and did check_built() from there.

It turns out that check_built() is modifying the default for its error_on argument in an undocumented way - if the build is happening in a non-interactive session, it defaults to "error", otherwise it acts as documented (defaults to "never").

The current docs (version 2.4.5):

error_on

Whether to throw an error on ⁠R CMD check⁠ failures. Note that the check is always completed
(unless a timeout happens), and the error is only thrown after completion. If "never", then 
no errors are thrown. If "error", then only ERROR failures generate errors. If "warning", then 
WARNING failures generate errors as well. If "note", then any check failure generated an error. 
Its default can be modified with the RCMDCHECK_ERROR_ON environment variable. If that is 
not set, then "never" is used.

The current code:

> devtools::check_built
function (path = NULL, cran = TRUE, remote = FALSE, incoming = remote, 
    force_suggests = FALSE, run_dont_test = FALSE, manual = FALSE, 
    args = "--timings", env_vars = NULL, check_dir = tempdir(), 
    quiet = FALSE, error_on = c("never", "error", "warning", 
        "note")) 
{
    if (missing(error_on) && !interactive()) {
        error_on <- "warning"
    }

It's probably also worth mentioning that RCMDCHECK_ERROR_ON doesn't seem to be consulted either?

My inclination for a fix would be to change the code rather than the docs (for error_on - I have no opinion on RCMDCHECK_ERROR_ON), because it can be mysterious when a pipeline does one thing while interactive code does another.

jennybc commented 8 months ago

The confusion arises from the fact that devtools inherits the documentation of error_on from rcmdcheck::rcmdcheck().

The most practical fix is to change the devtools docs.