Skip to content

only require 'java', users should explicitly require 'jruby'#5381

Merged
kares merged 4 commits intomasterfrom
require-java-only
Mar 17, 2020
Merged

only require 'java', users should explicitly require 'jruby'#5381
kares merged 4 commits intomasterfrom
require-java-only

Conversation

@kares
Copy link
Member

@kares kares commented Oct 23, 2018

seems to makes sense as using JRuby internals make the runtime "dirty"
e.g. ObjectSpace will end up finding invalid class pieces (e.g. for UNDEF)

still, its kind of a small "break" in term-of backwards compatibility
although most scripts floating around do the explicit require 'jruby'
... before scripting with internals such as JRuby.runtime

left-over from #5205 as noted at #5233

@kares kares added this to the JRuby 9.2.1.0 milestone Oct 23, 2018
@kares
Copy link
Member Author

kares commented Oct 23, 2018

marking for 9.2 ... its not a hard requirement, more of a discussion - we could post-pone for 9.3
but it would be nice to have "less" loading - be clear what is loaded (we officially only require 'java')
... and as noted above reflecting JRuby's classes (with JI) ends up with some invalid class state atm.
see #4528

@headius
Copy link
Member

headius commented Mar 4, 2020

@enebo @kares Where do we stand on this? I'm not opposed, but I have no idea whether these features are in use elsewhere.

@enebo
Copy link
Member

enebo commented Mar 4, 2020

I believe we should constrain require 'jruby' to only the places we know it will be used. Putting it near the top of JI makes no sense to me. So I think we should apply this PR but I also wonder why it is here at all? Do we use #reference or something somewhere? If so the second part of this PR would be to put the include wherever we do that.

@kares
Copy link
Member Author

kares commented Mar 5, 2020

think it has been there historically to have JRuby.runtime available - mostly due ext loading.
but we know have a better way of loading exts using JRuby::Util.load_ext(name) which avoids initializing JI for JRuby's internals ...
on places JRuby still needs JRuby.runtime an explicit require 'jruby' should do.
the only down side this has is that it might break some extensions
but I believe most of them should require 'jruby' if they depend on JRuby.runtime

@enebo
Copy link
Member

enebo commented Mar 5, 2020

9.3.0.0 seems like a good time...significant # and if this does break some extensions we have some time to get them to add a single line to fix it.

@kares
Copy link
Member Author

kares commented Mar 5, 2020

okay - will update PR as this implicit require 'jruby' now lives in Java

@kares kares force-pushed the require-java-only branch from 6b11c87 to ba4980b Compare March 11, 2020 11:20
@kares kares force-pushed the require-java-only branch from 40b266d to b492769 Compare March 12, 2020 07:37
@kares
Copy link
Member Author

kares commented Mar 12, 2020

PR is now ready ... the require 'jruby' removal is pretty much just two lines
needed to adjust tests and one place we assumed the library to be loaded, also avoided the requirement (to require 'jruby') from io/nonblock (would cause self-reflection on JRuby's Java classes so its all for the best)

@kares kares requested a review from enebo March 12, 2020 08:25
@headius
Copy link
Member

headius commented Mar 17, 2020

I'm approving this but it does feel a little weird now to have JRuby defined by default without any methods in it. Perhaps it was a bad idea originally to use the namespace directly for all these methods that would be better in their own namespaces (compilation utilities etc).

in any case, I'm fine going forward with this since most people don't even know about these utilities, and they're very much JRuby-specific anyway.

@kares kares merged commit c1bd26e into master Mar 17, 2020
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.

3 participants