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.

Sign up for GitHub

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

Jump to bottom

File.open ignores :universal_newline options on Windows #3736

Closed
enebo opened this issue Mar 15, 2016 · 6 comments
Closed

File.open ignores :universal_newline options on Windows #3736

enebo opened this issue Mar 15, 2016 · 6 comments
Labels
windows
Milestone
JRuby 1.7.25

Comments

@enebo
Copy link
Member

enebo commented Mar 15, 2016

Environment

both jruby 1.7.24 and JRuby 9.0.5.0 have this issue.

Expected Behavior

A file which contains \r\n should still retain the \r\n and not convert to \n if universal_newline is false.

require 'csv'
require "tempfile"

@tempfile = Tempfile.new(%w"temp .csv")
@tempfile.close
@path = @tempfile.path

File.open(@path, "wb") do |file|
  file << "1\t2\t3\r\n"
  file << "4\t5\r\n"
end

f = File.open(@path, {:universal_newline=>false})
p f.gets

Actual Behavior

We seem to always return \n as delimiter.
@enebo enebo added this to the JRuby 1.7.25 milestone Mar 15, 2016
@enebo enebo added the windows label Mar 15, 2016
@headius
Copy link
Member

headius commented Mar 15, 2016

Weird.

@enebo
Copy link
Member Author

enebo commented Mar 16, 2016

@headius I will land a fix today and I hope you can review it because it is ugly. I am hoping on 9k this is a much different fix but on 1.7 we have the CRLF wrapper and we set it in a place where we have no idea about ecflags. So my patch post-processes that wrapping and removes the CRLF wrapper based on ec settings :)

No doubt with fuller port on master we are only failing on master because windows is not hitting native paths. Hopefully, the fix will still be more harmonius...

enebo added a commit that referenced this issue Mar 16, 2016
@enebo
Fixes #3736. File.open ignores :universal_newline options on Windows
18121a7
@headius
Copy link
Member

headius commented Mar 28, 2016

@enebo Your fix for 1.7 seems "fine" even if it is pretty hacky. I'm going to take a quick look to see if it's possible to propagate ecflags to the place we create CRLF Wrapper but, but I know 1.7 has very weak ecflags processing right now.

@headius
Copy link
Member

headius commented Mar 28, 2016

@enebo I believe the patch below is a start at propagating ecflags into the actual channel wrapping. We process ecflags as part of the open process, and it's stored in the RubyFile that eventually calls fdopen, so propagating it should work ok here.

https://gist.github.com/headius/a02a91c3c744930f3876

For now I'm going to leave this change and look into the 9k fix.

@headius
Copy link
Member

headius commented Mar 28, 2016

Ok, I think I see the issue in 9k.

Because we can't open a file in text mode on Windows (Java has no such concept) we currently force read/write conversion to use UNIVERSAL_NEWLINE_DECORATOR. Your example, explicitly tries to turn that mode off, but since we're on Windows doing text mode it gets turned back on again.

So we need a way to propagate that universal newlines were explicitly disabled and not turn them on for text mode in that case.

@headius headius mentioned this issue Mar 28, 2016
Closed
@headius
Copy link
Member

headius commented Mar 28, 2016

Marking this one as fixed for 1.7. Additional work for 9k is in #3762.

@headius headius closed this as completed Mar 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
windows
Projects
None yet
Milestone
JRuby 1.7.25
Development

No branches or pull requests
2 participants
@headius @enebo