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.

Sign up for GitHub

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

Jump to bottom

Dir.glob returns doubles, if used with recursive directory matchers "**" #4353

Closed
MartinKoerner opened this issue Dec 2, 2016 · 9 comments
Closed

Dir.glob returns doubles, if used with recursive directory matchers "**" #4353

MartinKoerner opened this issue Dec 2, 2016 · 9 comments
Labels
core JRuby 9000
Milestone
JRuby 9.1.8.0

Comments

@MartinKoerner
Copy link

Environment

  • Mac OS Sierra 10.12.1
  • Directory structure with multiple subdirectories:
   tmp
    |\2
       |\abc
       |   \2 - file1.pdf
       |   \4 - file2.pdf
       |\def
       |   \2 - file3.pdf
       |   \4 - file4.pdf

Expected Behavior

   # MRI
   # rbenv shell 2.3.1
   # irb
      Dir.glob("tmp").size # = 1
      Dir.glob("tmp/2/abc/2/*.pdf").size # = 1
      Dir.glob("tmp/**/**/**/*.pdf").size # = 4

Actual Behavior

   # rbenv shell jruby-9.1.6.0
   # jirb
     Dir.glob("tmp").size # = 1
     Dir.glob("tmp/2/abc/2/*.pdf").size # = 1
     Dir.glob("tmp/**/**/**/*.pdf").size # = 12 <- WRONG!
     Dir.glob("tmp/**/**/**/*.pdf").uniq.size # 4 # doubles are equals according to String
sumitmah pushed a commit to sumitmah/jruby that referenced this issue Jan 7, 2017
[jruby#4353] Ignore consecutive DOUBLE_STAR in glob_helper.
e71d945
sumitmah pushed a commit to sumitmah/jruby that referenced this issue Jan 7, 2017
[jruby#4353] Ignore consecutive DOUBLE_STAR in glob_helper.
9e54342
sumitmah pushed a commit to sumitmah/jruby that referenced this issue Jan 7, 2017
[jruby#4353] Renamed few variables in glob_helper
9b5135c
@sumitmah
Copy link

sumitmah commented Jan 9, 2017

@kares does the following PR fixes the issue?
#4428

@kares
Copy link
Member

kares commented Jan 9, 2017

@sumitmah well, we can't tell for sure until we do not have a test-case, right? :)

@sumitmah
Copy link

sumitmah commented Jan 9, 2017

@kares I can add test case, I was concerned about other scenarios? do we have tests for them?

@headius
Copy link
Member

headius commented Jan 9, 2017

The PR seems logical. You can run other test cases by running spec/mspec/bin/mspec ci spec/ruby/core/dir or some tests somewhere in test/mri (probably test/mri/ruby/test_dir.rb). You can also add cases to spec/ruby or test/jruby (rather not add to test/mri) that mimic the above cases.

@sumitmah
Copy link

Thanks! @headius I'll check the specs.

sumitmah pushed a commit to sumitmah/jruby that referenced this issue Jan 10, 2017
[jruby#4353] Ignore consecutive DOUBLE_STAR in glob_helper.
c81c89a
sumitmah pushed a commit to sumitmah/jruby that referenced this issue Jan 10, 2017
[jruby#4353] Renamed few variables in glob_helper
53593e8
sumitmah pushed a commit to sumitmah/jruby that referenced this issue Jan 10, 2017
[jruby#4353] added test for Dir.glob consecutive double stars should …
aca8fee 
…return uniq results
@enebo
Copy link
Member

enebo commented Jan 10, 2017

@sumitmah I do not want to land this right before 9.1.7.0 but we will merge afterwards.

@sumitmah
Copy link

@enebo when is 9.1.7.0 release planned?

@sumitmah
Copy link

@headius @kares I've added test case

sumitmah pushed a commit to sumitmah/jruby that referenced this issue Jan 10, 2017
[jruby#4353] added test for Dir.glob consecutive double stars should …
b8edd91 
…return uniq results
sumitmah pushed a commit to sumitmah/jruby that referenced this issue Jan 11, 2017
[jruby#4353] Cleaning up dirs created during test
c59de88
enebo added a commit that referenced this issue Jan 11, 2017
@enebo
Merge pull request #4428 from sumitmah/4353-fix-dir-glob
bbd0440 
[#4353] Ignore consecutive DOUBLE_STAR  in glob_helper.
@kares kares added this to the JRuby 9.1.8.0 milestone Jan 12, 2017
@kares
Copy link
Member

kares commented Jan 12, 2017

should be resolved as #4428 got merged.

@kares kares closed this as completed Jan 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
core JRuby 9000
Projects
None yet
Milestone
JRuby 9.1.8.0
Development

No branches or pull requests
5 participants
@headius @enebo @kares @MartinKoerner @sumitmah