From 33f7056888642bd6fe2da04cd84adaee5f9556e5 Mon Sep 17 00:00:00 2001 From: Sergey Fedorov Date: Wed, 3 Nov 2021 10:00:56 +0100 Subject: [PATCH 1/4] Add sorting argument to Dir.glob --- spec/tags/core/dir/glob_tags.txt | 3 --- src/main/ruby/truffleruby/core/dir.rb | 7 ++++--- src/main/ruby/truffleruby/core/dir_glob.rb | 8 +++++--- src/main/ruby/truffleruby/core/file.rb | 11 ++++++----- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/spec/tags/core/dir/glob_tags.txt b/spec/tags/core/dir/glob_tags.txt index 500675547f86..f99bcf636ec1 100644 --- a/spec/tags/core/dir/glob_tags.txt +++ b/spec/tags/core/dir/glob_tags.txt @@ -1,4 +1 @@ fails:Dir.glob recursively matches files and directories in nested dot subdirectory with 'nested/**/*' from the current directory and option File::FNM_DOTMATCH -fails:Dir.glob result is sorted by default -fails:Dir.glob result is sorted with sort: true -fails:Dir.glob sort: false returns same files diff --git a/src/main/ruby/truffleruby/core/dir.rb b/src/main/ruby/truffleruby/core/dir.rb index be8ba9b276fc..9ca826084729 100644 --- a/src/main/ruby/truffleruby/core/dir.rb +++ b/src/main/ruby/truffleruby/core/dir.rb @@ -212,7 +212,7 @@ def home(user=nil) PrivateFile.expand_path("~#{user}") end - def [](*patterns, base: nil) + def [](*patterns, base: nil, sort: true) if patterns.size == 1 pattern = Truffle::Type.coerce_to_path(patterns[0], false) return [] if pattern.empty? @@ -220,10 +220,10 @@ def [](*patterns, base: nil) patterns = [pattern] end - glob patterns, base: base + glob patterns, base: base, sort: sort end - def glob(pattern, flags=0, base: nil, &block) + def glob(pattern, flags=0, base: nil, sort: true, &block) if Primitive.object_kind_of?(pattern, Array) patterns = pattern else @@ -236,6 +236,7 @@ def glob(pattern, flags=0, base: nil, &block) matches = [] index = 0 + flags |= File::FNM_GLOB_NOSORT if sort == false normalized_base = if Primitive.nil? base nil diff --git a/src/main/ruby/truffleruby/core/dir_glob.rb b/src/main/ruby/truffleruby/core/dir_glob.rb index 1d6402810953..b7d9453287d4 100644 --- a/src/main/ruby/truffleruby/core/dir_glob.rb +++ b/src/main/ruby/truffleruby/core/dir_glob.rb @@ -405,12 +405,14 @@ def self.single_compile(glob, flags=0) last end - def self.run(node, all_matches, glob_base_dir) + def self.run(node, all_matches, glob_base_dir, flags = 0) if ConstantEntry === node node.process_directory all_matches, nil, nil, glob_base_dir else matches = [] node.process_directory matches, nil, nil, glob_base_dir + matches.sort! if (flags & File::FNM_GLOB_NOSORT) == 0 + all_matches.concat(matches) end end @@ -461,10 +463,10 @@ def self.glob(base_dir, pattern, flags, matches) patterns = compile(pattern, left_brace_index, flags) patterns.each do |node| - run node, matches, base_dir + run node, matches, base_dir, flags end elsif node = single_compile(pattern, flags) - run node, matches, base_dir + run node, matches, base_dir, flags else matches end diff --git a/src/main/ruby/truffleruby/core/file.rb b/src/main/ruby/truffleruby/core/file.rb index 46657105e218..d44c1b91c62c 100644 --- a/src/main/ruby/truffleruby/core/file.rb +++ b/src/main/ruby/truffleruby/core/file.rb @@ -104,11 +104,12 @@ module Constants W_OK = 2 # test for write permission R_OK = 4 # test for read permission - FNM_NOESCAPE = 0x01 - FNM_PATHNAME = 0x02 - FNM_DOTMATCH = 0x04 - FNM_CASEFOLD = 0x08 - FNM_EXTGLOB = 0x10 + FNM_NOESCAPE = 0x01 + FNM_PATHNAME = 0x02 + FNM_DOTMATCH = 0x04 + FNM_CASEFOLD = 0x08 + FNM_EXTGLOB = 0x10 + FNM_GLOB_NOSORT = 0x40 if Truffle::Platform.windows? NULL = 'NUL' From a4219fe06ff81af1178195b9df12cc45175b894c Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 9 Nov 2021 13:19:41 +0100 Subject: [PATCH 2/4] Use Primitive.object_equal to compare to false --- src/main/ruby/truffleruby/core/dir.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/ruby/truffleruby/core/dir.rb b/src/main/ruby/truffleruby/core/dir.rb index 9ca826084729..89359b2fa748 100644 --- a/src/main/ruby/truffleruby/core/dir.rb +++ b/src/main/ruby/truffleruby/core/dir.rb @@ -236,7 +236,7 @@ def glob(pattern, flags=0, base: nil, sort: true, &block) matches = [] index = 0 - flags |= File::FNM_GLOB_NOSORT if sort == false + flags |= File::FNM_GLOB_NOSORT if Primitive.object_equal(sort, false) normalized_base = if Primitive.nil? base nil From bde0faf46259eb8e86203738d8bd20e35eeccba9 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 9 Nov 2021 13:21:33 +0100 Subject: [PATCH 3/4] Add ChangeLog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 857b7073a293..9a91687f129a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ Compatibility: * Add `WERRORFLAG` to `RbConfig` (#2519, @bjfish). * Update `MatchData` methods to return `String` instances when called on a subclass (#2453, @bjfish). * Implement `Proc#{==,eql?}` (#2453). +* Sort by default for `Dir.{glob,[]}` and add `sort:` keyword argument (#2523, @Strech). Performance: From ffa39a7a831775084494b3dd35cbfff709114b72 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 9 Nov 2021 13:49:03 +0100 Subject: [PATCH 4/4] Retag Dir tests --- test/mri/excludes/TestDir.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/test/mri/excludes/TestDir.rb b/test/mri/excludes/TestDir.rb index 8a4dad6522c7..efa1988de313 100644 --- a/test/mri/excludes/TestDir.rb +++ b/test/mri/excludes/TestDir.rb @@ -2,13 +2,10 @@ exclude :test_chdir, "needs investigation" exclude :test_glob_base, "needs investigation" exclude :test_glob_base_dir, "needs investigation" -exclude :test_glob_cases, "needs investigation" +exclude :test_glob_cases, "fails on macOS in CI" exclude :test_glob_gc_for_fd, "needs investigation" exclude :test_glob_too_may_open_files, "needs investigation" -exclude :test_seek, "needs investigation" +exclude :test_seek, "fails on macOS in CI" exclude :test_unknown_keywords, "needs investigation" exclude :test_chdir_conflict, "needs investigation" -exclude :test_glob_order, "needs investigation" -exclude :test_symlink, "needs investigation" -exclude :test_symlinks_not_resolved, "needs investigation" exclude :test_foreach, "needs investigation"