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
File.directory? of a uri:classloader resources ending in "/" incorrectly returns false #4350
Comments
Here's a small (60 line shar file) example which shows this: repro_shar.txt
|
@dmarcotte is showing me how to debug JRuby, and what it looks like from the debugger, though it is tough getting up to speed, is that path names ending in a here = File.dirname(__FILE__)
here_here = File.join(here, ".")
not_directories = [ here, here_here ].reject {|f| File.directory?(f) }
puts "GOT THESE WRONG: #{not_directories.join(" AND ")}" if not_directories.size > 0 Run from a jar, you see the WRONG output. Still swimming in stack traces and files I am not familiar with, so I don't have a fix yet. |
Debugging JRuby as it processes this, I have discovered that I can reduce the failing
|
I have a jar with one ruby file in "bin", with the above code. What happens is that Making the jar have a What I can't say is if this is a bug or not. |
I'm documenting my journey in case someone who actually understands JRuby picks up this bug. I tried adding a test to The problem seems to be at the line of code in def test_stat_trailing_slash_via_uri_classloader
jar_file = File.expand_path('../jar_with_relative_require1.jar', __FILE__)
$CLASSPATH << jar_file
jar_path = "uri:classloader:test/require_relative1.rb"
assert File.exist?(jar_path), "test is wrong, #{jar_path} doesn't even exist"
dir_in_jar = File.dirname(jar_path) + "/"
assert File.directory?(dir_in_jar), "#{dir_in_jar} claims to not be a directory"
end but the test PASSES because at this point in Enumeration<URL> urls = cl.getResources(pathname);
but in my failing test app-in-a-jar, at the same breakpoint,
and THAT path doesn't return resources ( though if you strip the trailing slash it does). I am not sure how to write a test to replicate what my test app does. because I am a noob, I am currently stalled because the debugger I am using won't let me step into the |
I was able to force a test failure by matching the jar path that def test_stat_trailing_slash_via_uri_classloader
jar_file = File.expand_path('../jar_with_relative_require1.jar', __FILE__)
$CLASSPATH << jar_file
source_file = "jar:file:#{jar_file}!/test/require_relative1.rb"
assert File.exist?(source_file), "test is wrong, #{source_file} doesn't even exist"
source_dir = File.dirname(source_file)
assert File.directory?(source_dir), "#{source_dir} not found"
source_dir += "/"
assert File.directory?(source_dir), "#{source_dir} claims to not be a directory"
end Still no clue how to fix it other than the obvious workaround-without-understanding of stripping the trailing slash in |
Hey thanks for you patience. I can never remember whether the leading slash should be there or not, so this workaround may not be far from a fix. Any thoughts @mkristian? |
@the-michael-toy @mkristian @headius fwiw adding a slash also does not work in 1.7.26. I do not see the harm in stripping the "/" since: 'dir' -> true, 'dir/' -> false, 'dir/.' -> true. In fact, it really seems like a bug to me. I think @mkristian is the ultimate judge though. If there a reason why we would want this to return false? |
The question I had which prevented me from submitting a fix, and which problems in learning how to use the debugger kept me from answering: Patch |
@mkristian I am optimistically fixing this so I can ask some people to run broader tests but I am still hoping for you to bless this fix. |
Test is named in a misleading way because I wrote it and it passed, the classloader-path re-writing difference between the test and a warble jar forced me to not use a classloader url in the test. Maybe?
|
thanks for the fix! |
@the-michael-toy I am confused by that .jrubydir logic in that method. To me, a path delimiter on the end of a file would never be considered to be a normative path since it is a delimiter and there is nothing after the delimiter. It is possible there is some reason why having a '/' at the end of a URI is valid here? I think we only use it for resolving real paths and not in the more generic sense of what you might use for a URI. Even then I am not sure that is valid for a URI? I am not sure why that one-off check exists but it is the one thing which gives me pause about my fix. |
@the-michael-toy 04ba645 I corrected the test name and tried to acknowledge you wrote that test code. Thanks for digging in. It really helped focus things. |
@enebo a fix for the for me |
@mkristian the problem is that the code you wrote for loading resources in the absence of a |
@enebo in the original bug, I started with essentially this failing code File.directory?(File.dirname(__FILE__) + "/.")
and after normalization and failure to find a .jrubydir, the uri that gets requested is If you leave the dot out though and just use a path that ends in slash, you end up with the same problem. File.directory?(File.dirname(__FILE__) + "/") |
But to the question about trailing slash in a URI ... it seems to indeed be legal and there are specific guidelines in the RFC about how to normalize |
@the-michael-toy ah thanks for that last comment. I sort of figured it is not really a delimeter in the general sense of an URI but in how we use URI I am not sure we need to worry about it since our URIs are exclusively related to filesystems. If we ever decide to support require "http://blergh/foo/bar/" then perhaps I introduced an issue but with that said we will probably have a new resource type for that and this code will still be ok...Too much coffee :) |
Hmmm .... You have made I suspect that UNIX throws an ENOENT on the second case and you probably would want to do that to, somewhere in the world there is a piece of code which will lose it's mind if you don't. Easy test to write .... (checking now) |
Yeah @enebo I don't know if this is a separate bug or just a deeper dive into this one, but even without any changes, MRI and JRuby handle /UNIX/PATH_TO_NORMAL_FILE/ differently ...
Here's MRI
And both 9.1.5.0 and 1.7.19 ( the two versions we use around here) ...
|
@the-michael-toy yeah I agree this is an issue. Your example also points out the difficulty of supporting this. In the case of directory? we can know we are testing for a directory. In that sense, we have knowledge to remove a stray '/' so the zip/jar stuff works as expected. In stat, we don't know what it is. My fix would break your last stat. This is particularly tricky to fix. For directory? if we knew it ended in we test without the '/'. For stat, if we have no / then it works as before the fix. If we do have / then we must assume the result is a directory or error? |
@the-michael-toy I would say that must be a new issue since we are not using JarResource for that. Can you open an issue for it? |
yup ... will do (#4403 ) |
Interestingly, running that test from a jar (and 9.1.6.0 ... sorry gem juggling failure )
|
This should solve all issues I hope :) we potentially ask zip file twice to check to see if it has children but for a file this will never work and we construct the proper subtype. The name is also not pruned. I think that is reasonable? |
Looks like MRI returns Errno::ENOTDIR on |
might be different depending on OS though |
@the-michael-toy I will make a note of ENOTDIR in #4403. This behavior is all wound together in respect to the notion that the resource exists but it ends in slash so it is ENOTDIR. When this is fixed the regression will break but we will realize why and fix it. At this point since regular files also return ENOENT I feel like it is still forward progress :) |
Environment
Bug description
Run from the file system the
cp_r
works as expected. If you run from a jar, you get an exception. After much investigation (see below), this is becauseFile.join(here, ".")
doesn't get recognized as a directory.This used to work in 1.7 and was discovered when testing our app under 9000. It boils down, after all the mapping of paths, to fail because
File.dirname(__FILE__) equals
uri:classloader:/dirname`File.join(dirname, ".")
gets normalized fromuri:classloader:/dirname/.
touri:classloader:/dirname/
jar:file:PATH_TO_JAR!/dirname/
URLResource.createClassloaderURI
callsgetResources
(here) to determine directory contents in the absence of a.jrubydir
file, which returns no data if passed a name with a trailing slashUpdate
I have actively been trying to find a fix for this, which makes this issue kind of long, sorry.
The text was updated successfully, but these errors were encountered: