Dir.glob is broken for files inside jar #1453

Closed
ratnikov opened this Issue Jan 28, 2014 · 5 comments

Projects

None yet

3 participants

@ratnikov
Contributor

Suppose we have a jar with following files:

$ jar tf foo.jar 
META-INF/
META-INF/MANIFEST.MF
foo/bar.txt

Dir["file:/.../foo.jar!/**/*"] will report correctly all three entries, whereas Dir.glob("file:/.../foo.jar!/**/*") will skip over foo/bar.txt

The reason Dir[] works is that RubyDir.aref uses special logic for jars to convert glob into a regex: https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/RubyDir.java#L209

The reason Dir.glob doesn't work is because it delegates control to MRI-style Dir.push_glob which descends into a directory only if it exists, and file:/.../foo.jar!foo/ does not exist.

To add insult to injury, https://github.com/jruby/jruby/blob/master/spec/java_integration/utilities/jar_glob_spec.rb is supposed to test this behavior, but it overrides Dir.glob that it is supposed to test, so it happily passes although implementation is broken.

Things to do:

  1. Fix the jar_glob_spec.rb test to behave properly.

  2. Fix the test.

As far fixing the test goes, I'm going to remove the jar specific logic from RubyDir in favor of making Dir.push_glob use FileResources as part of the #1452 effort. This should fix the issue, since JarDirectoryResource("foo/") will exist, allowing the logic to descend properly. If for some reason that won't work out (or we'd like the build fixed sooner) I could also try to refactor RubyDir.glob to perform the jar-specific logic of RubyDir.aref, and have RubyDir.aref invoke glob(..., 0) like it should.

@ratnikov
Contributor

Sent pull request for fixing the spec: #1454

@ratnikov
Contributor

Update: The behavior described was against 1.7.10, without my FileResource patch applied. With the patch applied, there's slightly different behavior with Dir.glob returning following entries:

["/META-INF", "/META-INF/", "/META-INF//", "/META-INF//MANIFEST.MF", "/META-INF/MANIFEST.MF", "/META-INF/MANIFEST.MF", "/foo/bar.txt" ]

Now behavior is incorrect for /META_INF since both /META-INF (as JarEntry) and /META-INF/ (as part of /META-INF/MANIFEST.MF) exist. Should be simple to fix by always pushing directory entry with a slash. I'll just update my PR to fix this.

@chrisseaton
Member

Is this related to #1466 do you think?

@ratnikov
Contributor
ratnikov commented Feb 1, 2014

Probably is. Although I'm not really sure why they'd be hanging, at least when I was running tests they weren't.

Could you check if my patch fixes the hanging for you?

@chrisseaton
Member

I just get the same failures as Travis does.

@enebo enebo closed this in #1468 Feb 4, 2014
@enebo enebo added this to the JRuby 1.7.11 milestone Feb 21, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment