From eb06eec74433e63c831c08cca2034ddd2cd936cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 2 Apr 2022 16:10:30 +0200 Subject: [PATCH 1/7] Utility: add a Path::glob() function. I was procrastinating this for way too long already. But as expected, it was an immense load of pain. --- doc/corrade-changelog.dox | 2 + src/Corrade/Utility/Path.cpp | 150 ++++++- src/Corrade/Utility/Path.h | 107 ++++- src/Corrade/Utility/Test/CMakeLists.txt | 8 + src/Corrade/Utility/Test/DirectoryTest.cpp | 5 +- src/Corrade/Utility/Test/PathTest.cpp | 415 +++++++++++++++++- .../Test/PathTestFilesGlob/.dir.txt/dummy | 0 .../Utility/Test/PathTestFilesGlob/.file.txt | 0 .../Test/PathTestFilesGlob/dir.txt/dummy | 0 .../Utility/Test/PathTestFilesGlob/file1.txt | 0 .../Utility/Test/PathTestFilesGlob/file11.txt | 0 .../Utility/Test/PathTestFilesGlob/file12.txt | 0 src/Corrade/Utility/Test/configure.h.cmake | 1 + 13 files changed, 663 insertions(+), 25 deletions(-) create mode 100644 src/Corrade/Utility/Test/PathTestFilesGlob/.dir.txt/dummy create mode 100644 src/Corrade/Utility/Test/PathTestFilesGlob/.file.txt create mode 100644 src/Corrade/Utility/Test/PathTestFilesGlob/dir.txt/dummy create mode 100644 src/Corrade/Utility/Test/PathTestFilesGlob/file1.txt create mode 100644 src/Corrade/Utility/Test/PathTestFilesGlob/file11.txt create mode 100644 src/Corrade/Utility/Test/PathTestFilesGlob/file12.txt diff --git a/doc/corrade-changelog.dox b/doc/corrade-changelog.dox index e6ecb7fa9..c27da7e89 100644 --- a/doc/corrade-changelog.dox +++ b/doc/corrade-changelog.dox @@ -137,6 +137,8 @@ namespace Corrade { - New @ref Corrade/Utility/Math.h header implementing @ref Utility::min() and @ref Utility::max() because having to @cpp #include @ce to get @ref std::min() and @ref std::max() is unacceptable. +- New @ref Utility::Path::glob() function for listing directory contents + matching a wildcard pattern - Added @ref Utility::String::lowercaseInPlace() and @relativeref{Utility::String,uppercaseInPlace()} together with @ref Utility::String::lowercase() and @relativeref{Utility::String,uppercase()} overloads taking a diff --git a/src/Corrade/Utility/Path.cpp b/src/Corrade/Utility/Path.cpp index c2bd9b42f..4f31af5cf 100644 --- a/src/Corrade/Utility/Path.cpp +++ b/src/Corrade/Utility/Path.cpp @@ -64,6 +64,7 @@ #include #include #include +#include #include #ifdef CORRADE_TARGET_APPLE #include @@ -750,15 +751,111 @@ Containers::Optional> list(const Container closedir(directory); + if(flags & (ListFlag::SortAscending|ListFlag::SortDescending)) + std::sort(list.begin(), list.end()); + /* We don't have rbegin() / rend() on Array (would require a custom + iterator class or a StridedArrayView), so just reverse the result */ + if(flags >= ListFlag::SortDescending && !(flags >= ListFlag::SortAscending)) + std::reverse(list.begin(), list.end()); + + /* GCC 4.8 and Clang 3.8 need extra help here */ + return Containers::optional(std::move(list)); + + /* Windows (not Store/Phone) is implemented via glob(), a subset of the + flag bits supported by both is expected to match */ + #elif defined(CORRADE_TARGET_WINDOWS) && !defined(CORRADE_TARGET_WINDOWS_RT) + return glob(join(path, "*"_s), GlobFlag(static_cast(flags))); + #else + #error + #endif + + /* Other not implemented */ + #else + Error{} << "Utility::Path::list(): not implemented on this platform"; + static_cast(path); + return {}; + #endif +} + +Containers::Optional> glob(const Containers::StringView pattern, GlobFlags flags) { + #if defined(CORRADE_TARGET_UNIX) || defined(CORRADE_TARGET_EMSCRIPTEN) || (defined(CORRADE_TARGET_WINDOWS) && !defined(CORRADE_TARGET_WINDOWS_RT)) + /* POSIX-compliant Unix, Emscripten */ + #if defined(CORRADE_TARGET_UNIX) || defined(CORRADE_TARGET_EMSCRIPTEN) + /* Don't allow escaping * and ? with \ for consistency with Windows and to + avoid clashing with directory separators on Windows. + + Also fail when encountering an, which means it'd fail also when stat() + on any file would fail, which is a different behavior from Path::list(). + There's no way to separate the two cases except for attaching the error + handler and then ... um ... trying to figure out whether the failure + happened during opendir() or during stat()? The error handler carries no + state pointer, so that would be excessively shitty. + + Without GLOB_ERR set, it would just try to carry on, ultimately claiming + a GLOB_NOMATCH. And a GLOB_NOMATCH is not an error state as it happens + when globbing an existing but empty directory. */ + int globFlags = GLOB_NOESCAPE|GLOB_ERR; + /* The output is sorted by default which means we don't have to do that. + Disable it if not requested as an optimization. */ + if(!(flags & (GlobFlag::SortAscending|GlobFlag::SortDescending))) + globFlags |= GLOB_NOSORT; + glob_t out; + const int result = glob(Containers::String::nullTerminatedView(pattern).data(), globFlags, nullptr, &out); + /* Having no results is fine, having an error is not */ + if(result == GLOB_NOMATCH) + return Containers::Array{}; + if(result != 0) { + Error err; + err << "Utility::Path::glob(): can't glob" << pattern << Debug::nospace << ":"; + Utility::Implementation::printErrnoErrorString(err, errno); + return {}; + } + + Containers::Array list; + + for(std::size_t i = 0; i != out.gl_pathc; ++i) { + /* If the pattern was a full path, the output returns a full path as + well. For consistency with list() and with the Windows API we return + just a filename portion. */ + const Containers::StringView file = split(out.gl_pathv[i]).second(); + if((flags >= GlobFlag::SkipDotAndDotDot) && (file == "."_s || file == ".."_s)) + continue; + + /* Compared to readdir() we don't implicitly get any entry type here, + so we have bear an extra overhead of stat() for every returned path + -- thus do it only if we're told to skip anything. If it fails for + the particular entry, treat it as "neither a file nor a directory" + and leave it in the list -- we're told to skip files/directories, + not "include only files/directories". */ + if(flags & (GlobFlag::SkipDirectories|GlobFlag::SkipFiles|GlobFlag::SkipSpecial)) { + /* stat() follows the symlink, lstat() doesn't. This is equivalent + to what's done in Path::list(), except that here it's always and + not just for symlinks. */ + struct stat st; + /* gl_pathv[i] is absolute if pattern was absolute, so no need to + prepend it again */ + if(stat(out.gl_pathv[i], &st) == 0) { + if(flags >= GlobFlag::SkipDirectories && S_ISDIR(st.st_mode)) + continue; + if(flags >= GlobFlag::SkipFiles && S_ISREG(st.st_mode)) + continue; + if(flags >= GlobFlag::SkipSpecial && !S_ISDIR(st.st_mode) && !S_ISREG(st.st_mode)) + continue; + } + } + + arrayAppend(list, file); + } + + globfree(&out); + /* Windows (not Store/Phone) */ #elif defined(CORRADE_TARGET_WINDOWS) && !defined(CORRADE_TARGET_WINDOWS_RT) WIN32_FIND_DATAW data; - /** @todo drop the StringView cast once widen(const std::string&) is - removed */ - HANDLE hFile = FindFirstFileW(Unicode::widen(Containers::StringView{join(path, "*"_s)}), &data); + HANDLE hFile = FindFirstFileW(Unicode::widen(pattern), &data); if(hFile == INVALID_HANDLE_VALUE) { Error err; - err << "Utility::Path::list(): can't list" << path << Debug::nospace << ":"; + err << "Utility::Path::glob(): can't glob" << pattern << Debug::nospace << ":"; Utility::Implementation::printWindowsErrorString(err, GetLastError()); return {}; } @@ -773,36 +870,51 @@ Containers::Optional> list(const Container Containers::Array list; - /* Explicitly add `.` for compatibility with other systems */ - if(!(flags & (ListFlag::SkipDotAndDotDot|ListFlag::SkipDirectories))) - arrayAppend(list, "."_s); + /* Explicitly add `.` for compatibility with other systems if the glob + pattern would include it. Windows doesn't have any special treatment for + dotfiles, so `*` should match it as well, the same as it matches + `..`. */ + if(!(flags & (GlobFlag::SkipDotAndDotDot|GlobFlag::SkipDirectories))) { + const Containers::StringView wildcard = split(pattern).second(); + /** @todo not robust enough, ?, ** or .***** matches them as well */ + if(wildcard == "*"_s || wildcard == ".*"_s) + arrayAppend(list, "."_s); + } while(FindNextFileW(hFile, &data) != 0 || GetLastError() != ERROR_NO_MORE_FILES) { - if((flags >= ListFlag::SkipDirectories) && (data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) + if((flags >= GlobFlag::SkipDirectories) && (data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) continue; - if((flags >= ListFlag::SkipFiles) && !(data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) + if((flags >= GlobFlag::SkipFiles) && !(data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) continue; /** @todo symlink support */ /** @todo are there any special files in WINAPI? */ - /* Not testing for dot, as it is not listed on Windows. Also it doesn't - cause any unnecessary temporary allocation if SkipDotAndDotDot is - used because `..` fits easily into SSO. */ + /* Not testing for `.`, as it is not listed on Windows -- we explicitly + added it above if it made sense. We could also check for the UTF-16 + variant before creating a String instance, but `..` fits easily into + SSO so there's no unnecessary allocation being done. Plus `..` + appears just once for the whole call, so that's a constant overhead + factor. */ Containers::String file = Unicode::narrow(data.cFileName); - if((flags >= ListFlag::SkipDotAndDotDot) && file == ".."_s) + if((flags >= GlobFlag::SkipDotAndDotDot) && file == ".."_s) continue; arrayAppend(list, std::move(file)); } + + /* Sorting done just here, Unix glob() can do it on its own so that's what + we use there instead. Reversal of the sorted list is done below for + both, however. */ + if(flags & (GlobFlag::SortAscending|GlobFlag::SortDescending)) + std::sort(list.begin(), list.end()); #else #error #endif - if(flags & (ListFlag::SortAscending|ListFlag::SortDescending)) - std::sort(list.begin(), list.end()); - /* We don't have rbegin() / rend() on Array (would require a custom - iterator class or a StridedArrayView), so just reverse the result */ - if(flags >= ListFlag::SortDescending && !(flags >= ListFlag::SortAscending)) + /* On Unix the sorting was done by glob() already, on Windows via + std::sort(), so it's just the descending order left, which is done by + reversing the sorted list. */ + if(flags >= GlobFlag::SortDescending && !(flags >= GlobFlag::SortAscending)) std::reverse(list.begin(), list.end()); /* GCC 4.8 and Clang 3.8 need extra help here */ @@ -810,7 +922,7 @@ Containers::Optional> list(const Container /* Other not implemented */ #else - Error{} << "Utility::Path::list(): not implemented on this platform"; + Error{} << "Utility::Path::glob(): not implemented on this platform"; static_cast(path); return {}; #endif diff --git a/src/Corrade/Utility/Path.h b/src/Corrade/Utility/Path.h index 513428539..773ebcc6d 100644 --- a/src/Corrade/Utility/Path.h +++ b/src/Corrade/Utility/Path.h @@ -502,7 +502,8 @@ CORRADE_ENUMSET_OPERATORS(ListFlags) @m_since_latest If @p path is not a directory or it can't be opened, prints a message to -@ref Error and returns @ref Containers::NullOpt. +@ref Error and returns @ref Containers::NullOpt. On Windows, this function is +implemented in terms of @ref glob() --- as @cpp glob(join(path, "*")) @ce. Expects that the @p path is in UTF-8. If it's already @ref Containers::StringViewFlag::NullTerminated, it's passed to system APIs @@ -514,10 +515,112 @@ passed to system APIs. @ref ListFlag::SkipFiles and @ref ListFlag::SkipDirectories affects the link target, not the link itself. This behavior is not implemented on Windows at the moment. -@see @ref isDirectory(), @ref exists() +@see @ref glob(), @ref isDirectory(), @ref exists() */ CORRADE_UTILITY_EXPORT Containers::Optional> list(Containers::StringView path, ListFlags flags = {}); +/** +@brief Directory globbing flag +@m_since_latest + +@see @ref GlobFlags, @ref glob() +*/ +enum class GlobFlag: unsigned char { + /* Values have to be kept consistent with ListFlag, as on Windows list() is + implemented via glob(). However the two are deliberately separate to + make room for possible extensions affecting only glob() and not list() + (such as enabling platform-specific behavior) */ + + /** Skip `.` and `..` directories */ + SkipDotAndDotDot = 1 << 0, + + /** + * Skip regular files + * @partialsupport On @ref CORRADE_TARGET_WINDOWS "Windows" and + * @ref CORRADE_TARGET_EMSCRIPTEN "Emscripten" skips everything except + * directories, as there's no concept of a special file. + */ + SkipFiles = 1 << 1, + + /** Skip directories (including `.` and `..`) */ + SkipDirectories = 1 << 2, + + /** + * Skip everything that is not a file or directory + * @partialsupport Has no effect on @ref CORRADE_TARGET_WINDOWS "Windows" + * and @ref CORRADE_TARGET_EMSCRIPTEN "Emscripten", as these platforms + * don't have a concept of a special file. + */ + SkipSpecial = 1 << 3, + + /** + * Sort items in ascending order. If both @ref GlobFlag::SortAscending and + * @ref GlobFlag::SortDescending is specified, ascending order is used. + */ + SortAscending = (1 << 4) | (1 << 5), + + /** + * Sort items in descending order. If both @ref GlobFlag::SortAscending and + * @ref GlobFlag::SortDescending is specified, ascending order is used. + */ + SortDescending = 1 << 5 +}; + +/** +@brief Directory globbing flags +@m_since_latest + +@see @ref glob() +*/ +typedef Containers::EnumSet GlobFlags; + +CORRADE_ENUMSET_OPERATORS(GlobFlags) + +/** +@brief Glob directory contents +@m_since_latest + +Expands upon @ref list() by allowing wildcard patterns in the path. An `*` +matches zero or more characters, a `?` matches exactly one characters. As with +@ref list(), the returned names are always relative to the last path component +of @p pattern. If @p path is not a directory or it can't be opened, prints a +message to @ref Error and returns @ref Containers::NullOpt. Usage examples: + +- @cpp "path/‌*" @ce lists contents of `path`. Equivalent to calling + @ref list() with @cpp "path" @ce and with @ref ListFlag::SkipDotAndDotDot + on Unix systems. On Windows it's equivalent to @ref list() with empty + @ref ListFlags as the system doesn't have any special treatment for files + starting with a dot. +- @cpp "path/‌.*" @ce lists files starting with a dot in `path`, including + `.` and `..` directories. Equivalent to calling @ref list() with + @cpp "path" @ce and empty @ref ListFlags on both Unix systems and Windows. +- @cpp "path/‌*.txt" @ce returns all names ending with `.txt` in `path`. + If @ref GlobFlag::SkipDirectories is not set, this may include not just + files, but also directories. +- @cpp "path/tile2?.jpg" @ce would match for example `tile20.jpg` to + `tile29.jpg` but not `tile2.jpg` or `tile205.jpg`. + +The function is implemented using platform-specific APIs. While the subset +shown above is guaranteed to work in a cross-platform way, behavior with other +wildcard specifiers may differ between platforms. Behavior with wildcards +appearing in the path instead of the filename is unspecified. + +Expects that the @p path is in UTF-8. If it's already +@ref Containers::StringViewFlag::NullTerminated, it's passed to system APIs +directly, otherwise a null-terminated copy is allocated first. On Windows the +path is instead first converted to UTF-16 using @ref Unicode::widen() and then +passed to system APIs. +@partialsupport On @ref CORRADE_TARGET_UNIX "Unix" platforms and + @ref CORRADE_TARGET_EMSCRIPTEN "Emscripten", symlinks are followed and + @ref ListFlag::SkipFiles and @ref ListFlag::SkipDirectories affects the + link target, not the link itself. This behavior is not implemented on + Windows at the moment. +@see @ref isDirectory(), @ref exists() +*/ +/* In the docs above, there's a Unicode ZWNJ between / and * to avoid compiler + warnings about nested block comments */ +CORRADE_UTILITY_EXPORT Containers::Optional> glob(Containers::StringView pattern, GlobFlags flags = {}); + /** @brief File size @m_since_latest diff --git a/src/Corrade/Utility/Test/CMakeLists.txt b/src/Corrade/Utility/Test/CMakeLists.txt index f2ad656a9..1aeda24d3 100644 --- a/src/Corrade/Utility/Test/CMakeLists.txt +++ b/src/Corrade/Utility/Test/CMakeLists.txt @@ -222,11 +222,13 @@ set(UtilityPathTest_SRCS PathTest.cpp) if(CORRADE_TARGET_IOS) set_source_files_properties( PathTestFiles + PathTestFilesGlob PathTestFilesSymlink PathTestFilesUtf8 PROPERTIES MACOSX_PACKAGE_LOCATION Resources) list(APPEND PathDirectoryTest_SRCS PathTestFiles + PathTestFilesGlob PathTestFilesSymlink PathTestFilesUtf8) endif() @@ -234,6 +236,12 @@ corrade_add_test(UtilityPathTest ${UtilityPathTest_SRCS} FILES PathTestFiles/dir/dummy PathTestFiles/file + PathTestFilesGlob/.file.txt + PathTestFilesGlob/file1.txt + PathTestFilesGlob/file11.txt + PathTestFilesGlob/file12.txt + PathTestFilesGlob/.dir.txt/dummy + PathTestFilesGlob/dir.txt/dummy PathTestFilesSymlink/dir/dummy PathTestFilesSymlink/dir-symlink PathTestFilesSymlink/file diff --git a/src/Corrade/Utility/Test/DirectoryTest.cpp b/src/Corrade/Utility/Test/DirectoryTest.cpp index 7febca96b..543866ab0 100644 --- a/src/Corrade/Utility/Test/DirectoryTest.cpp +++ b/src/Corrade/Utility/Test/DirectoryTest.cpp @@ -1489,9 +1489,10 @@ void DirectoryTest::listNonexistent() { Error redirectError{&out}; CORRADE_COMPARE(Directory::list("nonexistent"), std::vector{}); #ifdef CORRADE_TARGET_WINDOWS - /* Windows has its own code path and thus different errors */ + /* On Windows it's implemented in terms of glob(), thus different prefix, + and because it's WINAPI, also different error code */ CORRADE_COMPARE_AS(out.str(), - "Utility::Path::list(): can't list nonexistent: error 3 (", + "Utility::Path::glob(): can't glob nonexistent/*: error 3 (", TestSuite::Compare::StringHasPrefix); #elif defined(CORRADE_TARGET_EMSCRIPTEN) /* Emscripten uses a different errno for "No such file or directory" */ diff --git a/src/Corrade/Utility/Test/PathTest.cpp b/src/Corrade/Utility/Test/PathTest.cpp index 5f49c3e0a..7d70c504f 100644 --- a/src/Corrade/Utility/Test/PathTest.cpp +++ b/src/Corrade/Utility/Test/PathTest.cpp @@ -164,6 +164,24 @@ struct PathTest: TestSuite::Tester { void listUtf8Result(); void listUtf8Path(); + void globAll(); + void globSingleCharacter(); + void globMultipleCharacters(); + void globDotFiles(); + void globNoMatch(); + void globSkipDirectories(); + void globSkipDirectoriesSymlinks(); + void globSkipFiles(); + void globSkipFilesSymlinks(); + void globSkipSpecial(); + void globSkipSpecialSymlink(); + void globSkipDotAndDotDot(); + void globSort(); + void globNonexistent(); + void globNonNullTerminated(); + void globUtf8Result(); + void globUtf8Path(); + void size(); void sizeEmpty(); void sizeNonSeekable(); @@ -244,6 +262,7 @@ struct PathTest: TestSuite::Tester { void mapWriteUtf8(); Containers::String _testDir, + _testDirGlob, _testDirSymlink, _testDirUtf8, _writeTestDir; @@ -357,6 +376,24 @@ PathTest::PathTest() { &PathTest::listUtf8Result, &PathTest::listUtf8Path, + &PathTest::globAll, + &PathTest::globSingleCharacter, + &PathTest::globMultipleCharacters, + &PathTest::globDotFiles, + &PathTest::globNoMatch, + &PathTest::globSkipDirectories, + &PathTest::globSkipDirectoriesSymlinks, + &PathTest::globSkipFiles, + &PathTest::globSkipFilesSymlinks, + &PathTest::globSkipSpecial, + &PathTest::globSkipSpecialSymlink, + &PathTest::globSkipDotAndDotDot, + &PathTest::globSort, + &PathTest::globNonexistent, + &PathTest::globNonNullTerminated, + &PathTest::globUtf8Result, + &PathTest::globUtf8Path, + &PathTest::size, &PathTest::sizeEmpty, &PathTest::sizeNonSeekable, @@ -453,6 +490,7 @@ PathTest::PathTest() { #endif ) { _testDir = Path::join(Path::split(*Path::executableLocation()).first(), "PathTestFiles"); + _testDirGlob = Path::join(Path::split(*Path::executableLocation()).first(), "PathTestFilesGlob"); _testDirSymlink = Path::join(Path::split(*Path::executableLocation()).first(), "PathTestFilesSymlink"); _testDirUtf8 = Path::join(Path::split(*Path::executableLocation()).first(), "PathTestFilesUtf8"); _writeTestDir = Path::join(*Path::homeDirectory(), "Library/Caches"); @@ -460,6 +498,7 @@ PathTest::PathTest() { #endif { _testDir = Containers::String::nullTerminatedView(PATH_TEST_DIR); + _testDirGlob = Containers::String::nullTerminatedView(PATH_TEST_DIR_GLOB); _testDirSymlink = Containers::String::nullTerminatedView(PATH_TEST_DIR_SYMLINK); _testDirUtf8 = Containers::String::nullTerminatedView(PATH_TEST_DIR_UTF8); _writeTestDir = Containers::String::nullTerminatedView(PATH_WRITE_TEST_DIR); @@ -1891,9 +1930,10 @@ void PathTest::listNonexistent() { Error redirectError{&out}; CORRADE_VERIFY(!Path::list("nonexistent")); #ifdef CORRADE_TARGET_WINDOWS - /* Windows has its own code path and thus different errors */ + /* On Windows it's implemented in terms of glob(), thus different prefix, + and because it's WINAPI, also different error code */ CORRADE_COMPARE_AS(out.str(), - "Utility::Path::list(): can't list nonexistent: error 3 (", + "Utility::Path::glob(): can't glob nonexistent/*: error 3 (", TestSuite::Compare::StringHasPrefix); #elif defined(CORRADE_TARGET_EMSCRIPTEN) /* Emscripten uses a different errno for "No such file or directory" */ @@ -1990,6 +2030,377 @@ void PathTest::listUtf8Path() { TestSuite::Compare::Container); } +void PathTest::globAll() { + Containers::Optional> list = Path::glob(Path::join(_testDirGlob, "*")); + CORRADE_VERIFY(list); + + { + #ifndef CORRADE_TARGET_WINDOWS + CORRADE_COMPARE_AS(*list, Containers::array({ + "dir.txt", "file1.txt", "file11.txt", "file12.txt" + }), TestSuite::Compare::SortedContainer); + #else + CORRADE_COMPARE_AS(*list, Containers::array({ + /* Windows has no special treatment for dotfiles */ + ".", "..", ".dir.txt", ".file.txt", "dir.txt", "file1.txt", "file11.txt", "file12.txt" + }), TestSuite::Compare::SortedContainer); + #endif + } +} + +void PathTest::globSingleCharacter() { + Containers::Optional> list = Path::glob(Path::join(_testDirGlob, "file1?.txt")); + CORRADE_VERIFY(list); + + { + CORRADE_COMPARE_AS(*list, Containers::array({ + "file11.txt", "file12.txt" + }), TestSuite::Compare::SortedContainer); + } +} + +void PathTest::globMultipleCharacters() { + Containers::Optional> list = Path::glob(Path::join(_testDirGlob, "*.txt")); + CORRADE_VERIFY(list); + + { + #ifndef CORRADE_TARGET_WINDOWS + CORRADE_COMPARE_AS(*list, Containers::array({ + "dir.txt", "file1.txt", "file11.txt", "file12.txt" + }), TestSuite::Compare::SortedContainer); + #else + { + CORRADE_EXPECT_FAIL("Somehow, while a * glob pattern works as expected, a *.txt glob pattern omits .dir.txt but not dir.txt or .file.txt."); + CORRADE_COMPARE_AS(*list, Containers::array({ + /* Windows has no special treatment for dotfiles */ + ".dir.txt", ".file.txt", "dir.txt", "file1.txt", "file11.txt", "file12.txt" + }), TestSuite::Compare::SortedContainer); + } { + /* To verify it's at least partially correct */ + CORRADE_COMPARE_AS(*list, Containers::array({ + ".file.txt", "dir.txt", "file1.txt", "file11.txt", "file12.txt" + }), TestSuite::Compare::SortedContainer); + } + #endif + } +} + +void PathTest::globDotFiles() { + Containers::Optional> list = Path::glob(Path::join(_testDirGlob, ".*")); + CORRADE_VERIFY(list); + + { + CORRADE_COMPARE_AS(*list, Containers::array({ + ".", "..", ".dir.txt", ".file.txt" + }), TestSuite::Compare::SortedContainer); + } +} + +void PathTest::globNoMatch() { + /* Unlike globNonexistent(), this shouldn't fail -- the path exists, it + just has no files matching the pattern */ + Containers::Optional> list = Path::glob(Path::join(_testDirGlob, "*.pdf")); + CORRADE_VERIFY(list); + + { + CORRADE_COMPARE_AS(*list, Containers::array({ + }), TestSuite::Compare::SortedContainer); + } +} + +void PathTest::globSkipDirectories() { + Containers::Optional> list = Path::glob(Path::join(_testDirGlob, "*"), Path::GlobFlag::SkipDirectories); + CORRADE_VERIFY(list); + + { + #if defined(CORRADE_TARGET_IOS) && defined(CORRADE_TESTSUITE_TARGET_XCTEST) + CORRADE_EXPECT_FAIL_IF(!std::getenv("SIMULATOR_UDID"), + "iOS (in a simulator) has no idea about file types."); + #endif + #ifndef CORRADE_TARGET_WINDOWS + CORRADE_COMPARE_AS(*list, Containers::array({ + "file1.txt", "file11.txt", "file12.txt" + }), TestSuite::Compare::SortedContainer); + #else + CORRADE_COMPARE_AS(*list, Containers::array({ + /* Windows has no special treatment for dotfiles */ + ".file.txt", "file1.txt", "file11.txt", "file12.txt" + }), TestSuite::Compare::SortedContainer); + #endif + } +} + +void PathTest::globSkipDirectoriesSymlinks() { + Containers::Optional> list = Path::glob(Path::join(_testDirSymlink, "*"), Path::GlobFlag::SkipDirectories); + CORRADE_VERIFY(list); + + { + #if defined(CORRADE_TARGET_IOS) && defined(CORRADE_TESTSUITE_TARGET_XCTEST) + CORRADE_EXPECT_FAIL_IF(!std::getenv("SIMULATOR_UDID"), + "iOS (in a simulator) has no idea about file types."); + #endif + #if !defined(CORRADE_TARGET_UNIX) && !defined(CORRADE_TARGET_EMSCRIPTEN) + /* Possible on Windows too, but there we'd need to first detect if the + Git clone has the symlinks preserved */ + CORRADE_EXPECT_FAIL("Symlink support is implemented on Unix systems and Emscripten only."); + #endif + CORRADE_COMPARE_AS(*list, Containers::array({ + "file", "file-symlink" + }), TestSuite::Compare::SortedContainer); + } +} + +void PathTest::globSkipFiles() { + Containers::Optional> list = Path::glob(Path::join(_testDirGlob, "*"), Path::GlobFlag::SkipFiles); + CORRADE_VERIFY(list); + + { + #if defined(CORRADE_TARGET_IOS) && defined(CORRADE_TESTSUITE_TARGET_XCTEST) + CORRADE_EXPECT_FAIL_IF(!std::getenv("SIMULATOR_UDID"), + "iOS (in a simulator) has no idea about file types."); + #endif + #ifndef CORRADE_TARGET_WINDOWS + CORRADE_COMPARE_AS(*list, Containers::array({ + "dir.txt" + }), TestSuite::Compare::SortedContainer); + #else + CORRADE_COMPARE_AS(*list, Containers::array({ + /* Windows has no special treatment for dotfiles */ + ".", "..", ".dir.txt", "dir.txt" + }), TestSuite::Compare::SortedContainer); + #endif + } +} + +void PathTest::globSkipFilesSymlinks() { + Containers::Optional> list = Path::glob(Path::join(_testDirSymlink, "*"), Path::GlobFlag::SkipFiles); + CORRADE_VERIFY(list); + + { + #if defined(CORRADE_TARGET_IOS) && defined(CORRADE_TESTSUITE_TARGET_XCTEST) + CORRADE_EXPECT_FAIL_IF(!std::getenv("SIMULATOR_UDID"), + "iOS (in a simulator) has no idea about file types."); + #endif + #if !defined(CORRADE_TARGET_UNIX) && !defined(CORRADE_TARGET_EMSCRIPTEN) + /* Possible on Windows too, but there we'd need to first detect if the + Git clone has the symlinks preserved */ + CORRADE_EXPECT_FAIL("Symlink support is implemented on Unix systems and Emscripten only."); + #endif + #ifndef CORRADE_TARGET_WINDOWS + CORRADE_COMPARE_AS(*list, Containers::array({ + "dir", "dir-symlink" + }), TestSuite::Compare::SortedContainer); + #else + CORRADE_COMPARE_AS(*list, Containers::array({ + /* Windows has no special treatment for dotfiles */ + ".", "..", "dir", "dir-symlink" + }), TestSuite::Compare::SortedContainer); + #endif + } +} + +void PathTest::globSkipSpecial() { + Containers::Optional> list = Path::glob(Path::join(_testDirGlob, "*"), Path::GlobFlag::SkipSpecial); + CORRADE_VERIFY(list); + + { + #if defined(CORRADE_TARGET_IOS) && defined(CORRADE_TESTSUITE_TARGET_XCTEST) + CORRADE_EXPECT_FAIL_IF(!std::getenv("SIMULATOR_UDID"), + "iOS (in a simulator) has no idea about file types."); + #endif + #ifndef CORRADE_TARGET_WINDOWS + CORRADE_COMPARE_AS(*list, Containers::array({ + "dir.txt", "file1.txt", "file11.txt", "file12.txt" + }), TestSuite::Compare::SortedContainer); + #else + CORRADE_COMPARE_AS(*list, Containers::array({ + /* Windows has no special treatment for dotfiles */ + ".", "..", ".dir.txt", ".file.txt", "dir.txt", "file1.txt", "file11.txt", "file12.txt" + }), TestSuite::Compare::SortedContainer); + #endif + } +} + +void PathTest::globSkipSpecialSymlink() { + /* Symlinks should not be treated as special files, so they're shown */ + + Containers::Optional> list = Path::glob(Path::join(_testDirSymlink, "*"), Path::GlobFlag::SkipSpecial); + CORRADE_VERIFY(list); + + { + #if defined(CORRADE_TARGET_IOS) && defined(CORRADE_TESTSUITE_TARGET_XCTEST) + CORRADE_EXPECT_FAIL_IF(!std::getenv("SIMULATOR_UDID"), + "iOS (in a simulator) has no idea about file types."); + #endif + #ifndef CORRADE_TARGET_WINDOWS + CORRADE_COMPARE_AS(*list, Containers::array({ + "dir", "dir-symlink", "file", "file-symlink" + }), TestSuite::Compare::SortedContainer); + #else + CORRADE_COMPARE_AS(*list, Containers::array({ + /* Windows has no special treatment for dotfiles */ + ".", "..", "dir", "dir-symlink", "file", "file-symlink" + }), TestSuite::Compare::SortedContainer); + #endif + } +} + +void PathTest::globSkipDotAndDotDot() { + /* Without a leading dot, dot files are completely omitted (on Unix at + least) */ + Containers::Optional> list = Path::glob(Path::join(_testDirGlob, ".*"), Path::GlobFlag::SkipDotAndDotDot); + CORRADE_VERIFY(list); + + { + CORRADE_COMPARE_AS(*list, Containers::array({ + ".dir.txt", ".file.txt" + }), TestSuite::Compare::SortedContainer); + } +} + +void PathTest::globSort() { + { + Containers::Optional> list = Path::glob(Path::join(_testDirGlob, "*"), Path::GlobFlag::SortAscending); + CORRADE_VERIFY(list); + + #ifndef CORRADE_TARGET_WINDOWS + CORRADE_COMPARE_AS(*list, Containers::array({ + "dir.txt", "file1.txt", "file11.txt", "file12.txt" + }), TestSuite::Compare::Container); + #else + CORRADE_COMPARE_AS(*list, Containers::array({ + /* Windows has no special treatment for dotfiles */ + ".", "..", ".dir.txt", ".file.txt", "dir.txt", "file1.txt", "file11.txt", "file12.txt" + }), TestSuite::Compare::SortedContainer); + #endif + } { + Containers::Optional> list = Path::glob(Path::join(_testDirGlob, "*"), Path::GlobFlag::SortDescending); + CORRADE_VERIFY(list); + + #ifndef CORRADE_TARGET_WINDOWS + CORRADE_COMPARE_AS(*list, Containers::array({ + "file12.txt", "file11.txt", "file1.txt", "dir.txt" + }), TestSuite::Compare::Container); + #else + CORRADE_COMPARE_AS(*list, Containers::array({ + /* Windows has no special treatment for dotfiles */ + "file12.txt", "file11.txt", "file1.txt", "dir.txt", ".file.txt", ".dir.txt", "..", "." + }), TestSuite::Compare::SortedContainer); + #endif + } { + /* Ascending and descending together will pick ascending */ + Containers::Optional> list = Path::glob(Path::join(_testDirGlob, "*"), Path::GlobFlag::SortAscending|Path::GlobFlag::SortDescending); + CORRADE_VERIFY(list); + + #ifndef CORRADE_TARGET_WINDOWS + CORRADE_COMPARE_AS(*list, Containers::array({ + "dir.txt", "file1.txt", "file11.txt", "file12.txt" + }), TestSuite::Compare::Container); + #else + CORRADE_COMPARE_AS(*list, Containers::array({ + /* Windows has no special treatment for dotfiles */ + ".", "..", ".dir.txt", ".file.txt", "dir.txt", "file1.txt", "file11.txt", "file12.txt" + }), TestSuite::Compare::SortedContainer); + #endif + } +} + +void PathTest::globNonexistent() { + std::ostringstream out; + Error redirectError{&out}; + /* OTOH, non*existent would pass, returning 0 results. That's tested in + globNoMatch(). */ + CORRADE_VERIFY(!Path::glob("nonexistent/*")); + #ifdef CORRADE_TARGET_WINDOWS + /* Windows has its own code path and thus different errors */ + CORRADE_COMPARE_AS(out.str(), + "Utility::Path::glob(): can't glob nonexistent/*: error 3 (", + TestSuite::Compare::StringHasPrefix); + #elif defined(CORRADE_TARGET_EMSCRIPTEN) + /* Emscripten uses a different errno for "No such file or directory" */ + CORRADE_COMPARE_AS(out.str(), + "Utility::Path::glob(): can't glob nonexistent/*: error 44 (", + TestSuite::Compare::StringHasPrefix); + #else + CORRADE_COMPARE_AS(out.str(), + "Utility::Path::glob(): can't glob nonexistent/*: error 2 (", + TestSuite::Compare::StringHasPrefix); + #endif +} + +void PathTest::globNonNullTerminated() { + Containers::Optional> list = Path::glob(Path::join(_testDirGlob, "*X").exceptSuffix(1)); + CORRADE_VERIFY(list); + + { + #ifndef CORRADE_TARGET_WINDOWS + CORRADE_COMPARE_AS(*list, Containers::array({ + "dir.txt", "file1.txt", "file11.txt", "file12.txt" + }), TestSuite::Compare::SortedContainer); + #else + CORRADE_COMPARE_AS(*list, Containers::array({ + /* Windows has no special treatment for dotfiles */ + ".", "..", ".dir.txt", ".file.txt", "dir.txt", "file1.txt", "file11.txt", "file12.txt" + }), TestSuite::Compare::SortedContainer); + #endif + } +} + +void PathTest::globUtf8Result() { + const Containers::String list[]{"hýždě", "šňůra"}; + + Containers::Optional> actual = Path::glob(Path::join(_testDirUtf8, "*"), Path::GlobFlag::SortAscending); + CORRADE_VERIFY(actual); + + #ifdef CORRADE_TARGET_APPLE + /* Apple HFS+ stores filenames in a decomposed normalized form to avoid + e.g. `e` + `ˇ` and `ě` being treated differently. That makes sense. I + wonder why neither Linux nor Windows do this. */ + const Containers::String listDecomposedUnicode[]{"hýždě", "šňůra"}; + CORRADE_COMPARE_AS(list[1], + listDecomposedUnicode[1], + TestSuite::Compare::NotEqual); + #endif + + #ifdef CORRADE_TARGET_APPLE + /* However, Apple systems still can use filesystems other than HFS+, so + be prepared that it can compare to either */ + if((*actual)[1] == listDecomposedUnicode[1]) { + CORRADE_COMPARE_AS(*actual, + Containers::arrayView(listDecomposedUnicode), + TestSuite::Compare::Container); + } else + #endif + { + #ifndef CORRADE_TARGET_WINDOWS + CORRADE_COMPARE_AS(*actual, + Containers::arrayView(list), + TestSuite::Compare::Container); + #else + CORRADE_COMPARE_AS(*actual, Containers::array({ + /* Windows has no special treatment for dotfiles */ + ".", "..", "hýždě", "šňůra" + }), TestSuite::Compare::SortedContainer); + #endif + } +} + +void PathTest::globUtf8Path() { + Containers::Optional> list = Path::glob(Path::join({_testDirUtf8, "šňůra", "*"}), Path::GlobFlag::SortAscending); + CORRADE_VERIFY(list); + + #ifndef CORRADE_TARGET_WINDOWS + CORRADE_COMPARE_AS(*list, Containers::array({ + "dummy", "klíče" + }), TestSuite::Compare::SortedContainer); + #else + CORRADE_COMPARE_AS(*list, Containers::array({ + /* Windows has no special treatment for dotfiles */ + ".", "..", "dummy", "klíče" + }), TestSuite::Compare::SortedContainer); + #endif +} + /* Checks if we are reading it as binary (CR+LF is not converted to LF), nothing after \0 gets lost, and invalid UTF-8 chars (over 0x80) also cause no issues */ diff --git a/src/Corrade/Utility/Test/PathTestFilesGlob/.dir.txt/dummy b/src/Corrade/Utility/Test/PathTestFilesGlob/.dir.txt/dummy new file mode 100644 index 000000000..e69de29bb diff --git a/src/Corrade/Utility/Test/PathTestFilesGlob/.file.txt b/src/Corrade/Utility/Test/PathTestFilesGlob/.file.txt new file mode 100644 index 000000000..e69de29bb diff --git a/src/Corrade/Utility/Test/PathTestFilesGlob/dir.txt/dummy b/src/Corrade/Utility/Test/PathTestFilesGlob/dir.txt/dummy new file mode 100644 index 000000000..e69de29bb diff --git a/src/Corrade/Utility/Test/PathTestFilesGlob/file1.txt b/src/Corrade/Utility/Test/PathTestFilesGlob/file1.txt new file mode 100644 index 000000000..e69de29bb diff --git a/src/Corrade/Utility/Test/PathTestFilesGlob/file11.txt b/src/Corrade/Utility/Test/PathTestFilesGlob/file11.txt new file mode 100644 index 000000000..e69de29bb diff --git a/src/Corrade/Utility/Test/PathTestFilesGlob/file12.txt b/src/Corrade/Utility/Test/PathTestFilesGlob/file12.txt new file mode 100644 index 000000000..e69de29bb diff --git a/src/Corrade/Utility/Test/configure.h.cmake b/src/Corrade/Utility/Test/configure.h.cmake index da3475db6..93d770f70 100644 --- a/src/Corrade/Utility/Test/configure.h.cmake +++ b/src/Corrade/Utility/Test/configure.h.cmake @@ -31,6 +31,7 @@ #define JSONWRITER_TEST_DIR "${UTILITY_BINARY_TEST_DIR}/JsonWriterTestFiles" #define PATH_TEST_DIR "${UTILITY_TEST_DIR}/PathTestFiles" +#define PATH_TEST_DIR_GLOB "${UTILITY_TEST_DIR}/PathTestFilesGlob" #define PATH_TEST_DIR_SYMLINK "${UTILITY_TEST_DIR}/PathTestFilesSymlink" #define PATH_TEST_DIR_UTF8 "${UTILITY_TEST_DIR}/PathTestFilesUtf8" #define PATH_WRITE_TEST_DIR "${UTILITY_BINARY_TEST_DIR}/PathTestFiles" From 396a8ec062d8142328e53d68c616334cf383654a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 2 Apr 2022 17:09:37 +0200 Subject: [PATCH 2/7] Utility: work around a musl bug discovered when testing Path::glob(). Sigh. Not one day passes where I would just carry on coding against APIs that are neither crap nor buggy. --- src/Corrade/Utility/Path.cpp | 19 +++++++++++++++++-- src/Corrade/Utility/Test/PathTest.cpp | 8 ++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/Corrade/Utility/Path.cpp b/src/Corrade/Utility/Path.cpp index 4f31af5cf..cd05e2ca7 100644 --- a/src/Corrade/Utility/Path.cpp +++ b/src/Corrade/Utility/Path.cpp @@ -800,9 +800,24 @@ Containers::Optional> glob(const Container if(!(flags & (GlobFlag::SortAscending|GlobFlag::SortDescending))) globFlags |= GLOB_NOSORT; glob_t out; + /* With musl (and with Emscripten that uses musl), an error from opening a + directory (such as when globbing `nonexistent/``*`) that happens at: + https://github.com/bminor/musl/blob/6d8a515796270eb6cec8a278cb353a078a10f09a/src/regex/glob.c#L127-L130 + gets ignored by the code path that haldes the case of no matches, and so + we get GLOB_NOMATCH instead: + https://github.com/bminor/musl/blob/6d8a515796270eb6cec8a278cb353a078a10f09a/src/regex/glob.c#L261-L270 + Fortunately the opendir() failure sets errno which we can subsequently + check... unless it's a version before + https://github.com/emscripten-core/emscripten/commit/e05e72d9c49fe15578e73934ce525a894d1b712a + which apparently neither sets the errno nor calls the error handler, so + we have no way to check anything. Apart from that, this is still not + 100% bulletproof as is possible that something else would set errno even + in the successful scenario. */ + errno = 0; const int result = glob(Containers::String::nullTerminatedView(pattern).data(), globFlags, nullptr, &out); - /* Having no results is fine, having an error is not */ - if(result == GLOB_NOMATCH) + /* Having no results is fine, but only if we're not under musl that has the + bug explained above, so check errno as well */ + if(result == GLOB_NOMATCH && !errno) return Containers::Array{}; if(result != 0) { Error err; diff --git a/src/Corrade/Utility/Test/PathTest.cpp b/src/Corrade/Utility/Test/PathTest.cpp index 7d70c504f..b08c80de1 100644 --- a/src/Corrade/Utility/Test/PathTest.cpp +++ b/src/Corrade/Utility/Test/PathTest.cpp @@ -54,6 +54,10 @@ #include "Corrade/Utility/System.h" /* isSandboxed() */ #endif +#ifdef CORRADE_TARGET_EMSCRIPTEN +#include /* XFAIL in globNonexistent() */ +#endif + namespace Corrade { namespace Utility { namespace Test { namespace { struct PathTest: TestSuite::Tester { @@ -2308,6 +2312,10 @@ void PathTest::globSort() { void PathTest::globNonexistent() { std::ostringstream out; Error redirectError{&out}; + #if defined(CORRADE_TARGET_EMSCRIPTEN) && __EMSCRIPTEN_major__ < 3 + /* https://github.com/emscripten-core/emscripten/commit/e05e72d9c49fe15578e73934ce525a894d1b712a */ + CORRADE_EXPECT_FAIL("Emscripten < 3.0.0 has an old musl which doesn't seem to correctly propagate the error from opendir()."); + #endif /* OTOH, non*existent would pass, returning 0 results. That's tested in globNoMatch(). */ CORRADE_VERIFY(!Path::glob("nonexistent/*")); From 9cc544a412aa2f88daecab1cfbb7f1fea9462f17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 2 Apr 2022 18:44:47 +0200 Subject: [PATCH 3/7] Utility: Windows conflates "doesn't exist" and "no matches" errors. Unfortunate. I don't want the application to complain give me the same error when a directory doesn't contain any *.txt files or when the directory doesn't exist at all. The Path::glob() tests now fully pass. --- src/Corrade/Utility/Path.cpp | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/Corrade/Utility/Path.cpp b/src/Corrade/Utility/Path.cpp index cd05e2ca7..d2a7732f4 100644 --- a/src/Corrade/Utility/Path.cpp +++ b/src/Corrade/Utility/Path.cpp @@ -869,9 +869,35 @@ Containers::Optional> glob(const Container WIN32_FIND_DATAW data; HANDLE hFile = FindFirstFileW(Unicode::widen(pattern), &data); if(hFile == INVALID_HANDLE_VALUE) { + /* If we're matching `path/``*` and it returns ERROR_FILE_NOT_FOUND, it + means the path doesn't exist -- otherwise it would return at least a + `..`. This is a valid reason to error. + + If we're however matching e.g. `path/``*.txt` and it returns + ERROR_FILE_NOT_FOUND, we should not error if `path` is an existing directory that just doesn't contain any text files. So then we + subsequently check if the path is a directory, and if it is, we + return an empty list, consistently with the Unix implementation + above. */ + const unsigned int error = GetLastError(); + if(error == ERROR_FILE_NOT_FOUND) { + const Containers::Pair pathFilename = split(pattern); + /* Yes, this performs a second UTF-8 -> UTF-16 conversion + internally, because we need a smaller null-terminated string. An + alternative would be to cache the Unicode::widen(pattern) from + above, then somehow add a 16-bit zero to where the last 16-bit / + is, and then call the Windows API directly, but who wants to + write extra tests for all that? I don't. */ + if(pathFilename.second() != "*"_s && isDirectory(pathFilename.first())) + return Containers::Array{}; + } + Error err; err << "Utility::Path::glob(): can't glob" << pattern << Debug::nospace << ":"; - Utility::Implementation::printWindowsErrorString(err, GetLastError()); + /* Using the cached error from above and not GetLastError() in case + the isDirectory() call would produce another error internally. The + isDirectory() call also shouldn't be printing anything on its + own. */ + Utility::Implementation::printWindowsErrorString(err, error); return {}; } Containers::ScopeGuard closeHandle{hFile, From 5e3b5313a4aaf4e13698ab5ee94db8dd9024163c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 2 Apr 2022 20:31:56 +0200 Subject: [PATCH 4/7] Utility: repro case for Path::glob() where Windows matches 8.3 filenames. Wonderful, eh? The file.txtlol becomes something like FILE~1.TXT, with only three letters from the extension used and the rest ignored. Discovered completely by accident while trying to port PluginManager plugin discovery to Path::glob() and it matched *.modconf files where it should be matching just *.mod. --- src/Corrade/Utility/Test/CMakeLists.txt | 1 + src/Corrade/Utility/Test/PathTest.cpp | 28 +++++++++---------- .../Test/PathTestFilesGlob/file.txtlol | 0 3 files changed, 15 insertions(+), 14 deletions(-) create mode 100644 src/Corrade/Utility/Test/PathTestFilesGlob/file.txtlol diff --git a/src/Corrade/Utility/Test/CMakeLists.txt b/src/Corrade/Utility/Test/CMakeLists.txt index 1aeda24d3..cd494ea47 100644 --- a/src/Corrade/Utility/Test/CMakeLists.txt +++ b/src/Corrade/Utility/Test/CMakeLists.txt @@ -237,6 +237,7 @@ corrade_add_test(UtilityPathTest ${UtilityPathTest_SRCS} PathTestFiles/dir/dummy PathTestFiles/file PathTestFilesGlob/.file.txt + PathTestFilesGlob/file.txtlol PathTestFilesGlob/file1.txt PathTestFilesGlob/file11.txt PathTestFilesGlob/file12.txt diff --git a/src/Corrade/Utility/Test/PathTest.cpp b/src/Corrade/Utility/Test/PathTest.cpp index b08c80de1..5af658c7d 100644 --- a/src/Corrade/Utility/Test/PathTest.cpp +++ b/src/Corrade/Utility/Test/PathTest.cpp @@ -2041,12 +2041,12 @@ void PathTest::globAll() { { #ifndef CORRADE_TARGET_WINDOWS CORRADE_COMPARE_AS(*list, Containers::array({ - "dir.txt", "file1.txt", "file11.txt", "file12.txt" + "dir.txt", "file.txtlol", "file1.txt", "file11.txt", "file12.txt" }), TestSuite::Compare::SortedContainer); #else CORRADE_COMPARE_AS(*list, Containers::array({ /* Windows has no special treatment for dotfiles */ - ".", "..", ".dir.txt", ".file.txt", "dir.txt", "file1.txt", "file11.txt", "file12.txt" + ".", "..", ".dir.txt", ".file.txt", "dir.txt", "file.txtlol", "file1.txt", "file11.txt", "file12.txt" }), TestSuite::Compare::SortedContainer); #endif } @@ -2123,12 +2123,12 @@ void PathTest::globSkipDirectories() { #endif #ifndef CORRADE_TARGET_WINDOWS CORRADE_COMPARE_AS(*list, Containers::array({ - "file1.txt", "file11.txt", "file12.txt" + "file.txtlol", "file1.txt", "file11.txt", "file12.txt" }), TestSuite::Compare::SortedContainer); #else CORRADE_COMPARE_AS(*list, Containers::array({ /* Windows has no special treatment for dotfiles */ - ".file.txt", "file1.txt", "file11.txt", "file12.txt" + ".file.txt", "file.txtlol", "file1.txt", "file11.txt", "file12.txt" }), TestSuite::Compare::SortedContainer); #endif } @@ -2214,12 +2214,12 @@ void PathTest::globSkipSpecial() { #endif #ifndef CORRADE_TARGET_WINDOWS CORRADE_COMPARE_AS(*list, Containers::array({ - "dir.txt", "file1.txt", "file11.txt", "file12.txt" + "dir.txt", "file.txtlol", "file1.txt", "file11.txt", "file12.txt" }), TestSuite::Compare::SortedContainer); #else CORRADE_COMPARE_AS(*list, Containers::array({ /* Windows has no special treatment for dotfiles */ - ".", "..", ".dir.txt", ".file.txt", "dir.txt", "file1.txt", "file11.txt", "file12.txt" + ".", "..", ".dir.txt", ".file.txt", "dir.txt", "file.txtlol", "file1.txt", "file11.txt", "file12.txt" }), TestSuite::Compare::SortedContainer); #endif } @@ -2269,12 +2269,12 @@ void PathTest::globSort() { #ifndef CORRADE_TARGET_WINDOWS CORRADE_COMPARE_AS(*list, Containers::array({ - "dir.txt", "file1.txt", "file11.txt", "file12.txt" + "dir.txt", "file.txtlol", "file1.txt", "file11.txt", "file12.txt" }), TestSuite::Compare::Container); #else CORRADE_COMPARE_AS(*list, Containers::array({ /* Windows has no special treatment for dotfiles */ - ".", "..", ".dir.txt", ".file.txt", "dir.txt", "file1.txt", "file11.txt", "file12.txt" + ".", "..", ".dir.txt", ".file.txt", "dir.txt", "file.txtlol", "file1.txt", "file11.txt", "file12.txt" }), TestSuite::Compare::SortedContainer); #endif } { @@ -2283,12 +2283,12 @@ void PathTest::globSort() { #ifndef CORRADE_TARGET_WINDOWS CORRADE_COMPARE_AS(*list, Containers::array({ - "file12.txt", "file11.txt", "file1.txt", "dir.txt" + "file12.txt", "file11.txt", "file1.txt", "file.txtlol", "dir.txt" }), TestSuite::Compare::Container); #else CORRADE_COMPARE_AS(*list, Containers::array({ /* Windows has no special treatment for dotfiles */ - "file12.txt", "file11.txt", "file1.txt", "dir.txt", ".file.txt", ".dir.txt", "..", "." + "file12.txt", "file11.txt", "file1.txt", "file.txtlol", "dir.txt", ".file.txt", ".dir.txt", "..", "." }), TestSuite::Compare::SortedContainer); #endif } { @@ -2298,12 +2298,12 @@ void PathTest::globSort() { #ifndef CORRADE_TARGET_WINDOWS CORRADE_COMPARE_AS(*list, Containers::array({ - "dir.txt", "file1.txt", "file11.txt", "file12.txt" + "dir.txt", "file.txtlol", "file1.txt", "file11.txt", "file12.txt" }), TestSuite::Compare::Container); #else CORRADE_COMPARE_AS(*list, Containers::array({ /* Windows has no special treatment for dotfiles */ - ".", "..", ".dir.txt", ".file.txt", "dir.txt", "file1.txt", "file11.txt", "file12.txt" + ".", "..", ".dir.txt", ".file.txt", "dir.txt", "file.txtlol", "file1.txt", "file11.txt", "file12.txt" }), TestSuite::Compare::SortedContainer); #endif } @@ -2343,12 +2343,12 @@ void PathTest::globNonNullTerminated() { { #ifndef CORRADE_TARGET_WINDOWS CORRADE_COMPARE_AS(*list, Containers::array({ - "dir.txt", "file1.txt", "file11.txt", "file12.txt" + "dir.txt", "file.txtlol", "file1.txt", "file11.txt", "file12.txt" }), TestSuite::Compare::SortedContainer); #else CORRADE_COMPARE_AS(*list, Containers::array({ /* Windows has no special treatment for dotfiles */ - ".", "..", ".dir.txt", ".file.txt", "dir.txt", "file1.txt", "file11.txt", "file12.txt" + ".", "..", ".dir.txt", ".file.txt", "dir.txt", "file.txtlol", "file1.txt", "file11.txt", "file12.txt" }), TestSuite::Compare::SortedContainer); #endif } diff --git a/src/Corrade/Utility/Test/PathTestFilesGlob/file.txtlol b/src/Corrade/Utility/Test/PathTestFilesGlob/file.txtlol new file mode 100644 index 000000000..e69de29bb From 0e46fc908a2cf9a6456603e2daaca994a73c921b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 2 Apr 2022 20:53:29 +0200 Subject: [PATCH 5/7] [wip] Utility: disable Windows 8.3 filename matching in Path::glob(). If someone told me, 10 hours ago, that I would run into problems like this, I would just say fuck it and not even attempt to implement this function. Well, here we are. TODO: AND OF COURSE THIS CHANGE DOESN'T HAVE ANY EFFECT! FFS!! TODO: so i guess here i'd need to implement this BY HAND?! Even though I'm forced to pass * to the damn thing to list stuff in the first place?! God fucking dammit. TODO: https://devblogs.microsoft.com/oldnewthing/20050720-16/?p=34883 HEY RAYMOND INSTEAD OF SAYING "CAN'T DO" OR "IMPOSSIBLE", WHY COULDN'T YOU JUST ADD ONE MORE FLAG TO THE EX FUNCTION?? IT'S BEEN 17 YEARS SINCE THAT POST!!! --- src/Corrade/Utility/Path.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Corrade/Utility/Path.cpp b/src/Corrade/Utility/Path.cpp index d2a7732f4..c02dfd86f 100644 --- a/src/Corrade/Utility/Path.cpp +++ b/src/Corrade/Utility/Path.cpp @@ -867,7 +867,14 @@ Containers::Optional> glob(const Container /* Windows (not Store/Phone) */ #elif defined(CORRADE_TARGET_WINDOWS) && !defined(CORRADE_TARGET_WINDOWS_RT) WIN32_FIND_DATAW data; - HANDLE hFile = FindFirstFileW(Unicode::widen(pattern), &data); + /* Originally this used FindFirstFileW(), but that one implicitly includes + 8.3 filenames in the match. That sounded just like an unnecessary but + harmless work at first, but turns out it affects how files with + extensions longer than 3 characters are processed. So if globbing for + `*.txt` and there's a `*.txtlol` file, it'd match it as well, because + its 8.3 form is something like FILE~1.TXT. Haha. */ + /** @todo there's also FIND_FIRST_EX_CASE_SENSITIVE, expose? */ + HANDLE hFile = FindFirstFileExW(Unicode::widen(pattern), FindExInfoBasic, &data, FindExSearchNameMatch, nullptr, 0); if(hFile == INVALID_HANDLE_VALUE) { /* If we're matching `path/``*` and it returns ERROR_FILE_NOT_FOUND, it means the path doesn't exist -- otherwise it would return at least a From 7a991ccd2c9aadff0bc86d3cff23263dcd295c1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 2 Apr 2022 18:50:50 +0200 Subject: [PATCH 6/7] PluginManager: use Path::glob() instead of list() for plugin discovery. Halves the number of string allocations as we're skipping all *.conf files this way. --- src/Corrade/PluginManager/AbstractManager.cpp | 13 ++++--------- src/Corrade/PluginManager/Test/ManagerTest.cpp | 2 +- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/Corrade/PluginManager/AbstractManager.cpp b/src/Corrade/PluginManager/AbstractManager.cpp index 001336486..1906b51bf 100644 --- a/src/Corrade/PluginManager/AbstractManager.cpp +++ b/src/Corrade/PluginManager/AbstractManager.cpp @@ -428,16 +428,11 @@ void AbstractManager::setPluginDirectory(const Containers::StringView directory) when the directory doesn't exist, as a lot of existing code and tests relies on it. Figure out a better solution. */ if(Utility::Path::exists(_state->pluginDirectory)) { - Containers::Optional> d = Utility::Path::list( - _state->pluginDirectory, - Utility::Path::ListFlag::SkipDirectories| - Utility::Path::ListFlag::SkipDotAndDotDot| - Utility::Path::ListFlag::SortAscending); + Containers::Optional> d = Utility::Path::glob( + Utility::Path::join(_state->pluginDirectory, "*"_s + _state->pluginSuffix), + Utility::Path::GlobFlag::SkipDirectories| + Utility::Path::GlobFlag::SortAscending); if(d) for(const Containers::StringView filename: *d) { - /* File doesn't have module suffix, continue to next */ - if(!filename.hasSuffix(_state->pluginSuffix)) - continue; - /* Dig plugin name from filename */ const Containers::StringView name = filename.exceptSuffix(_state->pluginSuffix); diff --git a/src/Corrade/PluginManager/Test/ManagerTest.cpp b/src/Corrade/PluginManager/Test/ManagerTest.cpp index 693cb9cbe..a64c2200c 100644 --- a/src/Corrade/PluginManager/Test/ManagerTest.cpp +++ b/src/Corrade/PluginManager/Test/ManagerTest.cpp @@ -273,7 +273,7 @@ void ManagerTest::pluginDirectoryNotReadable() { PluginManager::Manager manager{directory}; } CORRADE_COMPARE_AS(out.str(), - Utility::formatString("Utility::Path::list(): can't list {}: error ", directory), + Utility::formatString("Utility::Path::glob(): can't glob {}/*{}: error ", directory, AbstractPlugin::pluginSuffix()), TestSuite::Compare::StringHasPrefix); } From eaf9d9279e6379b9913c26fd7ae42885d8ff05df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 2 Apr 2022 20:14:11 +0200 Subject: [PATCH 7/7] [wip] --- src/Corrade/PluginManager/AbstractManager.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Corrade/PluginManager/AbstractManager.cpp b/src/Corrade/PluginManager/AbstractManager.cpp index 1906b51bf..865a2934b 100644 --- a/src/Corrade/PluginManager/AbstractManager.cpp +++ b/src/Corrade/PluginManager/AbstractManager.cpp @@ -432,6 +432,7 @@ void AbstractManager::setPluginDirectory(const Containers::StringView directory) Utility::Path::join(_state->pluginDirectory, "*"_s + _state->pluginSuffix), Utility::Path::GlobFlag::SkipDirectories| Utility::Path::GlobFlag::SortAscending); + if(d) !Utility::Debug{} << d; if(d) for(const Containers::StringView filename: *d) { /* Dig plugin name from filename */ const Containers::StringView name = filename.exceptSuffix(_state->pluginSuffix);