JRuby

allow "/" as absolute path in Windows

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: JRuby 0.9.1, JRuby 0.9.2, JRuby 0.9.8, JRuby 0.9.9, JRuby 1.0.0RC1, JRuby 1.0.0RC2
  • Fix Version/s: JRuby 1.1RC3
  • Component/s: None
  • Labels:
    None
  • Environment:
    Windows/XP
  • Testcase included:
    yes
  • Number of attachments :
    3

Description

def test_chdir
wd = Dir.getwd
new_dir = "/"
assert_not_equal(wd, new_dir)
Dir.chdir(new_dir)
wd = Dir.getwd
assert_equal(wd, new_dir) # fails
end

Tom Enebo suggested this is caused by Java File.isAbsolute returning false.

The following is from http://java.sun.com/j2se/1.5.0/docs/api/java/io/File.html#isAbsolute()

On UNIX systems, a pathname is absolute if its prefix is "/". On Microsoft Windows systems, a pathname is absolute if its prefix is a drive specifier followed by "
", or if its prefix is "\\\\".

However, in non-Jruby Ruby, "/" works fine as an alias to e.g. C:\

Maybe if isAbsolute returns false, use what getAbsolutePath returns:

require 'java'
include_class('java.io.File') { |p, name| "J" + name }
myfile = JFile.new('/crap.txt')
exists = myfile.exists
is_file = myfile.is_file
is_absolute = myfile.is_absolute
absolute_path = myfile.get_absolute_path
puts "exists=#{exists}"
puts "is_file=#{is_file}"
puts "is_absolute=#{is_absolute}"
puts "absolute_path=#{absolute_path}"

      1. exists=true
        is_file=true
        is_absolute=false
        absolute_path=C:\crap.txt

  1. jrubyfile.patch
    23/May/07 10:09 AM
    0.8 kB
    Koichiro Ohba
  2. RubyIO.patch
    12/Jan/08 3:48 PM
    1 kB
    Tiziano Merzi
  3. test.rb
    03/Apr/07 9:07 PM
    0.2 kB
    Charles Oliver Nutter

Activity

Hide
Charles Oliver Nutter added a comment -

As convenient as it is, I might argue that Ruby is wrong here. Having / map to C: is convenient, but fails to accomodate additional drives. It would also fail mightily on my home desktop, where Windows XP is installed with D: as the root drive.

Show
Charles Oliver Nutter added a comment - As convenient as it is, I might argue that Ruby is wrong here. Having / map to C: is convenient, but fails to accomodate additional drives. It would also fail mightily on my home desktop, where Windows XP is installed with D: as the root drive.
Hide
John Muth added a comment -

Fair enough.
I thought it worth raising because the behavior differed from Ruby.
(Just a side-note: surely Ruby doesn't really map "/" to "C:\", but to whatever drive 'getwd' is?)
However!
Would you agree that "/" should not be translated to cwd silently? It was trouble to track down that that was where my working-happily-under-Ruby script was going wrong.

Show
John Muth added a comment - Fair enough. I thought it worth raising because the behavior differed from Ruby. (Just a side-note: surely Ruby doesn't really map "/" to "C:\", but to whatever drive 'getwd' is?) However! Would you agree that "/" should not be translated to cwd silently? It was trouble to track down that that was where my working-happily-under-Ruby script was going wrong.
Hide
Charles Oliver Nutter added a comment -

Attached test case based on original comment; marked to fix for 0.9.9.

Show
Charles Oliver Nutter added a comment - Attached test case based on original comment; marked to fix for 0.9.9.
Hide
Charles Oliver Nutter added a comment -

Bumping to 1.0...no progress in 099 timeframe.

Show
Charles Oliver Nutter added a comment - Bumping to 1.0...no progress in 099 timeframe.
Hide
Charles Oliver Nutter added a comment -

Someone with regular access to a Windows box needs to look into this. Would be nice to get in for 1.0, but it's not going to be in RC.

Show
Charles Oliver Nutter added a comment - Someone with regular access to a Windows box needs to look into this. Would be nice to get in for 1.0, but it's not going to be in RC.
Hide
Charles Oliver Nutter added a comment -

Bumping again...and in danger of falling off 1.0.

Show
Charles Oliver Nutter added a comment - Bumping again...and in danger of falling off 1.0.
Hide
Ray Krueger added a comment -

The test for this issue is somewhat invalid.

Dir.getwd returns a string, and on Windows that String is "C:\".
So of course assert_equal(wd, new_dir) fails, "C:\" does not equal "/"

The test case fails on Windows with Ruby 1.8.6

I tried changing it to assert_equal(Dir.new(wd), Dir.new(new_dir)), but it seems that Dir does not implement == correctly as it says...

test_chdir(TestSomething) [test.rb:10]:
<#<Dir:0x2ea11c0>> expected but was
<#<Dir:0x2ea1198>>.

The proper thing to do would be to test that C:\ and / are the same physical location, though I'm too much of a ruby newby to get that to work it seems...

Show
Ray Krueger added a comment - The test for this issue is somewhat invalid. Dir.getwd returns a string, and on Windows that String is "C:\". So of course assert_equal(wd, new_dir) fails, "C:\" does not equal "/" The test case fails on Windows with Ruby 1.8.6 I tried changing it to assert_equal(Dir.new(wd), Dir.new(new_dir)), but it seems that Dir does not implement == correctly as it says...
test_chdir(TestSomething) [test.rb:10]:
<#<Dir:0x2ea11c0>> expected but was
<#<Dir:0x2ea1198>>.
The proper thing to do would be to test that C:\ and / are the same physical location, though I'm too much of a ruby newby to get that to work it seems...
Hide
Ray Krueger added a comment -

To confirm John's point above Dir.cwd("/") on Windows changes you to the root of the 'current' drive.

I came up with the following idea, it passes on Windows and Linux...

Unable to find source-code formatter for language: ruby. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
require 'test/unit'

class TestSomething < Test::Unit::TestCase
  def test_chdir
    wd = Dir.getwd
    new_dir = "/"
    assert_not_equal(wd, new_dir)

    Dir.chdir(new_dir)
    wd = Dir.getwd
    puts wd

    slash_entries = Dir.entries(new_dir).sort
    root_entries = Dir.entries(wd).sort
    
    assert_equal(slash_entries, root_entries)

    #assert_equal(wd, new_dir) # fails

    end
end
Show
Ray Krueger added a comment - To confirm John's point above Dir.cwd("/") on Windows changes you to the root of the 'current' drive. I came up with the following idea, it passes on Windows and Linux...
Unable to find source-code formatter for language: ruby. Available languages are: javascript, sql, xhtml, actionscript, none, html, xml, java
require 'test/unit'

class TestSomething < Test::Unit::TestCase
  def test_chdir
    wd = Dir.getwd
    new_dir = "/"
    assert_not_equal(wd, new_dir)

    Dir.chdir(new_dir)
    wd = Dir.getwd
    puts wd

    slash_entries = Dir.entries(new_dir).sort
    root_entries = Dir.entries(wd).sort
    
    assert_equal(slash_entries, root_entries)

    #assert_equal(wd, new_dir) # fails

    end
end
Hide
Koichiro Ohba added a comment -

JVM on Windows recognizes only

 "\\\\" and "X:\\"
as the absolute root path.

I think any other expression (such as '/') should be canonicalized
before the check if it is an absolute path or not.

How about this patch? I tried Ray's test case and it was successful.

Show
Koichiro Ohba added a comment - JVM on Windows recognizes only
 "\\\\" and "X:\\"
as the absolute root path. I think any other expression (such as '/') should be canonicalized before the check if it is an absolute path or not. How about this patch? I tried Ray's test case and it was successful.
Hide
Ray Krueger added a comment -

I'm not sure what this bug is about really...

On Windows / or
is a relative path. It is relative to the drive you are on. The only absolute path is C:/ or C:

For example, in straight java...

System.out.println(new File("\\").isAbsolute());  //false
        System.out.println(new File("/").isAbsolute());   //false
        
        System.out.println(new File("C:\\").isAbsolute());   //true
        System.out.println(new File("C:/").isAbsolute());    //true

Unless I'm missing something, I think this bug is a non-issue.

Show
Ray Krueger added a comment - I'm not sure what this bug is about really... On Windows / or
is a relative path. It is relative to the drive you are on. The only absolute path is C:/ or C:
For example, in straight java...
System.out.println(new File("\\").isAbsolute());  //false
        System.out.println(new File("/").isAbsolute());   //false
        
        System.out.println(new File("C:\\").isAbsolute());   //true
        System.out.println(new File("C:/").isAbsolute());    //true
Unless I'm missing something, I think this bug is a non-issue.
Hide
Ray Krueger added a comment -

The above should read...

In java on Windows / or \\ is a relative path. It is relative to the drive you are on. The only absolute path is C:/ or C:\\
Show
Ray Krueger added a comment - The above should read...
In java on Windows / or \\ is a relative path. It is relative to the drive you are on. The only absolute path is C:/ or C:\\
Hide
Daniel Berger added a comment -

This is not actually an MRI issue per se, but rather it's how the MSVCRT _chdir() function works out of the box, and Dir.chdir just uses your systems chdir function. MS must have added it for portability reasons. Here's proof:

require 'Win32API'

Chdir = Win32API.new('msvcrt', '_chdir', 'P', 'I')
Getcwd = Win32API.new('msvcrt', '_getcwd', 'PI', 'P')

def chdir(dir)
Chdir.call(dir)
end

def getcwd(buf=0.chr * 256, len=buf.length)
Getcwd.call(buf, len)
end

p getcwd # => C:
Documents and Settings
djberge
chdir("/") # => Yep, works
p getcwd # => C:

Show
Daniel Berger added a comment - This is not actually an MRI issue per se, but rather it's how the MSVCRT _chdir() function works out of the box, and Dir.chdir just uses your systems chdir function. MS must have added it for portability reasons. Here's proof: require 'Win32API' Chdir = Win32API.new('msvcrt', '_chdir', 'P', 'I') Getcwd = Win32API.new('msvcrt', '_getcwd', 'PI', 'P') def chdir(dir) Chdir.call(dir) end def getcwd(buf=0.chr * 256, len=buf.length) Getcwd.call(buf, len) end p getcwd # => C:
Documents and Settings
djberge chdir("/") # => Yep, works p getcwd # => C:
Hide
Daniel Berger added a comment -

Let's try that again:

p getcwd    # => C:\\Documents and Settings\\djberge
chdir("/")  # => Yep, works
p getcwd    # => C:\\
Show
Daniel Berger added a comment - Let's try that again:
p getcwd    # => C:\\Documents and Settings\\djberge
chdir("/")  # => Yep, works
p getcwd    # => C:\\
Hide
Daniel Berger added a comment -

Further research indicates that the SetCurrentDirectory() function from kernel32 behaves the same way, i.e. understands "/" to mean root of the current path.

Show
Daniel Berger added a comment - Further research indicates that the SetCurrentDirectory() function from kernel32 behaves the same way, i.e. understands "/" to mean root of the current path.
Hide
Charles Oliver Nutter added a comment -

No good solution provided for this yet, so it's going to be post 1.0. If something great comes up, it would be a nice 1.0.1 fix.

Show
Charles Oliver Nutter added a comment - No good solution provided for this yet, so it's going to be post 1.0. If something great comes up, it would be a nice 1.0.1 fix.
Hide
Charles Oliver Nutter added a comment -

Are we any closer to understanding this issue and having a fix? Try to have it for 1.1.

Show
Charles Oliver Nutter added a comment - Are we any closer to understanding this issue and having a fix? Try to have it for 1.1.
Hide
Charles Oliver Nutter added a comment -

Assigning Windows-related bug to Nick.

Show
Charles Oliver Nutter added a comment - Assigning Windows-related bug to Nick.
Hide
Daniel Berger added a comment -

I'm still of the opinion that we should defer to the MSVCRT on this issue, which is to change to the root directory of the current drive (not necessarily the C: drive).

irb(main):001:0> Dir.pwd
=> "H:/My Documents"
irb(main):002:0> Dir.chdir('/')
=> 0
irb(main):003:0> Dir.pwd
=> "H:/"
Show
Daniel Berger added a comment - I'm still of the opinion that we should defer to the MSVCRT on this issue, which is to change to the root directory of the current drive (not necessarily the C: drive).
irb(main):001:0> Dir.pwd
=> "H:/My Documents"
irb(main):002:0> Dir.chdir('/')
=> 0
irb(main):003:0> Dir.pwd
=> "H:/"
Hide
Ray Krueger added a comment -

I agree, and still argue that this bug is invalid.

The path "/" is relative on Windows; it is relative to the "current" drive. That's exactly what Daniel's example shows above.

Show
Ray Krueger added a comment - I agree, and still argue that this bug is invalid. The path "/" is relative on Windows; it is relative to the "current" drive. That's exactly what Daniel's example shows above.
Hide
Charles Oliver Nutter added a comment -

Ray and Daniel: so does Koichiro's patch and the provided test case above it seem like the best way to go for this?

Show
Charles Oliver Nutter added a comment - Ray and Daniel: so does Koichiro's patch and the provided test case above it seem like the best way to go for this?
Hide
Ray Krueger added a comment -

I'm not sure. It depends on the use case we're trying to meet here (which still isn't clear to me ).
The Jvm and "java.io.File" do not canonicalize the "/". Canonicalizing the "/" to "C:/" is different than how the JVM behaves.

Show
Ray Krueger added a comment - I'm not sure. It depends on the use case we're trying to meet here (which still isn't clear to me ). The Jvm and "java.io.File" do not canonicalize the "/". Canonicalizing the "/" to "C:/" is different than how the JVM behaves.
Hide
Charles Oliver Nutter added a comment -

We do many things different from Java when you're in Ruby. I think making it act like Ruby is the priority here.

Show
Charles Oliver Nutter added a comment - We do many things different from Java when you're in Ruby. I think making it act like Ruby is the priority here.
Hide
Tiziano Merzi added a comment -

Hi,
I think that you must fix.

in jruby
Dir.entries("/") => like Dir.entries(".")

But in Java and Matz ruby

(new File.("/")).listFiles()
Dir.entries("/")
=> files of / of current drive

There isn't single root in windows.

I send a patch JRubuFile.patch

Show
Tiziano Merzi added a comment - Hi, I think that you must fix. in jruby Dir.entries("/") => like Dir.entries(".") But in Java and Matz ruby (new File.("/")).listFiles() Dir.entries("/") => files of / of current drive There isn't single root in windows. I send a patch JRubuFile.patch
Hide
Charles Oliver Nutter added a comment -

Punting issues from 1.1 RC2 to 1.1 final.

Show
Charles Oliver Nutter added a comment - Punting issues from 1.1 RC2 to 1.1 final.
Hide
Vladimir Sizikov added a comment -

Tiziano, it seems that you've attached the wrong path, unrelated to this issue.

Show
Vladimir Sizikov added a comment - Tiziano, it seems that you've attached the wrong path, unrelated to this issue.
Hide
Vladimir Sizikov added a comment - - edited

OK, it seems we more or less have an agreement what behavior should be.
Basically, JRuby should mimick MRI behavior as much as possible, if we are to say that JRuby is compatible implementation of Ruby.

Having said that,

 '/' or '\\' 
should be treated as RELATIVE paths, relative to current DRIVE. That's how MRI treats them.

Fix is in progress.

Show
Vladimir Sizikov added a comment - - edited OK, it seems we more or less have an agreement what behavior should be. Basically, JRuby should mimick MRI behavior as much as possible, if we are to say that JRuby is compatible implementation of Ruby. Having said that,
 '/' or '\\' 
should be treated as RELATIVE paths, relative to current DRIVE. That's how MRI treats them. Fix is in progress.
Hide
Vladimir Sizikov added a comment -

Fixed in rev. r6007 on trunk.

The following methods corrected: chdir, new, open, entries, glob.
For all of them regression tests were added.

If you see that some other methods need to be corrected as well,
please open new issue.

Show
Vladimir Sizikov added a comment - Fixed in rev. r6007 on trunk. The following methods corrected: chdir, new, open, entries, glob. For all of them regression tests were added. If you see that some other methods need to be corrected as well, please open new issue.
Hide
Charles Oliver Nutter added a comment -

Migrating bugs marked as resolved before 1.1RC3 and tagged as version 1.1 as fixed in RC3. Only two bugs are actually fixed between RC3 and 1.1

Show
Charles Oliver Nutter added a comment - Migrating bugs marked as resolved before 1.1RC3 and tagged as version 1.1 as fixed in RC3. Only two bugs are actually fixed between RC3 and 1.1

People

Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: