Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 7f28c6785efa
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 7d240aa18018
Choose a head ref
  • 4 commits
  • 3 files changed
  • 1 contributor

Commits on Apr 21, 2015

  1. Always do this check; does not appear to break test_pid anymore. 

    This should help bring popen behavior in line with MRI per
rails/rails#19777.
    @headius
    headius committed Apr 21, 2015
    Copy the full SHA
    821bb77 View commit details

  2. Better argument processing. 

    * Peel off leading env hash
* Peel off options
* Process coercions correctly

Helps get the case from rails/rails#19777 running right.
    @headius
    headius committed Apr 21, 2015
    Copy the full SHA
    30cd6e8 View commit details

  3. Derp.

    @headius
    headius committed Apr 21, 2015
    Copy the full SHA
    5c93efa View commit details

  4. Minor reformat.

    @headius
    headius committed Apr 21, 2015
    Copy the full SHA
    7d240aa View commit details
Showing with 67 additions and 62 deletions.
  1. +65 −60 core/src/main/java/org/jruby/RubyIO.java
  2. +1 −1 core/src/main/java/org/jruby/util/ShellLauncher.java
  3. +1 −1 core/src/main/java/org/jruby/util/TypeConverter.java
125 changes: 65 additions & 60 deletions core/src/main/java/org/jruby/RubyIO.java
View file
Original file line number Diff line number Diff line change
@@ -2802,7 +2802,7 @@ public IRubyObject ungetc(IRubyObject number) {
throw getRuntime().newIOError("unread stream");
}

ungetcCommon((int)number.convertToInteger().getLongValue());
ungetcCommon((int) number.convertToInteger().getLongValue());

return getRuntime().getNil();
}
@@ -3798,7 +3798,7 @@ private static IRubyObject openKeyArgs(ThreadContext context, IRubyObject recv,
IRubyObject path, v;

path = StringSupport.checkEmbeddedNulls(runtime, RubyFile.get_path(context, argv[0]));
failIfDirectory(runtime, (RubyString)path); // only in JRuby
failIfDirectory(runtime, (RubyString) path); // only in JRuby
// MRI increments args past 0 now, so remaining uses of args only see non-path args

if (opt.isNil()) {
@@ -4099,13 +4099,24 @@ public static IRubyObject popen(ThreadContext context, IRubyObject recv, IRubyOb
Ruby runtime = context.runtime;

IRubyObject cmdObj;
int firstArg = 0;
int argc = args.length;

IRubyObject envHash = null;

if (argc > 0 && !(envHash = TypeConverter.checkHashType(runtime, args[0])).isNil()) {
if (argc < 2) throw runtime.newArgumentError(1, 2);
firstArg++;
argc--;
}

if (Platform.IS_WINDOWS) {
String[] tokens = args[0].convertToString().toString().split(" ", 2);
String[] tokens = args[firstArg].convertToString().toString().split(" ", 2);
String commandString = tokens[0].replace('/', '\\') +
(tokens.length > 1 ? ' ' + tokens[1] : "");
cmdObj = runtime.newString(commandString);
} else {
cmdObj = args[0].convertToString();
cmdObj = args[firstArg].convertToString();
}

if ("-".equals(cmdObj.toString())) {
@@ -4114,15 +4125,15 @@ public static IRubyObject popen(ThreadContext context, IRubyObject recv, IRubyOb

try {
IOOptions ioOptions;
if (args.length == 1) {
if (argc == 1) {
ioOptions = newIOOptions(runtime, ModeFlags.RDONLY);
} else if (args[1] instanceof RubyFixnum) {
ioOptions = newIOOptions(runtime, RubyFixnum.num2int(args[1]));
ioOptions = newIOOptions(runtime, RubyFixnum.num2int(args[firstArg + 1]));
} else {
ioOptions = newIOOptions(runtime, args[1].convertToString().toString());
ioOptions = newIOOptions(runtime, args[firstArg + 1].convertToString().toString());
}

ShellLauncher.POpenProcess process = ShellLauncher.popen(runtime, cmdObj, ioOptions.getModeFlags());
ShellLauncher.POpenProcess process = ShellLauncher.popen(runtime, new IRubyObject[]{cmdObj}, (RubyHash)envHash, ioOptions.getModeFlags());

// Yes, this is gross. java.lang.Process does not appear to be guaranteed
// "ready" when we get it back from Runtime#exec, so we try to give it a
@@ -4207,61 +4218,53 @@ private static class Ruby19POpen {

public Ruby19POpen(Ruby runtime, IRubyObject[] args) {
IRubyObject[] _cmdPlusArgs = null;
RubyHash _env = null;
IRubyObject _env = null;
IRubyObject _cmd;
IRubyObject arg0 = args[0].checkArrayType();

if (args[0] instanceof RubyHash) {
// use leading hash as env
if (args.length > 1) {
_env = (RubyHash)args[0];
} else {
Arity.raiseArgumentError(runtime, 0, 1, 2);
}
int firstArg = 0;
int argc = args.length;

if (Platform.IS_WINDOWS) {
String[] tokens = args[1].convertToString().toString().split(" ", 2);
String commandString = tokens[0].replace('/', '\\') +
(tokens.length > 1 ? ' ' + tokens[1] : "");
_cmd = runtime.newString(commandString);
} else {
_cmd = args[1].convertToString();
if (argc > 0 && !(_env = TypeConverter.checkHashType(runtime, args[0])).isNil()) {
if (argc < 2) throw runtime.newArgumentError(1, 2);
firstArg++;
argc--;
} else {
_env = null;
}

IRubyObject arg0 = args[firstArg].checkArrayType();

if (arg0.isNil()) {
if ((arg0 = TypeConverter.checkStringType(runtime, args[firstArg])).isNil()) {
throw runtime.newTypeError(args[firstArg], runtime.getString());
}
} else if (args[0] instanceof RubyArray) {
RubyArray arg0Ary = (RubyArray)arg0;
_cmdPlusArgs = new IRubyObject[]{arg0};
} else {
RubyArray arg0Ary = (RubyArray) arg0;
if (arg0Ary.isEmpty()) throw runtime.newArgumentError("wrong number of arguments");
if (arg0Ary.eltOk(0) instanceof RubyHash) {
// leading hash, use for env
_env = (RubyHash)arg0Ary.delete_at(0);
_env = arg0Ary.delete_at(0);
}
if (arg0Ary.isEmpty()) throw runtime.newArgumentError("wrong number of arguments");
if (arg0Ary.size() > 1 && arg0Ary.eltOk(arg0Ary.size() - 1) instanceof RubyHash) {
// trailing hash, use for opts
_env = (RubyHash)arg0Ary.eltOk(arg0Ary.size() - 1);
_env = arg0Ary.eltOk(arg0Ary.size() - 1);
}
_cmdPlusArgs = (IRubyObject[])arg0Ary.toJavaArray();
_cmdPlusArgs = arg0Ary.toJavaArray();
}

if (Platform.IS_WINDOWS) {
String commandString = _cmdPlusArgs[0].convertToString().toString().replace('/', '\\');
_cmdPlusArgs[0] = runtime.newString(commandString);
} else {
_cmdPlusArgs[0] = _cmdPlusArgs[0].convertToString();
}
_cmd = _cmdPlusArgs[0];
if (Platform.IS_WINDOWS) {
String commandString = _cmdPlusArgs[0].convertToString().toString().replace('/', '\\');
_cmdPlusArgs[0] = runtime.newString(commandString);
} else {
if (Platform.IS_WINDOWS) {
String[] tokens = args[0].convertToString().toString().split(" ", 2);
String commandString = tokens[0].replace('/', '\\') +
(tokens.length > 1 ? ' ' + tokens[1] : "");
_cmd = runtime.newString(commandString);
} else {
_cmd = args[0].convertToString();
}
_cmdPlusArgs[0] = _cmdPlusArgs[0].convertToString();
}
_cmd = _cmdPlusArgs[0];

this.cmd = (RubyString)_cmd;
this.cmdPlusArgs = _cmdPlusArgs;
this.env = _env;
this.env = (RubyHash)_env;
}
}

@@ -4271,21 +4274,23 @@ public static IRubyObject popen19(ThreadContext context, IRubyObject recv, IRuby

IRubyObject pmode = null;
RubyHash options = null;

switch(args.length) {
case 1:
break;
case 2:
if (args[1] instanceof RubyHash) {
options = (RubyHash) args[1];
} else {
pmode = args[1];
}
break;
case 3:
options = args[2].convertToHash();
pmode = args[1];
break;
IRubyObject tmp;

int firstArg = 0;
int argc = args.length;

if (argc > 0 && !TypeConverter.checkHashType(runtime, args[0]).isNil()) {
firstArg++;
argc--;
}

if (argc > 0 && !(tmp = TypeConverter.checkHashType(runtime, args[args.length - 1])).isNil()) {
options = (RubyHash)tmp;
argc--;
}

if (argc > 1) {
pmode = args[firstArg + 1];
}

RubyIO io = new RubyIO(runtime, (RubyClass) recv);
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/util/ShellLauncher.java
View file
Original file line number Diff line number Diff line change
@@ -749,7 +749,7 @@ private static Process popenShared(Ruby runtime, IRubyObject[] strings, Map env,

String[] args = parseCommandLine(runtime.getCurrentContext(), runtime, strings);
LaunchConfig lc = new LaunchConfig(runtime, strings, false);
boolean useShell = Platform.IS_WINDOWS ? lc.shouldRunInShell() : false;
boolean useShell = lc.shouldRunInShell();
if (addShell) for (String arg : args) useShell |= shouldUseShell(arg);

// CON: popen is a case where I think we should just always shell out.
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/util/TypeConverter.java
View file
Original file line number Diff line number Diff line change
@@ -281,7 +281,7 @@ public static IRubyObject checkHashType(Ruby runtime, IRubyObject obj) {

// rb_check_string_type
public static IRubyObject checkStringType(Ruby runtime, IRubyObject obj) {
return TypeConverter.convertToTypeWithCheck(obj, runtime.getHash(), "to_str");
return TypeConverter.convertToTypeWithCheck(obj, runtime.getString(), "to_str");
}

// MRI: rb_check_array_type