r-lib / rcmdcheck

Run R CMD check from R and collect the results
https://rcmdcheck.r-lib.org
Other
115 stars 27 forks source link

devtools::check (ultimately) invokes callr::rcmd_safe in a way that doesn't respect site profiles #192

Closed gmbecker closed 1 year ago

gmbecker commented 2 years ago

In corporate, shared computing environments (notably including RStudio server pro installations), site profiles are used extensively to configure e.g., libpaths and repositories.

devtools::check, however, ultimately starts an R process to run R CMD check that doesn't respect that site profile, due to this being the default behavior of callr::rcmd_safe, which devtools::check (after multiple levels of redirection) calls to actually perform the check.

This renders devtools::check unusable in these contexts. At the very least, there should be an argument to devtools::check which is passed down all the way to callr::rcmd_safe, but even beyond that, it really seems like the default (or only) behavior should be to mimic its parent process which, unless it was run with,e .g., --vanilla will be using the site profile if one exists.

Callstack from check to rcmd_safe:

Browse[2]> sys.calls()
[[1]]
devtools::check("~/gabe/checkedout/switchr")

[[2]]
check_built(built_path, cran = cran, remote = remote, incoming = incoming, 
    force_suggests = force_suggests, run_dont_test = run_dont_test, 
    manual = manual, args = args, env_vars = env_vars, quiet = quiet, 
    check_dir = check_dir, error_on = error_on)

[[3]]
withr::with_envvar(env_vars, action = "replace", {
    rcmdcheck::rcmdcheck(path, quiet = quiet, args = args, check_dir = check_dir, 
        error_on = error_on)
})

[[4]]
force(code)

[[5]]
rcmdcheck::rcmdcheck(path, quiet = quiet, args = args, check_dir = check_dir, 
    error_on = error_on)

[[6]]
with_dir(dirname(targz), do_check(targz, package = desc$get("Package")[[1]], 
    args = args, libpath = libpath, repos = repos, quiet = quiet, 
    timeout = timeout, env = env))

[[7]]
force(code)

[[8]]
do_check(targz, package = desc$get("Package")[[1]], args = args, 
    libpath = libpath, repos = repos, quiet = quiet, timeout = timeout, 
    env = env)

[[9]]
rcmd_safe("check", cmdargs = c(basename(targz), args), libpath = c(libdir, 
    libpath), user_profile = FALSE, repos = repos, stderr = "2>&1", 
    block_callback = callback, spinner = !quiet && should_add_spinner(), 
    timeout = timeout, fail_on_status = FALSE, env = chkenv)

Session info:

> sessionInfo()
R version 4.1.2 (2021-11-01)
Platform: x86_64-apple-darwin17.0 (64-bit)
Running under: macOS Big Sur 10.16

Matrix products: default
BLAS:   /Library/Frameworks/R.framework/Versions/4.1/Resources/lib/libRblas.0.dylib
LAPACK: /Library/Frameworks/R.framework/Versions/4.1/Resources/lib/libRlapack.dylib

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] devtools_2.4.3 usethis_2.1.5 

loaded via a namespace (and not attached):
 [1] magrittr_2.0.3    pkgload_1.2.4     R6_2.5.1          rlang_1.0.2      
 [5] fastmap_1.1.0     tcltk_4.1.2       tools_4.1.2       pkgbuild_1.3.1   
 [9] sessioninfo_1.2.2 cli_3.3.0         withr_2.5.0       ellipsis_0.3.2   
[13] remotes_2.4.2     rprojroot_2.0.3   lifecycle_1.0.1   crayon_1.5.1     
[17] brio_1.1.3        processx_3.5.3    purrr_0.3.4       callr_3.7.0      
[21] fs_1.5.2          ps_1.6.0          testthat_3.1.3    memoise_2.0.1    
[25] glue_1.6.2        cachem_1.0.6      compiler_4.1.2    desc_1.4.1       
[29] prettyunits_1.1.1
hadley commented 2 years ago

Moved to rcmdcheck since that seems to be the source of the problem.

gaborcsardi commented 2 years ago

In corporate, shared computing environments (notably including RStudio server pro installations), site profiles are used extensively to configure e.g., libpaths and repositories.

callr respects libpaths and also repositories, it uses the values of the current session:

❯ dir.create(tmp <- tempfile())
❯ .libPaths(c(tmp, .libPaths()))
❯ .libPaths()
[1] "/private/var/folders/ph/fpcmzfd16rgbbk8mxvy9m2_h0000gn/T/RtmpwEQzvx/file8b21da1ecc2"
[2] "/Users/gaborcsardi/Library/R/arm64/4.2/library"
[3] "/Library/Frameworks/R.framework/Versions/4.2-arm64/Resources/library"

❯ callr::r(function() .libPaths())
[1] "/private/var/folders/ph/fpcmzfd16rgbbk8mxvy9m2_h0000gn/T/RtmpwEQzvx/file8b21da1ecc2"
[2] "/Users/gaborcsardi/Library/R/arm64/4.2/library"
[3] "/Library/Frameworks/R.framework/Versions/4.2-arm64/Resources/library"

If the libpath and the repos option is respected, then is there anything else that you need from the profile?

Otherwise, it is best to ignore profiles in most cases, because people load packages in them, and that's probably not something you want for R CMD check.

gmbecker commented 2 years ago

Hi @gaborcsardi,

R CMD check hits both the user and site profile out of the box. I would expect rcmdcheck to do the same by default, even you want to leave that configurable so it can be turned off, e.g., in the situations you describe.

gaborcsardi commented 2 years ago

We can't change the default, that is a breaking change.

Do you have an example where the current behavior is causing problems?

gmbecker commented 2 years ago

It would be a user-facing change in behavior, yes. The current behavior isn't right though, for the reasons I stated above. Currently rcmdcheck does not do the same thing as R CMD check run from a terminal on the same machine, a fact which as far as I can tell is not directly documented anywhere.

But even if the default behavior remains incorrect, neither devtools::check nor rcmdcheck accept the system_profile and user_profile arguments at all so it's not overridable.

And yes, when devtools::check is invoked from the build menu in Rstudio Server, something happens that causes the issue I describe, in practice, in a real corporate setting. Running R CMD check (by turning devtools funcs off in the build tools config or by running it from the terminal pane) works fine as it hits the necessary site profile.

gaborcsardi commented 2 years ago

works fine as it hits the necessary site profile.

I am sorry, what I am trying to ask is, what do you need from the site profile?

gaborcsardi commented 2 years ago

Currently rcmdcheck does not do the same thing as R CMD check run from a terminal on the same machine

I just tried and R -q -e 'rcmdcheck::rcmdcheck(...)' sets the library path and also the repos option for the check process, to the same values as the profiles would set them in a regular R process.

This is why I am asking, what other option or setting do you have in the profile that is important for the check?

gmbecker commented 2 years ago

repos is the one that the client is being bitten by. They are not set when devtools::check is invoked via the build tools menu/button in Rstudio. I can't speak to why not, but I can tell you it doesn't.

When "use devtools functions when available" is unchecked, and R CMD check invoked directly by the IDE, the site profile is hit and everything works.

gaborcsardi commented 2 years ago

That's probably on RStudio then, it passes --vanilla to the R process that starts the check:

> ps::ps_cmdline(ps::ps_children()[[2]])
[1] "/Library/Frameworks/R.framework/Versions/4.2-arm64/Resources/bin/exec/R"  
[2] "--vanilla"                                                                
[3] "-s"                                                                       
[4] "-e"                                                                       
[5] "devtools::check(document~+~=~+~FALSE,~+~check_dir~+~=~+~dirname(getwd()))"

But they do not pass --vanilla to the non-devtools check:

> ps::ps_cmdline(ps::ps_children()[[2]])
[1] "sh"                                                                   
[2] "/Library/Frameworks/R.framework/Versions/4.2-arm64/Resources/bin/Rcmd"
[3] "check"                                                                
[4] "dotenv_1.0.3.9000.tar.gz"                                             
gaborcsardi commented 1 year ago

As this is something to be addressed in RStudio, I am going to close it here.