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

Redefine uri classloader meaning #2278

Merged
merged 2 commits into from Dec 6, 2014
Merged

Conversation

mkristian
Copy link
Member

@mkristian mkristian commented Dec 5, 2014

to use uri:classloader:/ only makes sense if jruby is really loading from the Thread.currentThread().getContextClassLoader() (CCL), so in order to use it you first need to check runtime.getJRubyClassLoader.getParent to be CCL and if not supply some alternative.

this PR redefines uri:classloader:/ to be runtime.getJRubyClassLoader.getParent which always in all possible situation will pick the same classloader, the one which loaded jruby itself.

@enebo
Copy link
Member

@enebo enebo commented Dec 5, 2014

Will this break any existing applications? Your change means it will always pick the same CL that loaded JRuby but is there the possibility that somone intended to load from a different CL or would that not work unless jruby CL is parent of another? I guess I am asking if there is any chance of regressions to better understand the risk.

@mkristian
Copy link
Member Author

@mkristian mkristian commented Dec 5, 2014

the change is to pick always runtime.getJRubyClassLoader.getParent and this
classloader comes fromthe RubyInstanceConfig or via
ScriptingContainer.setClassLoader.

regression - well uri:classloader: is only used by
IsolatedScriptingContainer right now. I assume I am the only one which is
going to use it after jruby-1.7.17 is released ;)

@mkristian
Copy link
Member Author

@mkristian mkristian commented Dec 6, 2014

@enebo now since #2055 is fixed without uri:classloade: there is not risk in this anymore ! I am going to merge this so we can finally use it for https://github.com/sonatype/nexus-oss which will see quite some testing then.

mkristian added 2 commits Dec 6, 2014
…time.getJRubyClassLoader

this allows to set JRubyHome to uri:classloader:/META-INF/jruby.home for all situations where
there is a jruby.home packed inside a jar.

fixes problem with loadService and knoplerfish OSGi

fixed missed test and cleanup javadocs and add more junit-tests for uri:classloader:

make IsolatedScriptingContainer work on felix-4.2.1 and probably on other osgi frameworks outside the pax test cases
@mkristian mkristian force-pushed the redefine-uri-classloader-meaning branch from 3cb1927 to b04987a Compare Dec 6, 2014
mkristian added a commit that referenced this issue Dec 6, 2014
@mkristian mkristian merged commit 1eedf72 into jruby-1_7 Dec 6, 2014
1 check was pending
@mkristian mkristian deleted the redefine-uri-classloader-meaning branch Dec 6, 2014
@mkristian
Copy link
Member Author

@mkristian mkristian commented Dec 6, 2014

@enebo I also made a little change before I merged it to use runtime.getJRubyClassLoader as meaning of "uri:classloader:/" which actually matches how findLibraryWithClassloaders works.

@enebo
Copy link
Member

@enebo enebo commented Dec 6, 2014

Cool. Glad to see this work out for release.

@headius headius added this to the Invalid or Duplicate milestone May 4, 2015
@headius headius added this to the Invalid or Duplicate milestone May 4, 2015
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.

None yet

3 participants