Always use /bin/sh in system as per GNU R; get visibility correct

com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/system/ProcessSystemFunctionFactory.java

@@ -44,11 +44,9 @@ public class ProcessSystemFunctionFactory extends SystemFunctionFactory {
     @TruffleBoundary
     private Object execute(String command, boolean intern) {
         Object result;
-        String shell = RContext.getInstance().stateREnvVars.get("SHELL");
-        if (shell == null) {
-            shell = "/bin/sh";
-        }
-        log(String.format("%s -c %s", shell, command), "Process");
+        // GNU R uses popen which always invokes /bin/sh
+        String shell = "/bin/sh";
+        log(String.format("%s -c \"%s\"", shell, command), "Process");
         ProcessBuilder pb = new ProcessBuilder(shell, "-c", command);
         updateEnvironment(pb);
         pb.redirectInput(Redirect.INHERIT);
@@ -91,6 +89,13 @@ public class ProcessSystemFunctionFactory extends SystemFunctionFactory {
         return result;
     }
 
+    /**
+     * Any environment variables that have been added to this session must be forwarded to the
+     * process (Java does not provide a {@code setenv} call, so {@code Sys.setenv} calls only affect
+     * {@code stateEnvVars}. Any explicit settings in the command call (arising from the {@code env}
+     * argument to the {@code system2} call, will override these by virtue of being explicitly set
+     * in the new shell.
+     */
     private static void updateEnvironment(ProcessBuilder pb) {
         Map<String, String> pEnv = pb.environment();
         Map<String, String> rEnv = RContext.getInstance().stateREnvVars.getMap();

com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/system/SystemFunction.java

@@ -24,7 +24,7 @@ package com.oracle.truffle.r.nodes.builtin.base.system;
 
 import static com.oracle.truffle.r.nodes.builtin.CastBuilder.Predef.stringValue;
 import static com.oracle.truffle.r.nodes.builtin.CastBuilder.Predef.toBoolean;
-import static com.oracle.truffle.r.runtime.RVisibility.OFF;
+import static com.oracle.truffle.r.runtime.RVisibility.CUSTOM;
 import static com.oracle.truffle.r.runtime.builtins.RBehavior.COMPLEX;
 import static com.oracle.truffle.r.runtime.builtins.RBuiltinKind.INTERNAL;
 
@@ -32,11 +32,14 @@ import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.r.nodes.builtin.CastBuilder;
 import com.oracle.truffle.r.nodes.builtin.RBuiltinNode;
+import com.oracle.truffle.r.nodes.function.visibility.SetVisibilityNode;
 import com.oracle.truffle.r.runtime.RError;
 import com.oracle.truffle.r.runtime.builtins.RBuiltin;
 
-@RBuiltin(name = "system", visibility = OFF, kind = INTERNAL, parameterNames = {"command", "intern"}, behavior = COMPLEX)
+@RBuiltin(name = "system", visibility = CUSTOM, kind = INTERNAL, parameterNames = {"command", "intern"}, behavior = COMPLEX)
 public abstract class SystemFunction extends RBuiltinNode {
+    @Child private SetVisibilityNode visibility = SetVisibilityNode.create();
+
     @Override
     protected void createCasts(CastBuilder casts) {
         casts.arg("command").mustBe(stringValue(), RError.Message.SYSTEM_CHAR_ARG).asStringVector().findFirst();
@@ -45,7 +48,9 @@ public abstract class SystemFunction extends RBuiltinNode {
 
     @Specialization
     protected Object system(VirtualFrame frame, String command, boolean intern) {
-        return SystemFunctionFactory.getInstance().execute(frame, command.trim(), intern);
+        Object result = SystemFunctionFactory.getInstance().execute(frame, command.trim(), intern);
+        visibility.execute(frame, intern);
+        return result;
     }
 
 }