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

XmlNodeSet.to_s return value conflicts with JRuby 9.2.9 #1968

Closed
headius opened this issue Jan 14, 2020 · 2 comments
Closed

XmlNodeSet.to_s return value conflicts with JRuby 9.2.9 #1968

headius opened this issue Jan 14, 2020 · 2 comments

Comments

@headius
Copy link
Contributor

headius commented Jan 14, 2020

XmlNodeSet defines a to_a method here:

public IRubyObject to_a(ThreadContext context) {

JRuby originally defined a superclass to_a method (in RubyBasicObject) that did not take a ThreadContext parameter. However in JRuby 9.2.9, we made another set of changes to pass ThreadContext through more methods, including the to_a in RubyBasicObject. This added a new signature RubyArray to_a(ThreadContext) with the old RubyArray to_a() left deprecated.

As a result, the method in XmlNodeSet no longer compiles because it attempts to return a more general type (IRubyObject as opposed to RubyArray).

@headius
Copy link
Contributor Author

headius commented Jan 14, 2020

Originally reported in a comment to #1253.

headius added a commit to headius/nokogiri that referenced this issue Jan 14, 2020
The existing signature conflicts with one added to JRuby 9.2.9.
Specifically, the new signature in JRuby returns RubyArray, which
causes a compilation error on this line in Nokogiri because it
attempts to use a more general return type.

We would prefer to keep the specific return type in JRuby.

* If we patch JRuby, then 9.2.9 will never be able to compile any
  version of Nokogiri.
* If we patch Nokogiri, all versions of JRuby can compile current
  and future Nokogiri. Versions prior to 9.2.9 will be able to
  compile all existing releases of Nokogiri.

I do not believe the change in 9.2.9 breaks anything at runtime,
since the JVM does not care about this particular return type
mismatch unless someone actually returns a non-RubyArray object.

Fixes sparklemotion#1968
headius added a commit to headius/nokogiri that referenced this issue Jan 14, 2020
The existing signature conflicts with one added to JRuby 9.2.9.
Specifically, the new signature in JRuby returns RubyArray, which
causes a compilation error on this line in Nokogiri because it
attempts to use a more general return type.

We would prefer to keep the specific return type in JRuby.

* If we patch JRuby, then 9.2.9 will never be able to compile any
  version of Nokogiri.
* If we patch Nokogiri, all versions of JRuby can compile current
  and future Nokogiri. Versions prior to 9.2.9 will be able to
  compile all existing releases of Nokogiri.

I do not believe the change in 9.2.9 breaks anything at runtime,
since the JVM does not care about this particular return type
mismatch unless someone actually returns a non-RubyArray object.

Fixes sparklemotion#1968
@flavorjones flavorjones added this to the v1.11.0 milestone Jan 15, 2020
@flavorjones
Copy link
Member

PR merged, will be fixed in v1.11.0.

flavorjones pushed a commit that referenced this issue Mar 1, 2020
The existing signature conflicts with one added to JRuby 9.2.9.
Specifically, the new signature in JRuby returns RubyArray, which
causes a compilation error on this line in Nokogiri because it
attempts to use a more general return type.

We would prefer to keep the specific return type in JRuby.

* If we patch JRuby, then 9.2.9 will never be able to compile any
  version of Nokogiri.
* If we patch Nokogiri, all versions of JRuby can compile current
  and future Nokogiri. Versions prior to 9.2.9 will be able to
  compile all existing releases of Nokogiri.

I do not believe the change in 9.2.9 breaks anything at runtime,
since the JVM does not care about this particular return type
mismatch unless someone actually returns a non-RubyArray object.

Fixes #1968
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants