JRuby doesn't support DATA and __END__ #3579

Closed
chrisseaton opened this Issue Jan 5, 2016 · 12 comments

Comments

Projects
None yet
4 participants
@chrisseaton
Contributor

chrisseaton commented Jan 5, 2016

Looks like it was removed during development for 9.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jan 5, 2016

Member

Oops. I guess test coverage is a little weak here.

Member

headius commented Jan 5, 2016

Oops. I guess test coverage is a little weak here.

@chrisseaton

This comment has been minimized.

Show comment
Hide comment
@chrisseaton

chrisseaton Jan 5, 2016

Contributor

No you've just got the tests tagged!

Contributor

chrisseaton commented Jan 5, 2016

No you've just got the tests tagged!

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jan 5, 2016

Member

Looks like @enebo removed it in fbaf038 and never got back to reimplementing it.

Member

headius commented Jan 5, 2016

Looks like @enebo removed it in fbaf038 and never got back to reimplementing it.

@chrisseaton

This comment has been minimized.

Show comment
Hide comment
@chrisseaton

chrisseaton Jan 5, 2016

Contributor

I added some stuff to your lexer to get it working in Truffle in 46bdc3c so that may be useful for you.

Contributor

chrisseaton commented Jan 5, 2016

I added some stuff to your lexer to get it working in Truffle in 46bdc3c so that may be useful for you.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jan 5, 2016

Member

@chrisseaton Thanks, that probably works for most uses of DATA. It doesn't work for stdin though.

[] ~/projects/jruby $ cat blah.rb
puts DATA.read
__END__
hello

[] ~/projects/jruby $ ruby23 < blah.rb
hello

[] ~/projects/jruby $ jruby -X+T < blah.rb
WARNING: JRuby+Truffle is designed to be run with a JVM that has the Graal compiler. The compilation is disabled Without the Graal compiler and it runs much slower. See https://github.com/jruby/jruby/wiki/Truffle-FAQ#how-do-i-get-jrubytruffle
Truffle internal error: internal implementation error - RuntimeException java.io.IOException: Can't read file /Users/headius/projects/jruby/- org.jruby.truffle.runtime.RubyContext.execute(RubyContext.java:602)
internal implementation error - RuntimeException java.io.IOException: Can't read file /Users/headius/projects/jruby/- org.jruby.truffle.runtime.RubyContext.execute(RubyContext.java:602)
    at org.jruby.truffle.nodes.methods.ExceptionTranslatingNode.execute(ExceptionTranslatingNode.java:78)
    at org.jruby.truffle.nodes.instrument.RubyWrapperNode.execute(RubyWrapperNode.java:61)
    at org.jruby.truffle.nodes.RubyRootNode.execute(RubyRootNode.java:58)
Member

headius commented Jan 5, 2016

@chrisseaton Thanks, that probably works for most uses of DATA. It doesn't work for stdin though.

[] ~/projects/jruby $ cat blah.rb
puts DATA.read
__END__
hello

[] ~/projects/jruby $ ruby23 < blah.rb
hello

[] ~/projects/jruby $ jruby -X+T < blah.rb
WARNING: JRuby+Truffle is designed to be run with a JVM that has the Graal compiler. The compilation is disabled Without the Graal compiler and it runs much slower. See https://github.com/jruby/jruby/wiki/Truffle-FAQ#how-do-i-get-jrubytruffle
Truffle internal error: internal implementation error - RuntimeException java.io.IOException: Can't read file /Users/headius/projects/jruby/- org.jruby.truffle.runtime.RubyContext.execute(RubyContext.java:602)
internal implementation error - RuntimeException java.io.IOException: Can't read file /Users/headius/projects/jruby/- org.jruby.truffle.runtime.RubyContext.execute(RubyContext.java:602)
    at org.jruby.truffle.nodes.methods.ExceptionTranslatingNode.execute(ExceptionTranslatingNode.java:78)
    at org.jruby.truffle.nodes.instrument.RubyWrapperNode.execute(RubyWrapperNode.java:61)
    at org.jruby.truffle.nodes.RubyRootNode.execute(RubyRootNode.java:58)
@chrisseaton

This comment has been minimized.

Show comment
Hide comment
@chrisseaton

chrisseaton Jan 5, 2016

Contributor

Gah didn't think of that.

Contributor

chrisseaton commented Jan 5, 2016

Gah didn't think of that.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jan 5, 2016

Member

I should have a fix shortly that will enable both runtimes to do it mostly right.

Member

headius commented Jan 5, 2016

I should have a fix shortly that will enable both runtimes to do it mostly right.

@headius headius closed this in c39e9b1 Jan 6, 2016

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jan 6, 2016

Member

@chrisseaton @enebo I pushed my fix, which mostly involved getting access to the real FileChannel for the read file, putting it in a RubyFile, and grabbing that on the other side for DATA. This is now fully compatible with MRI's DATA since it's a real File with the actual descriptor/channel used to parse. Not that anyone is ever going to care.

@enebo The previous commit removes buffering from RubyInstanceConfig.getScriptSource. I could not think of a path in our code where the resulting stream wouldn't be buffered already or get buffered by RubyIO anyway.

@chrisseaton I also added LexerSource.getRemainingAsChannel for you to use in creating an appropriate DATA for Truffle.

Member

headius commented Jan 6, 2016

@chrisseaton @enebo I pushed my fix, which mostly involved getting access to the real FileChannel for the read file, putting it in a RubyFile, and grabbing that on the other side for DATA. This is now fully compatible with MRI's DATA since it's a real File with the actual descriptor/channel used to parse. Not that anyone is ever going to care.

@enebo The previous commit removes buffering from RubyInstanceConfig.getScriptSource. I could not think of a path in our code where the resulting stream wouldn't be buffered already or get buffered by RubyIO anyway.

@chrisseaton I also added LexerSource.getRemainingAsChannel for you to use in creating an appropriate DATA for Truffle.

@enebo

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Jan 6, 2016

Member

@headius yeah script source is fine the others I might worry about

Member

enebo commented Jan 6, 2016

@headius yeah script source is fine the others I might worry about

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jan 6, 2016

Member
Member

headius commented Jan 6, 2016

@enebo

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Jan 6, 2016

Member

@headius last time I checked they were not but that was also because all buffering had been removed so I added buffering back into the mix :)

Member

enebo commented Jan 6, 2016

@headius last time I checked they were not but that was also because all buffering had been removed so I added buffering back into the mix :)

@eregon

This comment has been minimized.

Show comment
Hide comment
@eregon

eregon Jan 6, 2016

Member

We need a spec for this STDIN case 😄

Member

eregon commented Jan 6, 2016

We need a spec for this STDIN case 😄

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