JRuby

IO.popen dosn't support forward slashes in Windows (diffrent behaviour to MRI)

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: JRuby 1.1.5
  • Fix Version/s: None
  • Component/s: Windows
  • Labels:
    None
  • Environment:
    Windows XP
  • Number of attachments :
    3

Description

Jruby produces error's When trying to launch a executable that includes forward slashes in the path. This is different behavior than MRI were you both methods work

irb(main):001:0> IO.popen("./temp/test.exe")
=> #<IO:0x1afd9cc>
irb(main):002:0> '.' is not recognized as an internal or external command,
operable program or batch file.

However back slashes work.

irb(main):004:0> IO.popen(".\\temp
test.exe")
=> #<IO:0x145e5a6>

  1. Text File
    jruby-3237.patch
    03/Mar/09 3:43 PM
    1 kB
    Zac Brown
  2. File
    test.exe
    03/Mar/09 3:43 PM
    28 kB
    Zac Brown
  3. File
    test-popen.rb
    03/Mar/09 3:43 PM
    0.2 kB
    Zac Brown

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Charles Oliver Nutter added a comment -

Won't be in 1.1.6, but perhaps we can fix for 1.1.7? Help coming up with tests and patches, please!

Show
Charles Oliver Nutter added a comment - Won't be in 1.1.6, but perhaps we can fix for 1.1.7? Help coming up with tests and patches, please!
Hide
Permalink
Thomas E Enebo added a comment -

No patches or specs...bumping...

Show
Thomas E Enebo added a comment - No patches or specs...bumping...
Hide
Permalink
Zac Brown added a comment -

It looks like this has to do with popen executing CLI commands. Just typing something like './temp/test.exe' isn't valid as basic Windows CMD syntax.

My guess for how MRI gets past this is by forking a process that executes the binary listed so it can just bypass the CLI.

I'm going to dig into this a bit more.

Show
Zac Brown added a comment - It looks like this has to do with popen executing CLI commands. Just typing something like './temp/test.exe' isn't valid as basic Windows CMD syntax. My guess for how MRI gets past this is by forking a process that executes the binary listed so it can just bypass the CLI. I'm going to dig into this a bit more.
Hide
Permalink
Zac Brown added a comment -

This patch seems to be about the simplest way to solve this issue. By swapping '/' for '
' in the command string on the Windows platform, it makes life easier rather than trying to manipulate it in some other ugly fashion.

I'm not sure if the manner in which I determined what platform JRuby is running is kosher or not but it works when running the 'same' script on OS X, Linux and Windows. A suggestion I encountered but didn't follow up on was using 'System.getProperty("os.name")'. However, org.jruby.platform.Platform seems to make sense at this point. I'm not sure why else that class would exist.

Show
Zac Brown added a comment - This patch seems to be about the simplest way to solve this issue. By swapping '/' for ' ' in the command string on the Windows platform, it makes life easier rather than trying to manipulate it in some other ugly fashion. I'm not sure if the manner in which I determined what platform JRuby is running is kosher or not but it works when running the 'same' script on OS X, Linux and Windows. A suggestion I encountered but didn't follow up on was using 'System.getProperty("os.name")'. However, org.jruby.platform.Platform seems to make sense at this point. I'm not sure why else that class would exist.
Hide
Permalink
Zac Brown added a comment -

I should note that the test suite still passes with this patch.

Show
Zac Brown added a comment - I should note that the test suite still passes with this patch.
Hide
Permalink
Zac Brown added a comment -

Appears this is an unsuitable solution since it doesn't cover the case of doing something like:

IO.popen("ipconfig /all")

since the fix above would change that to:

IO.popen("ipconfig \all")

I'll continue messing with this.

Show
Zac Brown added a comment - Appears this is an unsuitable solution since it doesn't cover the case of doing something like: IO.popen("ipconfig /all") since the fix above would change that to: IO.popen("ipconfig \all") I'll continue messing with this.
Hide
Permalink
Zac Brown added a comment -

Updated patch. Will only switch the forward slashes to backslashes in the path to the command. Since Windows goofily uses forward slashes as switches for command line tools, this was necessary to get things like 'IO.popen("ipconfig /all")' working properly.

Show
Zac Brown added a comment - Updated patch. Will only switch the forward slashes to backslashes in the path to the command. Since Windows goofily uses forward slashes as switches for command line tools, this was necessary to get things like 'IO.popen("ipconfig /all")' working properly.
Hide
Permalink
Zac Brown added a comment -

Updated to match JRuby style code. Also added test-popen.rb script for testing on Windows and the test.exe program. Source to test.exe is as follows:

#include <stdio.h>
void main(void)

{ printf("Hello World!\n"); }
Show
Zac Brown added a comment - Updated to match JRuby style code. Also added test-popen.rb script for testing on Windows and the test.exe program. Source to test.exe is as follows: #include <stdio.h> void main(void) { printf("Hello World!\n"); }
Hide
Permalink
Charles Oliver Nutter added a comment -

I committed the fix in r9347, but still need to integrate the test.

Show
Charles Oliver Nutter added a comment - I committed the fix in r9347, but still need to integrate the test.
Hide
Permalink
Hiro Asari added a comment -

Do we have a test for this?

Show
Hiro Asari added a comment - Do we have a test for this?
Hide
Permalink
Thomas E Enebo added a comment -

This has been fixed for ages and I am sure there is forward slash popen tests somewhere now....resolving.

Show
Thomas E Enebo added a comment - This has been fixed for ages and I am sure there is forward slash popen tests somewhere now....resolving.

People

  • Assignee:
    Thomas E Enebo
    Reporter:
    Mikael Rudberg
Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: