Add support for JRuby #11

headius · 2020-01-06T20:45:36Z

This PR contains the JRuby implementation of io/console based on FFI. It should be pushed as a java platform gem any time the io-console gem is updated.

Fixes #10.

headius · 2020-01-06T20:50:57Z

One bit of process I'm not sure about: this -java gem can only be built with JRuby, and likewise the standard gem can't be build on JRuby, because the gemspec uses RUBY_ENGINE. This could be modified to allow building both versions on any impl, since they're just packaging files (i.e. there's no compile step required for either JRuby or MRI).

nobu · 2020-01-10T08:30:23Z

Do you want io-console to include the Java version?
Otherwise, it would be possible to install a stub file for JRuby, I think, like https://github.com/ruby/io-console/tree/jruby-support

headius · 2020-01-10T15:32:51Z

@nobu I think it would be best for the gem to actually contain our version of the library, as we already do for psych, json, racc, and other libraries. We will have to push a -java platform gem either way, and moving the sources from JRuby into the gem means we can maintain it alongside the CRuby version.

We want to match CRuby's use of default/preinstalled gems as closely as possible going forward.

headius · 2020-01-10T15:34:06Z

Your addition of --platform will allow any implementation to build either platform gem, so I like that part.

lib/io/console/jruby/native_console.rb

This commit makes the following changes: * Move JRuby-specific console logic to io/console/jruby * Add console.rb to load appropriate impl for the current engine * Adjust tags to match passing tests * Add test_io_console to MRI stdlib suite See ruby/io-console#11

headius · 2020-01-13T19:46:19Z

I have re-pushed the primary commit with paths fixed. This version is now also live in JRuby proper and passing CI.

I also pushed a second commit that fixes a broken test on JRuby. The path to the "rubybin" executable was tweaked to substitute /ruby/ for the name of the TTY-less executable, which caused the jruby executable to become jjruby. The test works as before on MRI.

Add `--platform` option tentatively.

This fixes ruby#10. Whenever a gem is pushed for the default platform a gem must be pushed for the 'java' platform to avoid breaking JRuby users.

eregon · 2020-02-12T17:31:55Z

I'd like to have this FFI implementation work on TruffleRuby too, see ruby/reline#124 and oracle/truffleruby#1904 (comment).

I think using the java platform wouldn't work for that, but simply use the default ruby platform and skip compilation on JRuby/TruffleRuby in extconf.rb.

nobu · 2020-02-17T03:03:47Z

lib/io/console/jruby/native_console.rb

+    t[:c_lflag] &= ~(LibC::ECHOE|LibC::ECHOK)
+  end
+
+  def raw(*, &block)


The options are not supported.

I mean min, time and intr options which are supported by IO#raw.

nobu · 2020-02-17T03:12:16Z

Isn't it better to place these files under a different name, e.g. "ffilib/", if truffleruby will use them too?

nobu · 2020-02-17T03:13:03Z

I meant "not only for jruby".

nobu · 2020-02-17T03:22:24Z

I'm thinking of installing/moving ffi-version files by extconf.rb.

eregon · 2020-02-17T10:57:38Z

@nobu I think there should be a single gem push for simplicity, so a single platform.
I'd suggest having the files under lib/io/console/ffi, and always ship them.

nobu · 2020-02-18T02:36:56Z

I pushed 0.5.6, which just skips the extension on other than CRuby.
Merger of ffi version will be after it catches 0.5 up.

eregon · 2020-02-18T17:11:22Z

@nobu Thanks, I can confirm gem install reline works on TruffleRuby now.

aycabta · 2020-02-19T05:14:53Z

I can confirm gem install reline works on TruffleRuby now.

Great! I guess that it's possible to install on JRuby too.

And, I think the intr: option should be supported by the io-console of other platforms because it's used when users press Ctrl-C to reset an inputting text.

headius · 2020-09-22T08:38:43Z

So the issue with reline was resolved. The remaining issue would be io-console getting installed by a JRuby user, possibly through a transitive dependency. I would still like to see this gem contain the FFI implementation, so it could be shared across non-CRuby implementations. The FFI would be the fallback for platforms or runtimes that do not build the C extension. Some day, perhaps it could replace the C extension?

eregon · 2020-09-22T16:29:53Z

I agree it would be good to have the FFI implementation here too.

I think @nobu had some concern in #11 (comment) and @aycabta in #11 (comment). @headius Could you try to address them so it's fully compatible with the C extension?

headius · 2020-09-22T18:32:40Z

I should be clear: this code was not originally written by me. I am hoping through this PR to release it into the public trust, so we can collaborate on making it a drop-in replacement for io/console.

I would welcome suggestions and patches for concrete improvements to make.

@nobu What do you mean "the options are not supported?" Are you referring to the LibC flags you linked? Or something like the intr: option @aycabta mentioned?

@aycabta Yes I agree this should match the C extension as closely as possible, but I'm not familiar with the intr: option.

headius · 2021-01-15T17:34:43Z

I updated my branch to include the changes @nobu made as well as one fix from @aardvark179. Not much has changed since the original discussion of this PR (e.g. the intr: option is still not supported.

headius · 2021-01-15T17:40:18Z

In the short term, can we get a -java gem release that includes the FFI logic but not the C extension?

At the moment we can't install reline on Windows because of the io-console requirement, which forces users to have a make command installed even though nothing will be built on JRuby.

Our users have been ok with this version for many years, even if it does not have all features and does not pass all tests. We will take responsibility for the code and bug reports against the FFI logic (which will probably be filed with JRuby rather than io-console).

headius · 2021-02-04T21:11:48Z

For others following along...

I have implemented the missing functionality in the FFI io/console and it now runs green on CRuby. It also runs green on JRuby with the exception of a few tests that require PTY.spawn to produce a child process with its own TTY, which is broken on JRuby (jruby/jruby#6552).

The FFI implementation can be shipped now, I believe.

See #22

headius force-pushed the jruby-support branch from 3f2cc75 to 058d2fb Compare January 6, 2020 20:45

headius mentioned this pull request Jan 6, 2020

Switch readline to reline and irb to irb gem. jruby/jruby#5803

Closed

hsbt requested a review from nobu January 7, 2020 04:42

nobu mentioned this pull request Jan 10, 2020

Added --platform option to build command rubygems/rubygems#3079

Merged

4 tasks

nobu reviewed Jan 11, 2020

View reviewed changes

lib/io/console/jruby/native_console.rb Outdated Show resolved Hide resolved

headius mentioned this pull request Jan 13, 2020

Prepare io/console for move to gem. jruby/jruby#6035

Merged

headius force-pushed the jruby-support branch from 058d2fb to 524936e Compare January 13, 2020 19:42

nobu force-pushed the master branch from 0122a21 to 8c8b0b6 Compare January 17, 2020 15:11

nobu and others added 4 commits January 28, 2020 10:09

Delegate to JRuby version

3bf1a7b

Add `--platform` option tentatively.

Add support for JRuby.

bccb2a7

This fixes ruby#10. Whenever a gem is pushed for the default platform a gem must be pushed for the 'java' platform to avoid breaking JRuby users.

Moved JRuby version files into particular path

b0691d2

Add build:java

54e59c7

nobu force-pushed the master branch from b14957f to 5facbfc Compare February 14, 2020 11:14

This was referenced Feb 16, 2020

Why does reline depend on io-console? ruby/reline#124

Closed

Remove gem dependency on io-console ruby/reline#125

Closed

nobu requested changes Feb 17, 2020

View reviewed changes

eregon mentioned this pull request Feb 18, 2020

io-console: no member named 'pathv' in 'struct rb_io_t' oracle/truffleruby#1904

Closed

eregon mentioned this pull request May 29, 2020

TruffleRuby v20.1.0 doesn't patch StringIO #14

Closed

Change NCCS to match declared size instead of defined values.

eddd042

headius force-pushed the jruby-support branch from a240630 to eddd042 Compare January 15, 2021 17:32

Merge branch 'master' into jruby-support

5ed324f

nobu approved these changes Jan 16, 2021

View reviewed changes

nobu merged commit 7b7681b into ruby:master Jan 16, 2021

headius deleted the jruby-support branch January 26, 2021 19:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for JRuby #11

Add support for JRuby #11

headius commented Jan 6, 2020

headius commented Jan 6, 2020

nobu commented Jan 10, 2020

headius commented Jan 10, 2020

headius commented Jan 10, 2020

headius commented Jan 13, 2020

eregon commented Feb 12, 2020

nobu Feb 17, 2020

nobu Oct 1, 2020

nobu commented Feb 17, 2020

nobu commented Feb 17, 2020

nobu commented Feb 17, 2020

eregon commented Feb 17, 2020

nobu commented Feb 18, 2020

eregon commented Feb 18, 2020

aycabta commented Feb 19, 2020 •

edited

headius commented Sep 22, 2020

eregon commented Sep 22, 2020

headius commented Sep 22, 2020

headius commented Jan 15, 2021

headius commented Jan 15, 2021

headius commented Feb 4, 2021 •

edited

Add support for JRuby #11

Add support for JRuby #11

Conversation

headius commented Jan 6, 2020

headius commented Jan 6, 2020

nobu commented Jan 10, 2020

headius commented Jan 10, 2020

headius commented Jan 10, 2020

headius commented Jan 13, 2020

eregon commented Feb 12, 2020

nobu Feb 17, 2020

Choose a reason for hiding this comment

nobu Oct 1, 2020

Choose a reason for hiding this comment

nobu commented Feb 17, 2020

nobu commented Feb 17, 2020

nobu commented Feb 17, 2020

eregon commented Feb 17, 2020

nobu commented Feb 18, 2020

eregon commented Feb 18, 2020

aycabta commented Feb 19, 2020 • edited

headius commented Sep 22, 2020

eregon commented Sep 22, 2020

headius commented Sep 22, 2020

headius commented Jan 15, 2021

headius commented Jan 15, 2021

headius commented Feb 4, 2021 • edited

aycabta commented Feb 19, 2020 •

edited

headius commented Feb 4, 2021 •

edited