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

no result with glob pattern "\\" #5333

Closed
ahorek opened this Issue Sep 28, 2018 · 2 comments

Comments

Projects
None yet
4 participants
@ahorek
Copy link
Contributor

ahorek commented Sep 28, 2018

Environment

jruby 9.2.1.0-SNAPSHOT (2.5.0) 2018-09-28 8e6904f Java HotSpot(TM) 64-Bit Server VM 25.181-b13 on 1.8.0_181-b13 +jit [linux-x86_64]

after this change rails/rails#33860
jruby build started to fail https://travis-ci.org/rails/rails/jobs/434684112

Expected Behavior

mri

Dir["/mnt/c/moje/graal/rails/rails/actionpack/test/fixtures/../\\../test/abstract_unit.rb*"]
=> ["/mnt/c/moje/graal/rails/rails/actionpack/test/fixtures/../../test/abstract_unit.rb"]

Actual Behavior

jruby

Dir["/mnt/c/moje/graal/rails/rails/actionpack/test/fixtures/../\\../test/abstract_unit.rb*"]
=> []

@kares kares added core Rails labels Oct 1, 2018

@enebo enebo added this to the JRuby 9.2.1.0 milestone Oct 1, 2018

@headius

This comment has been minimized.

Copy link
Member

headius commented Oct 9, 2018

Confirmed locally on master. Note that it works if the \\ are replaced with //:

irb(main):005:0> Dir["/Users/headius/projects/jruby/lib/../../bench2018/*.rb*"]
=> ["/Users/headius/projects/jruby/lib/../../bench2018/run_all.rb", "/Users/headius/projects/jruby/lib/../../bench2018/bench_mandelbrot.rb", "/Users/headius/projects/jruby/lib/../../bench2018/bench_regexp_match.rb", "/Users/headius/projects/jruby/lib/../../bench2018/args.rb", "/Users/headius/projects/jruby/lib/../../bench2018/bench_weird_sort.rb", "/Users/headius/projects/jruby/lib/../../bench2018/bench_chaining.rb", "/Users/headius/projects/jruby/lib/../../bench2018/bench_aref.rb", "/Users/headius/projects/jruby/lib/../../bench2018/bench_fields_hashes.rb", "/Users/headius/projects/jruby/lib/../../bench2018/bench_strptime.rb", "/Users/headius/projects/jruby/lib/../../bench2018/bench_red_black.rb", "/Users/headius/projects/jruby/lib/../../bench2018/bench_kwargs.rb", "/Users/headius/projects/jruby/lib/../../bench2018/bench_hash.rb"]
irb(main):006:0> Dir["/Users/headius/projects/jruby/lib/../\\../bench2018/*.rb*"]
=> []
irb(main):007:0> Dir["/Users/headius/projects/jruby/lib/..///../bench2018/*.rb*"]
=> ["/Users/headius/projects/jruby/lib/..///../bench2018/run_all.rb", "/Users/headius/projects/jruby/lib/..///../bench2018/bench_mandelbrot.rb", "/Users/headius/projects/jruby/lib/..///../bench2018/bench_regexp_match.rb", "/Users/headius/projects/jruby/lib/..///../bench2018/args.rb", "/Users/headius/projects/jruby/lib/..///../bench2018/bench_weird_sort.rb", "/Users/headius/projects/jruby/lib/..///../bench2018/bench_chaining.rb", "/Users/headius/projects/jruby/lib/..///../bench2018/bench_aref.rb", "/Users/headius/projects/jruby/lib/..///../bench2018/bench_fields_hashes.rb", "/Users/headius/projects/jruby/lib/..///../bench2018/bench_strptime.rb", "/Users/headius/projects/jruby/lib/..///../bench2018/bench_red_black.rb", "/Users/headius/projects/jruby/lib/..///../bench2018/bench_kwargs.rb", "/Users/headius/projects/jruby/lib/..///../bench2018/bench_hash.rb"]

Perhaps a simple issue with normalizing path separators?

@headius

This comment has been minimized.

Copy link
Member

headius commented Oct 19, 2018

Ok so I'm dumping my research since I don't know if I'll fix this over the weekend.

The logic that cleans up this rogue slash is from MRI glob_helper function, which largely has an equivalent in JRuby...except that the logic is ordered somewhat differently.

In MRI, glob_helper scans the whole set of path chunks for magic characters, and then has two separate branches to process them with and without magic. In JRuby, we check each segment for magic characters separately, but when there's no magic we do not process the segment and simply advance.

I think this may be possible to fix by considering backslash characters to be "magic" when escaping is on. I tried adding an "else" branch to the magic check that cleans up slashes, but it appears to break other things and there's a lot of missing logic in that else branch on MRI.

headius added a commit to headius/jruby that referenced this issue Oct 19, 2018

Always use full logic for processing glob segments.
This is a workaround for issues found in jruby#5333. I discovered while
researching a fix for that bug that our `glob_helper` appears to
be missing "else" logic for the segment processing that handles
magic characters and unescaping. Put simply, if there's no magic
in the segment it leaves it entirely unprocessed, allowing its
contents to just get rolled into the next segment loop (which may
or may not also have magic). However this leaves escapes in place.

This "fix" basically just omits the magic check for each segment
and allows full processing.

headius added a commit to headius/jruby that referenced this issue Oct 25, 2018

Narrow change to fix for glob logic for jruby#5333 to only escaped segs
This narrows my fix to only fire for segments with magic chars
(* and ? and the like) or backslashes in the presence of escaped
mode (i.e. not FNM_NOESCAPE). This will slow down the processing
of segments that contain only backslash escapes, but they were not
processed properly before anyway. Segments without backslashes or
magic characters are unaffected. Segments with backslashes and
magic characters are unaffected.

@headius headius closed this Oct 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.