Skip to content

Conversation

@headius
Copy link
Contributor

@headius headius commented Sep 24, 2021

This imports the JRuby code for io/wait, with tests green (see caveat) and hooked up to CI.

One test is skipped on JRuby due to jruby/jruby#6852, which will be fixed in JRuby 9.3.1.0.

This support is needed to release a JRuby-specific io-wait gem, a dependency of net-protocol. Without JRuby support in this gem we can't use any of the other net* gems.

@headius
Copy link
Contributor Author

headius commented Sep 24, 2021

I have no idea why the JRuby jobs are stuck.

@headius
Copy link
Contributor Author

headius commented Nov 29, 2021

I have pushed the missing bits for JRuby and rebased on current master.

Additional changes from previous review:

  • Rakefile tweaks to get paths set properly for JRuby, with the JRuby-specific wait.rb moved under ext/java/lib.
  • Gemspec tweaks to include lib/io/wait.jar and the Java ext files in the gem

This addresses both of @ahorek's review comments. I also tested that the built gem can install and replace the builtin io/wait in JRuby.

So it is pretty close now.

@headius
Copy link
Contributor Author

headius commented Nov 30, 2021

Something is still not right with the JRuby builds.

@olleolleolle
Copy link
Contributor

is it missing a bundle exec before rake?

@headius
Copy link
Contributor Author

headius commented Nov 30, 2021

There are no dependencies for CRuby (dev or otherwise) so no bundling is done in any of the builds.

I will add the necessary bits for JRuby.

@headius
Copy link
Contributor Author

headius commented Nov 30, 2021

Looks like we have a winner!

This can be squashed during merge, or if preferred I can rewrite the commits to better reflect the changes I made.

@headius
Copy link
Contributor Author

headius commented Dec 7, 2021

Pinging maintainers... I believe this is ready to merge and is necessary for JRuby 9.4 to source io-wait (and the net/* libraries that use it) from the gems. High priority for JRuby 9.4 release in the next month or two.

@headius
Copy link
Contributor Author

headius commented Jan 27, 2022

I have updated this from master again. Is there anything else needed to merge?

@headius
Copy link
Contributor Author

headius commented Jan 27, 2022

The omitted test has been un-omitted, since that issue was fixed in JRuby 9.3.1.0 (current is 9.3.3.0).

@headius
Copy link
Contributor Author

headius commented Feb 1, 2022

I rebased this on master and it will merge now. I'm fine with merge commit or squashed.

Copy link
Member

@nobu nobu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines seem unnecessary.

headius and others added 2 commits February 2, 2022 09:02
@headius
Copy link
Contributor Author

headius commented Feb 2, 2022

I have addressed all review comments.

* use case/when for chained ifs
* include only necessary files for JRuby
@nobu nobu merged commit d182718 into ruby:master Feb 3, 2022
@hsbt
Copy link
Member

hsbt commented Feb 3, 2022

🎉

@headius headius deleted the jruby branch February 3, 2022 13:36
@headius
Copy link
Contributor Author

headius commented Feb 3, 2022

Thank you both!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants