From 2bfa2f3a6fb41bfaa21bab8a50c35402403587b5 Mon Sep 17 00:00:00 2001 From: Mick Jordan <mick.jordan@oracle.com> Date: Wed, 14 Sep 2016 16:23:13 -0700 Subject: [PATCH] Always use /bin/sh in system as per GNU R; get visibility correct --- .../base/system/ProcessSystemFunctionFactory.java | 15 ++++++++++----- .../nodes/builtin/base/system/SystemFunction.java | 11 ++++++++--- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/system/ProcessSystemFunctionFactory.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/system/ProcessSystemFunctionFactory.java index d0812bf2d2..0b503f6864 100644 --- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/system/ProcessSystemFunctionFactory.java +++ b/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(); diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/system/SystemFunction.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/system/SystemFunction.java index 237680183c..5ac50544b4 100644 --- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/base/system/SystemFunction.java +++ b/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; } } -- GitLab