Could callr support user hooks?

[klmr]

My specific use-case is klmr/box#199. The package ‘box’ needs to figure out which script file it is called from. It uses the script filename on the R command line for this, and since ‘callr’ creates a temporary script path, ‘box’ goes looking for modules in that temporary path, rather than in the (user’s) calling code’s path. getwd() does not work, since the working directory is generally unrelated to the script path. Likewise here::here() won’t work (in fact, it falls back to getwd() in the situations I care about).

The linked issue shows a workaround, but that requires users to add code to both the caller and the callee. Ideally I’d like to add automatic support for ‘callr’ to ‘box’ but I believe this requires support from ‘callr’.

As far as I can tell ‘callr’ currently has no way of finding the calling script’s path: I can’t find any indication in its environment variables, the command line or the options. In fact, finding the calling script’s path in R is nontrivial (the ‘box’ implementation is ~100 SLOC, and hard-codes special handling for RStudio and for several packages).

I’ll admit that there’s limited usefulness for this outside of this specific case. And to have a solution that doesn’t depend on ‘box’ my suggestion would be to add a user hook for callr:::run_r which can modify the command line options being passed to the R subprocess (or the environment variables).

If I submitted such a PR, would it be considered for inclusion?

At its simplest, an implementation could look like this:

diff --git a/R/run.R b/R/run.R
index f3e9308..a87a87c 100644
--- a/R/run.R
+++ b/R/run.R
@@ -14,6 +14,10 @@ run_r <- function(options) {
     (!is.null(stdout) && !is.null(stderr) && stdout == stderr)
   )
 
+  for (fun in getHook('callr.run_r')) {
+    options <- tryCatch(match.fun(fun)(options), error = function (.) options)
+  }
+
   res <- with(
     options,
     with_envvar(

… But that exposes current implementation details (the presence of run_r, and the structure of the options list), and would imply that any future change to them would be a breaking change.

[gaborcsardi]

which can modify the command line options being passed to the R subprocess (or the environment variables).

Well, you can already modify environment variables, just set them, and they'll be passed to the subprocess.

[klmr]

which can modify the command line options being passed to the R subprocess (or the environment variables).

Well, you can already modify environment variables, just set them, and they'll be passed to the subprocess.

Yes, that’s essentially the workaround discussed in the linked issue (the workaround uses argument passing; the principle’s the same). But I can’t do so transparently when the user uses ‘callr’.

I should have led with a reprex. From the user’s perspective, the following should work, and “surprisingly” currently doesn’t:

• File /somepath/a.r

  callr::r(function () box::use(./b))

• File /somepath/b.r

  .on_load = function (...) message('hello from b')

When executing a.r (e.g. via RScript /somepath/a.r), the expected output is “hello from b”. Instead, I am getting the error message

Unable to load module “./b”; not found in “/var/folders/dr/rp46tjqs5qzfj_ghjpxqrttr0000gn/T//Rtmpzg81an”

The reason for the failure is that box::use(./b) isn’t executed from inside path/to/a.r, it’s executed from inside $TMPDIR/Rtmpzg81an/somefile.R.

Ideally from a user’s perspective, this should “just work”. But as the author of ‘box’, the best I can do is look at the environment and the command line for clues (unless I drop down to the OS level and snoop about the parent process).

(I realise that in the reprex above ‘box’ wouldn’t have the opportunity to install a hook in the caller, so the envisioned PR wouldn’t work. But in many relevant situations ‘box’ would already be loaded in the caller.)

---

Maybe more pragmatically: Is there a reliable way of finding out that I am inside a ‘callr’ subprocess? In the current release of ‘callr’ I could check if startsWith(basename(tail(commandArgs(), 1L)), 'callr-scr-'). However, this isn’t documented. Is it guaranteed to be reliable? Could a guaranteed, stable way be added?

[gaborcsardi]

Is there a reliable way of finding out that I am inside a ‘callr’ subprocess?

I would happy to expose some info in the subprocess, e.g.

• the fact that we are in a callr process,
• the original working directory of the parent process.

Do you need anything else?

[klmr]

• The original working directory is already available (getwd() in the child process already yields the correct result but as mentioned it’s unrelated to the script path so it’s not useful for my purposes).
• The parent process command line (commandArgs(trailingOnly = FALSE)) would be great. If this is passed via an environment variable, it would need to be encoded in some form to work around word splitting.

Beyond that, unfortunately for my specific purposes the information I’d need are quite complex (i.e.: whether the code was launched by knitr, Shiny or testthat, or whether it’s running inside RStudio …; currently, only ‘testthat’ uses environment variables to signal this), hence the idea for the user hook.

But having the original command line would already help a lot.

[gaborcsardi]

I can pass the knitr.in.progress and rstudio.notebook.executing options.

IDK what Shiny has to detect running in an app.

RStudio also sets the RSTUDIO env var. But I am not sure if a callr sub-process counts as an rstudio R session in this case.

We can pass the command line as well, but that seems a bit weird. What information do you use from that?

[klmr]

For knitr and RStudio, the mere presence is unfortunately not sufficient for my purposes — ‘box’ needs the information provided by knitr::current_input(dir = TRUE) and rstudioapi::getActiveDocumentContext()$path, respectively.

IDK what Shiny has to detect running in an app.

Shiny has a function shiny::isRunning for this. It doesn’t set any option/environment variable as far as I can tell.

We can pass the command line as well, but that seems a bit weird. What information do you use from that?

The script path (if present). Unfortunately depending on how R is called (Rscript/R CMD BATCH) this is passed differently.

… you may start to see why ‘box’ takes ~ 100 lines of code to figure out the calling code’s path in all these contexts.

[gaborcsardi]

Yeah, this does look quite messed up, and maybe adding hooks is the best way. With the caveat that people would need to load box on in the main process.

I can do this in the next release. You can also submit a PR if you like. Ideally it would be possible to set multiple hooks, so some simple api to do that would suffice I think. E.g. having named hooks (you can prefix the names with box:: for box) and a single callr::add_hook() function that can return options and/or env vars to set in the subprocess. If you call callr::add_hook() with NULL for a named hook, it will remove it.

No pressure, you can also wait for me to add this, I would guess I would do it in about a month or so.

[klmr]

I’d be happy to submit a PR.

Just to clarify, your idea would be to have the following API:

callr::add_hook(
    'hook-name',
    function() list(name = 'string', …) # list of key-value pairs
)

Rather than using the base::setHook/user hooks functionality?

---

Having a separate function that tells the caller whether they’re inside ‘callr’ would still be useful to have, to allow graceful (though not perfect) handling of situations where ‘box’ wasn’t loaded in the parent process.

[gaborcsardi]

Maybe something like this?

callr::add_hook("box::hook" = function() list(options = ..., envvars = ...))

IDK if we should have separate hooks for setting options and env vars, maybe not.

IDK if base::setHook et al. can handle arbitrary user-defined hooks.

Having a separate function that tells the caller whether they’re inside ‘callr’ would still be useful to have, to allow graceful (though not perfect) handling of situations where ‘box’ wasn’t loaded in the parent process.

Yeah.

[klmr]

IDK if we should have separate hooks for setting options and env vars, maybe not.

I agree, seems like overkill.

IDK if base::setHook et al. can handle arbitrary user-defined hooks.

It currently does, and in general it should, the API/documentation has no limitations (the API also really only handles setting and getting lists of functions, nothing about calling them, so there’s no reasonable way in which it could put constraints on the functions).

On the flip side, the base R API doesn’t allow naming the hooks the way you suggested. Not sure whether that’s useful (it makes removing hooks slightly easier).

[gaborcsardi]

Yeah, I think naming would be important, so let's just put them in a list or env in callr with callr::add_hook().

[gaborcsardi]

Closed by #203.