This brings us closer to green tests.

This patch looks wrong to me. At least it looks wrong that if at this point the URL in question can ever start with jar:. If so then this patch will make a URL like 'jar:jar:file:...'. Perhaps this cannot happen here?

new File( location ).toURI.toURL is always a file:// url

oh - well I could be wrong ;)

Well that makes sense to me :) Thanks for clarification.

actually looking a bit more around the code. the patch seems totally wrong in the sense that the filename needs to be tested whether it has some "character sequence" which indicates a jar file entry.

the LibrarySearcher.ResourceLibrary takes FileResource and there are already a few subclasses of it, like JarFileResource. the only missing piece would be a getURI method on the FileResource interface and then whole loadJar method would just use the url.uri from the FileResource.

indeed the FileResource in question is a JarFileResource. happy to provide patch for the URI with "tests" ;)

@mkristian can you ask dfr about this on irc? This is pretty clearly in his zone now as he made all the *Resources.

One of my 'this seems wrong 'feelings was wondering what happens if you open a file called 'exciting!'. Making this scheme resolution part of the resource types gets rid of that question regardless of whether URI somehow appropriately escapes a non-jar ! or whatever.

From IRC discussion:

I'm reluctant to add getURI on FileResource, since FileResource is meant to abstract away ruby file access and getURI is a very load-service specific thing that is only required when trying to load jars.

Seems like a better solution would be:

1. make sure FileResource.absolutePath retains the schema. That way we should be able to just do new URI(resource.absolutePath()) and if it used to refer to a jar entry, as long it's prefixed with jar:file: it should automatically work.
2. The problem isthat JRuby is permissive to sloppy uri definitions like file:foo/bar.jar!/zeta.rb which is technically incorrect. I think it's reasonable for JarResource to still accept such urls but prefix them with jar so that it becomes: jar:file:foo/bar.jar!/zeta.rb. This may be confusing to the user: File.new("file:/foo/bar.jar!/zeta.rb").path #=> "jar:file:/foo/bar.jar!/zeta.rb" but I think it's technically more correct.

Comments?

@ratnikov : Your points seem right to me. Allowing the sloppy URLs and converting to strict should keep compatibility and encourage correct use.

ad 2) personally I would never needed use such construct - usually I run my code against MRI and JRuby ;) so looks like convenient feature ! internally we need to be strict since it we might need to convert to an URI object to use it.

ad 1) not sure absoluePath does not sound like something with scheme prefixed and for me sounds wrong. it always possible to hide getURI behind a second interface. and it could ensure that the URI is already valid inside the ExtendedFileResource and is only parsed once !

but I can live with absolutePath as well, just does not look nice

I guess what I try to say is File.new( "foo.rb" ).path #=> "file:foo.rb" looks strange. or is the scheme part on "some" file objects ?

Since "foo.jar" is a regular file, File.new( "foo.jar" ).path #=> "foo.jar" because "foo.jar" is also a valid URI and I'm assuming it'll work for classloader. If it won't, assumign that a non-absolute URI is a file is probably okay.

My comment was more about things which always have a URI, such as file:/... or things within jars, etc.

the more I think about the more it feels wrong:
File.new( "pom.xml" ).path => "file:pom.xml"
on MRI it is
File.new( "pom.xml" ).path => "pom.xml"

but to load jar file into the JRubyClassloader you need in RegularFileResource#absolutePath the scheme part so require 'my.jar' works. but for File.new( 'my.jar').path you want to act like MRI and have 'my.jar'

so the strict schema is ONLY needed for for jar-files jar:file:some.jar!/someother.jar, or file:my.jar or classpath:/some.jar so you can add it to the JRubyClassloader.

even stranger examples can be contructed with the JRuby only scheme classpath: which is not a registered protocol in the JVM.

really I am not sure about
File.new( 'classpath:/my.jar' ).path => 'jar:file:/home/some.jar!/my.jar' but only if classpath can find a jar otherwise you see something like
File.new( 'classpath:/my.jar' ).path => 'bundle:0.16.1/my.jar'
or whatever protocol the current classloader produces.

java.net.URL.new 'my.jar' => Java::JavaNet::MalformedURLException: no protocol: my.jar

I was considering doing it more like:

URL url;
try {
URI uri = new URI(resource.absolutePath());
if (uri.isAbsolute()){
url = uri.toURL();
} else {
url = new File(uri);
}
} catch (URISyntaxException badUri){
// treat bad uris as files, I guess
url = new File(resource.absolutePath());
}

So for:

• regular files no protocol gets appended, and they become non-absolute uris which gets treated as file.
• for stuff in jar, JarResource would make sure absolutePath always returns jar:file:
• classpath: are not supported right now anyway. =/

I'm also okay about having this exception for JRubyClassLoader. In fact, maybe it makes sense to add JRubyClassLoader.addJar(FileResource resource) method that will do all this evil dark magic.

probably what I do not understand is this:

when you construct a FileResource the URI is at hand - no fancy if-else-logic. and FileResource even provides a getInputStream method, which basically direct or indirect uses the URI to get the stream. then it forgets about its universal-resource-identifier and later you what to pass a FileResource into the JRubyClassloader to retrieve the URI from the absolutePath info and this string must be somehow convertible to an URI.

anyways - I guess tomorrow I need to find out why classpath:/META-INF/jruby.home/ruby/shared/jopenssl.jar does not work anymore.

before I get the feeling that I am just not able to get my point understood I created a PR to demonstrate:

#1857

let's continue from there - maybe that is a starting point to meet each other @ratnikov

I "fixed" it with 3e88f44

the main reason here is that we still have plenty of issues hanging regarding some "embedded" jars not loaded with JRubyClassloader and debug.loadService says it found the jar and with some extra println you see that the new URL(...) gets passed into the classloader. so everything looks OK but still it does not work. one possible reason is something like this:

new URL(new URL("file:/s"), "b",new URLStreamHandler() {     
        protected URLConnection openConnection( URL u ) throws IOException
        {
            return new File( "something").toURI().toURL().openConnection();
        }})

i.e. new URL(url.toString).openStream != url.openStream

please if you have suggestions to improve this commit please tell me.