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.

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

Clean up deprecated calls into JRuby #2027

Merged
merged 2 commits into from
Sep 22, 2020

Conversation

headius
Copy link
Contributor

@headius headius commented Apr 26, 2020

This PR removes all deprecation warnings from the Java extension by using newer preferred APIs in JRuby.

The bulk of changes are to use RubyException.toThrowable() to construct a Java throwable for a Ruby exception, rather than the deprecated RaiseException constructor. This change is the likely the most controversial since it breaks compatibility with JRuby versions older than 9.2.

The other changes are to use RuntimeError instead of NativeException for raising unexpected Java XML errors, and to move away from overriding the old to_s19 form from JRuby 1.7.

The deprecations here in JRuby are at least two years old. Some of
these changes are not backward-compatible with JRuby versions
prior to that, but those JRuby versions are no longer supported.

Summary of changes:

* Use RubyException.toThrowable to construct a Throwable wrapper
  for an exception, rather than constructing RaiseException
  directly. The new mechanism allows exceptions to override what
  type of Java throwable they represent. This change is not
  compatible with JRuby versions prior to 9.2.
* Avoid the long-deprecated (pre-9k) NativeException by raising
  RuntimeError for some unexpected exceptions instead of using
  RaiseException.createNativeRaiseException. The NativeException
  class has been unsupported for many years in JRuby, and these
  are better represented as RuntimeError.
* Deprecate to_s19 override and flip delegation to to_s. All of
  the "19" methods were deprecated for JRuby 9k, and many have
  since been removed. The delegation should now prefer the non-19
  name and avoid the 19 version.
@codeclimate
Copy link

codeclimate bot commented Apr 26, 2020

Code Climate has analyzed commit de8c32d and detected 0 issues on this pull request.

View more on Code Climate.

headius added a commit to headius/jruby that referenced this pull request Apr 26, 2020
This was still being used by Nokogiri as recently as Feb 2019, so
we can't delete it for some time. This usage was removed in the
middle of the 9.2 series by @jvshahid in
sparklemotion/nokogiri#1874.

See sparklemotion/nokogiri#2027 for
recent work to remove deprecated calls from Nokogiri.
@AppVeyorBot
Copy link

Build nokogiri 1.0.592 failed (commit 6804c7e4d2 by @headius)

@headius
Copy link
Contributor Author

headius commented Apr 26, 2020

The failed build is for ruby-head... I assume that's not my fault.

@headius
Copy link
Contributor Author

headius commented Sep 22, 2020

I believe this is ready after the change by @kares.

@AppVeyorBot
Copy link

Build nokogiri 1.0.679 completed (commit eaa04e6a7c by @headius)

Copy link
Contributor

@kares kares left a comment

Choose a reason for hiding this comment

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

👍

@flavorjones
Copy link
Member

Great, thank you both! Will merge this onto master for release in the next v1.11.0 release candidate.

@headius Please let me know if you want this pulled into a v1.10.x (stable) release; I'm assuming it's OK to just ship in a v1.11.0 final later this year.

@flavorjones flavorjones merged commit 6c8cb7b into sparklemotion:master Sep 22, 2020
flavorjones added a commit that referenced this pull request Sep 22, 2020
@headius
Copy link
Contributor Author

headius commented Sep 22, 2020

@flavorjones Yeah there's no rush on this. 👍

@headius headius deleted the remove_deprecated_jruby branch September 22, 2020 22:36
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.

4 participants