lbartnik / subprocess

Other
49 stars 10 forks source link

Default value of environment for spawn_process #25

Closed johndharrison closed 7 years ago

johndharrison commented 7 years ago

Currently the default value for the environment passed with spawn_process is environment = character(). Should this be changed to

environment = Sys.getenv()

or

environment = Sys.getenv()[!grepl("R_", names(Sys.getenv()))

For example:

is_windows <- function () (tolower(.Platform$OS.type) == "windows")

R_binary <- function () {
  R_exe <- ifelse (is_windows(), "R.exe", "R")
  return(file.path(R.home("bin"), R_exe))
}

handle1 <- spawn_process(R_binary(), c('--no-save'))
handle2 <- spawn_process(R_binary(), c('--no-save'), environment = Sys.getenv())
Sys.sleep(1)
process_write(handle1, 'Sys.getenv()\n')
process_write(handle2, 'Sys.getenv()\n')
out1 <- process_read(handle1)
out2 <- process_read(handle2)

> out1[grepl("^PATH", out1)]
character(0)
> out2[grepl("^PATH", out2)]
[1] "PATH                    /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"

process_kill(handle1)
process_kill(handle2)
lbartnik commented 7 years ago

Thanks for reporting that! However, I'd rather fix it in C - I thought execve would use parent's environment if the argument was empty but it seems one needs to explicitly provide parent's environment. PR #26 should fix this issue.

lbartnik commented 7 years ago

All platforms pass the new test in #26 but please confirm that it solves your issue as well.

johndharrison commented 7 years ago

I have tested on Linux and MAC on an application that was previously failing without passing the parent environment via the environment argument. With the fixes in #26 the application is running with an empty environment argument.