Feature: user hooks

[klmr]

This is a WIP PR to implement #200. It

• adds a function add_hook with semantics roughly as discussed
• invokes the user hooks in setup_context
• adds a unit test for the new feature
• sets an environment variable CALLR_IS_RUNNING=true, which signals to a subprocess that it is being invoked via ‘callr’.

The last point was also discussed in #200 but is not necessarily related to the hook feature. I’m happy to pull it out, but it feels too small for its own PR. The convention to use the lower-case value true for the environment variable mirrors several other packages, including ‘testthat’.

At the moment the implementation puts no restrictions on the user’s modifications of the options that are used to construct the subprocess. This makes the implementation simple, but it also leaks implementation details and increases the maintenance burden (because future changes to the options structure would be breaking changes).

I don’t suggest keeping this structure, I’m only using it here to start a discussion about the desired signature of the hook functions. What should the hook be allowed to see and to modify (and at what point during the launching of the subprocess)?

There’s probably other stuff to improve in the documentation of the function, and maybe its implementation.

[kiwiroy]

I'm thinking that hooks would be a good addition to callr. When using R in a Singularity container transferring the environment to the child process can become challenging. For example, wanting to override a variable set in the container. This is trivial for the initial running container, using renv as an example.

# R is a symlink to the R-4.1.1.sif file
env SINGULARITYENV_RENV_PATHS_CACHE=/trusted/cache R

This is translated (prefix removed) by singularity so the R process has RENV_PATHS_CACHE=/trusted/cache. However, a child process does not pass on the prefixed value, but the unprefixed 'target' environment variable.

One can imagine renv would do similarly to callr:::setup_context for the renv_* variables and prefix them for the child process.

# test for running in container
if(file.exists("/.singularity.d)") {
  callr::add_hook(renv_singularity = function (options) {
    full_env <- Sys.getenv()
    renv_set <- names(full_env)[grep("^RENV_", names(full_env))]
    sing_set <- paste0("SINGULARITY_", renv_set)
    ...
  })
}

One workaround is to ensure the project .Renviron has the prefixed versions, but this relies on keeping the values in step with the starting of the container.

[gaborcsardi]

@klmr Do you plan to finish this? No rush at all, but also, feel free to close it if you don't need it or want it any more.

[klmr]

@gaborcsardi Ah, I was actually waiting for some feedback, sorry for not making that clear.

In principle I’d be happy if the PR were merged as-is (it certainly fits my specific purpose). But as mentioned it leaks some undocumented implementation details of ‘callr’ into the public API (namely, the options structure).

[gaborcsardi]

Oh, sorry. I'll try look at it soon.

[gaborcsardi]

I think we can add this as is (I'll fix the ...length()), and then we'll see if it needs changes.

[gaborcsardi]

Thanks!

[klmr]

Awesome! Thanks for merging it.