Skip to content
Browse files

Revert "Fix for JRUBY-4908: IO.popen4 returns the wrong pid in Linux"

This reverts commit edadd2b.
  • Loading branch information...
1 parent fda69d3 commit b44b31f22ee42877018cac53271b6fea2b688cd7 @headius headius committed Nov 8, 2010
Showing with 11 additions and 50 deletions.
  1. +2 −2 src/org/jruby/RubyIO.java
  2. +9 −34 src/org/jruby/util/ShellLauncher.java
  3. +0 −14 test/test_io.rb
View
4 src/org/jruby/RubyIO.java
@@ -3490,7 +3490,7 @@ public static IRubyObject popen(ThreadContext context, IRubyObject recv, IRubyOb
}
}
- @JRubyMethod(rest = true, frame = true, meta = true)
+ @JRubyMethod(required = 1, rest = true, frame = true, meta = true)
public static IRubyObject popen3(ThreadContext context, IRubyObject recv, IRubyObject[] args, Block block) {
Ruby runtime = context.getRuntime();
@@ -3516,7 +3516,7 @@ public static IRubyObject popen3(ThreadContext context, IRubyObject recv, IRubyO
}
}
- @JRubyMethod(rest = true, frame = true, meta = true)
+ @JRubyMethod(required = 1, rest = true, frame = true, meta = true)
public static IRubyObject popen4(ThreadContext context, IRubyObject recv, IRubyObject[] args, Block block) {
Ruby runtime = context.getRuntime();
View
43 src/org/jruby/util/ShellLauncher.java
@@ -536,33 +536,18 @@ private static Process popenShared(Ruby runtime, IRubyObject[] strings) throws I
File pwd = new File(runtime.getCurrentDirectory());
try {
- String[] args = parseCommandLine(runtime.getCurrentContext(), runtime, strings);
- boolean useShell = false;
- for (String arg : args) useShell |= shouldUseShell(arg);
-
// CON: popen is a case where I think we should just always shell out.
if (strings.length == 1) {
- if (useShell) {
- // single string command, pass to sh to expand wildcards
- String[] argArray = new String[3];
- argArray[0] = shell;
- argArray[1] = shell.endsWith("sh") ? "-c" : "/c";
- argArray[2] = strings[0].asJavaString();
- childProcess = Runtime.getRuntime().exec(argArray, getCurrentEnv(runtime), pwd);
- } else {
- childProcess = Runtime.getRuntime().exec(args, getCurrentEnv(runtime), pwd);
- }
+ // single string command, pass to sh to expand wildcards
+ String[] argArray = new String[3];
+ argArray[0] = shell;
+ argArray[1] = shell.endsWith("sh") ? "-c" : "/c";
+ argArray[2] = strings[0].asJavaString();
+ childProcess = Runtime.getRuntime().exec(argArray, getCurrentEnv(runtime), pwd);
} else {
- if (useShell) {
- String[] argArray = new String[args.length + 2];
- argArray[0] = shell;
- argArray[1] = shell.endsWith("sh") ? "-c" : "/c";
- System.arraycopy(args, 0, argArray, 2, args.length);
- childProcess = Runtime.getRuntime().exec(argArray, getCurrentEnv(runtime), pwd);
- } else {
- // direct invocation of the command
- childProcess = Runtime.getRuntime().exec(args, getCurrentEnv(runtime), pwd);
- }
+ // direct invocation of the command
+ String[] args = parseCommandLine(runtime.getCurrentContext(), runtime, strings);
+ childProcess = Runtime.getRuntime().exec(args, getCurrentEnv(runtime), pwd);
}
} catch (SecurityException se) {
throw runtime.newSecurityError(se.getLocalizedMessage());
@@ -1313,16 +1298,6 @@ private static String getShell(Ruby runtime) {
return RbConfigLibrary.jrubyShell();
}
- private static boolean shouldUseShell(String command) {
- boolean useShell = false;
- for (char c : command.toCharArray()) {
- if (c != ' ' && !Character.isLetter(c) && "*?{}[]<>()~&|\\$;'`\"\n".indexOf(c) != -1) {
- useShell = true;
- }
- }
- return useShell;
- }
-
static void log(Ruby runtime, String msg) {
if (RubyInstanceConfig.DEBUG_LAUNCHING) {
runtime.getErr().println("ShellLauncher: " + msg);
View
14 test/test_io.rb
@@ -473,18 +473,4 @@ def test_clear_dollar_bang_after_open_block
def ensure_files(*files)
files.each {|f| File.open(f, "w") {|g| g << " " } }
end
- private :ensure_files
-
- # JRUBY-4908
- unless WINDOWS
- def test_sh_used_appropriately
- # should not use sh
- p, o, i, e = IO.popen4("/bin/ps -a")
- assert_match p.to_s, i.read.lines.grep(/\/bin\/ps/).first
-
- # should use sh
- p, o, i, e = IO.popen4("/bin/ps -a | grep ps'")
- assert_no_match Regexp.new(p.to_s), i.read.grep(/\/bin\/ps/).first
- end
- end
end

0 comments on commit b44b31f

Please sign in to comment.
Something went wrong with that request. Please try again.