✅ Add TruffleRuby and JRuby to CI

[nevans]

Will fix #454 (once it's passing)

net-imap issues/PRs

• ♻️ Short-circuit frozen SequenceSet modifications #473 (JRuby FrozenError doesn't use #inspect)
• ✅ Update Regexp.linear_time? tests for non-CRuby #477
• ✅ Update ResponseReader, UIDFetchData, DeprecatedClientOptions tests #476
• ✅ Add timeouts to CI workflow #478
• Don't wait for lock before closing the socket in #disconnect

JRuby issues/PRs

• Data fixes and improvements jruby/jruby#8829 - to fix Data subclasses
• Splat with multi-line expression improperly expands jruby/jruby#8830 - fixed the response parser tests
• Prism compiler fails with lonely operator in complex call sequence jruby/jruby#8832
• Struct issues: 002db9f.
• All (or nearly all) of the SSL connection tests are failing. Hopefully this only requires small tweaks to net-imap test code.

TruffleRuby issues/PRs:

• "%p" % nil is "nil" on CRuby but "" on TruffleRuby oracle/truffleruby#3846
• Recursive inspect for Data gives StackOverflowError oracle/truffleruby#3847
• Frozen Struct can be modified oracle/truffleruby#3850
• Incompatible Struct#initialize with mixed keyword args oracle/truffleruby#3855
• StringIO#gets("\r\n", 10) returns an extra \n oracle/truffleruby#3856
• Support the same Regexps to be Regexp.linear_time? as CRuby oracle/truffleruby#3858

Misc

• I either omitted or marked as pending everything that's currently failing.
• TODO: diagnose deadlock on error (between get_tagged_response and receive_responses?)
• Both JRuby and TruffleRuby still fail in various ways when inheriting from Data.
  • I temporarily added in a crazy feature-check to our Data polyfill. Rather than just check RUBY_ENGINE and RUBY_ENGINE_VERSION and defined?(::Data) and ::Data.respond_to?(:define), it tries to use all of the features that were causing tests to fail. If anything looks off, it prints a warning and falls back to the polyfill. I'm not going to merge this feature check. But it's in the branch. 😉
  • net-imap v0.6.0 will still be deleting DataLite.

[eregon]

I had a quick look through the failures at https://github.com/ruby/net-imap/actions/runs/14711726747/job/41285563407?pr=470

Failure: test_recursive_inspect(Net::IMAP::TestData)
/home/runner/work/net-imap/net-imap/test/net/imap/test_data_lite.rb:180:in `test_recursive_inspect'
     177:         # anonymous class
     178:         list = klass[value: 1, tail: [2, 3, 4]]
     179:         seen = "#<data #{klass.inspect}:...>"
  => 180:         assert_equal(
     181:           "#<data value=1, head=nil," \
     182:           " tail=#<data value=2, head=#{seen}," \
     183:           " tail=#<data value=3, head=#{seen}," \
<internal:core> core/throw_catch.rb:36:in `catch'
<internal:core> core/throw_catch.rb:36:in `catch'
<"#<data value=1, head=nil, tail=#<data value=2, head=#<data #<Class:0x411e8>:...>, tail=#<data value=3, head=#<data #<Class:0x411e8>:...>, tail=#<data value=4, head=#<data #<Class:0x411e8>:...>, tail=nil>>>>"> expected but was
<"#<data value=1, head=, tail=#<data value=2, head=#<data #<Class:0x411e8>:...>, tail=#<data value=3, head=#<data #<Class:0x411e8>:...>, tail=#<data value=4, head=#<data #<Class:0x411e8>:...>, tail=>>>>">

That looks like an issue of Data#inspect or #to_s in truffleruby not calling inspect on the values.
But https://github.com/oracle/truffleruby/blob/ac88a0fe68bf957f75af7d316594b89731fdec4e/src/main/ruby/truffleruby/core/data.rb#L141-L152 seems to clearly call inspect on each value, so it's a mystery (unless something redefines nil.inspect? I would hope not, but would be worth checking).
EDIT: it might be related to #470 (comment)
Indeed, and the actual difference is "%p" % nil is "" on TruffleRuby but "nil" on CRuby => oracle/truffleruby#3846

For the other failures I don't see anything obvious, I think they'll need investigation with some debugging, I suggest to do that later and so omit these 9 failures for now.

[eregon]

Both JRuby and TruffleRuby still fail in various ways when inheriting from Data or Struct.

One thought I had given this gem is 3.1+ would be to use Struct (which exists since forever) instead of Data (+ freeze it, BTW thanks for this issue, should be an easy quick fix).
Then you wouldn't need DataLite at all and wouldn't need to test that Data|DataLite work the same/good enough for the usage here.

But you're saying there are other issues with Struct too?

[nevans]

One thought I had given this gem is 3.1+ would be to use Struct (which exists since forever) instead of Data

That's fair. I did consider going that route. But I also wanted the more constrained API of Data vs Struct.

I originally started down the path of building a facade over Struct. And then a switched from that to just a PORO. But the API I wanted turned out to be the Data-API. So in the end, I just used Data. And I'd already done much of the work for several features using Data... but I was waiting for v0.6.0 (which will require ruby > 3.2) before merging them.

FetchData is a good example of where the Struct API comes into direct conflict with the natural (Principle of Least Surprise) API for a FETCH response object: #[] should delegate to attr#[], and #size should delegate to attr["RFC822.SIZE"]. My long-term goal for many (not all) of the existing structs is to add deprecation warnings to Struct-specific methods for a couple of versions (i.e: a couple of years), then swap out their implementations for Data or POROs.

So I was originally going to hold off on merging any new data structures until v0.6.0, just to avoid these issues. But work priorities bumped up the schedule for #333 and #329, which were both features I'd already mostly completed on my own time, based on Data. So I had to decide between 1) using a fork for ~18 months, 2) converting the objects to Struct, 3) converting the objects to a Struct-facade (probably just a subset of the Data API, 4) converting to a PORO API subset, or 5) using a Data polyfill which would only be used for one (now-EOL) version of ruby. 🤷🏻 (In the end we used a fork in production for almost three months before I merged and released everything.)

(+ freeze it, BTW thanks for this issue, should be an easy quick fix).

great! 👍🏻

Then you wouldn't need DataLite at all and wouldn't need to test that Data|DataLite work the same/good enough for the usage here.

My intention has always been (see #352 (comment)) for v0.6.0 to delete DataLite and set Net::IMAP::Data = ::Data. It only exists for 1) backwards compatibility with a single (now-EOL) version of ruby, and 2) YAML encoding. And the next release of psych will add YAML encoding: ruby/psych#692. 🎉

While working on this ticket, I briefly considered keeping it in v0.6.0, just for TruffleRuby and JRuby... but... I'm not gonna do that. 😉 I expect the JRuby and TruffleRuby bugfixes to come faster than my 0.6.0 release plans. 🙂

But you're saying there are other issues with Struct too?

Yep. If you don't get to it first, I'll open TruffleRuby issue for it later today. But the failing net-imap tests are here: 7fd8e2c.

And, to me, they seem very similar to the Data issues. My intuition may be completely wrong, but I would not be surprised if the fixes for Data also fix Struct, or vice versa.

[nevans]

And, to me, they seem very similar to the Data issues. My intuition may be completely wrong, but I would not be surprised if the fixes for Data also fix Struct, or vice versa.

On further investigation, I suspect the Struct issue is not inheritance related, but a difference in how Struct handles mixed positional and keyword arguments.

I created oracle/truffleruby#3855.

[nevans]

@eregon here's the issue I saw with IO#gets(CRLF, limit):

#!/usr/bin/env ruby

puts ?=*72
puts "Ruby Engine:   #{RUBY_ENGINE} #{RUBY_ENGINE_VERSION}"

require 'bundler/inline'
gemfile do
  gem "stringio", ">= 3.1.7"
end
require "stringio"
puts "Using StringIO #{StringIO::VERSION}"
io = StringIO.new("123456789\r\n123456789\r\n")
pp stringio: io.gets("\r\n", 10)

r, w = IO.pipe
w.write "123456789\r\n123456789\r\n"
w.close
pp io_pipe: r.gets("\r\n", 10)

__END__
$ ruby test.rb && RBENV_VERSION=truffleruby-24.2.1 ruby test.rb
========================================================================
Ruby Engine:   ruby 3.4.2
Using StringIO 3.1.7
{stringio: "123456789\r"}
{io_pipe: "123456789\r"}
========================================================================
Ruby Engine:   truffleruby 24.2.1
Using StringIO 3.0.1
{:stringio=>"123456789\r\n"}
{:io_pipe=>"123456789\r"}

I didn't make a TruffleRuby ticket for this. I'm guessing this is already a known issue, given that it silently failed to use a newer version of StringIO (even after I manually gem installed it). And it looks like it only affects the test which uses StringIO and probably doesn't affect real IO.

[eregon]

Thank you for reporting those and your work on adding TruffleRuby & JRuby in CI!

Using Struct was just a quick idea, thank you for the detailed reply, I didn't expect that much details but it's good context.
Yeah, if any of these objects is exposed publicly then it's a lot trickier to switch between both.

Re the StringIO issue, you are right, currently StringIO on TruffleRuby always uses our internal pure-Ruby implementation of it. But we should fix it to match the gem, so I created an issue from your comment: oracle/truffleruby#3856

[duerst]

@nevans

My long-term goal for many (not all) of the existing structs is to add deprecation warnings to Struct-specific methods for a couple of versions (i.e: a couple of years), then swap out their implementations for Data or POROs.

Sorry for the side question, but can you explain what a PORO is? I wasn't able to find this with a Web search, probably just my bad.

[eregon]

PORO = Plain Old Ruby Object (might derive from POJO use in Java?), i.e. just objects with a regular class and typically Object superclass, e.g. using attr_reader.