Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IO.read(bytes) behaves differently to MRI on Windows #2308

Closed
deployable opened this issue Dec 11, 2014 · 4 comments
Closed

IO.read(bytes) behaves differently to MRI on Windows #2308

deployable opened this issue Dec 11, 2014 · 4 comments

Comments

@deployable
Copy link

From issue: djberg96/ptools#23 (comment) @djberg96 found a behaviour difference in JRuby to MRI

MRI documents IO#read behaviour
> If length is a positive integer, it try to read length bytes without any conversion (binary mode).

It appears JRuby doesn't track MRI behaviour on Windows when specifying a byte length to read (f.read(bytes)). MRI changes to binary mode. JRuby still does the line end conversion.

ruby 1.9.3p551 (2014-11-13) [i386-mingw32]

C:\>c:\lang\ruby-1.9.3-p551-i386-mingw32\bin\pry
[1] pry(main)> File.open('test_slashrn','rb'){|f| f.read }
=> "something\r\notherthing\r\nthirdthing\r\n"
[2] pry(main)> File.open('test_slashrn'){|f| f.read }
=> "something\notherthing\nthirdthing\n"
[3] pry(main)> File.open('test_slashrn'){|f| f.read(35) }
=> "something\r\notherthing\r\nthirdthing\r\n"

jruby 1.7.17 (1.9.3p392) 2014-12-09 fafd1a7 on Java HotSpot(TM) 64-Bit Server VM 1.7.0_07-b10 +jit [Windows 7-amd64]

C:\>c:\lang\jruby-1.7.17\bin\pry
←[0G[1] pry(main)> File.open('test_slashrn','rb'){|f| f.read }
=> "something\r\notherthing\r\nthirdthing\r\n"
[2] pry(main)> File.open('test_slashrn'){|f| f.read }
=> "something\notherthing\nthirdthing\n"
[3] pry(main)> File.open('test_slashrn'){|f| f.read(35) }
=> "something\notherthing\nthirdthing\n"

This isn't an issue on Linux as neither MRI nor JRuby attempt conversion from \r\n to \n on the read.

@headius
Copy link
Member

headius commented Dec 15, 2014

This may work on master, where we have a more strict port of MRI's IO and line-ending logic. I'm not sure if master works on Windows yet though...

@headius
Copy link
Member

headius commented Apr 30, 2015

Master does work on Windows now, so if you can test 9.0.0.0.pre2 and report back that would be fantastic.

The problem here on both 1.7 and 9k is that we can't just rely on system-level O_BINARY and O_TEXT; there's no way to set those in Java NIO channels. MRI's logic does not manually do any crlf conversion; it merely sets the appropriate flag, and Windows POSIX APIs do the translation for it.

In 9k, I think we'll need to figure out a way to set binmode/textmode, or introduce a channel wrapper that can do the same. In 1.7, we need to modify the logic to tell our crlf-converting wrapper not to convert for binary reads.

This is not going to get fixed for 1.7.20, so I'm bumping it to 21.

@headius headius modified the milestones: JRuby 1.7.21, JRuby 1.7.20 Apr 30, 2015
@enebo enebo modified the milestones: JRuby 1.7.21, JRuby 1.7.22 Jul 7, 2015
@enebo enebo modified the milestones: JRuby 1.7.22, JRuby 1.7.23 Aug 20, 2015
@enebo enebo modified the milestones: JRuby 1.7.23, JRuby 1.7.24 Nov 24, 2015
@enebo enebo modified the milestones: JRuby 1.7.24, JRuby 1.7.25 Jan 20, 2016
@enebo
Copy link
Member

enebo commented Apr 11, 2016

This is broken on 1.7 and 9k in that we do not return proper crlf or lack thereof. On 1.7, we only fail in the third test (read(amount)) and we return as \n vs \r\n. On 9k, we also fail the third test similarly plus we fail the first test and return \n. I am punting because I do not want to dig into guts of IO in 1.7 this close to release.

@enebo enebo modified the milestones: JRuby 1.7.26, JRuby 1.7.25 Apr 11, 2016
@enebo enebo modified the milestones: JRuby 1.7.26, JRuby 1.7.27 Aug 26, 2016
@headius
Copy link
Member

headius commented Mar 15, 2017

Not going to be fixed in 1.7. This works properly on Windows with JRuby 9k. Please upgrade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants