diff options
author | 2017-02-16 18:11:19 +0000 | |
---|---|---|
committer | 2017-02-17 14:51:57 +0000 | |
commit | 748f8af16b2648faa9569ae7bc77d93a7a2fbd67 (patch) | |
tree | a0038e8dd57bf1f0134e156f399476f5c55a6c0d | |
parent | 1210a2053fd44497bc04bba0bbd434d907cfe486 (diff) |
Bazel client, Windows: impl. ForEachDirectoryEntry
This is the last function we needed from
file_posix for MSYS, so now we can remove that
from the compilation.
Fixes https://github.com/bazelbuild/bazel/issues/2386
The problem originally was that I used CloseHandle
to close the HANDLE, instead of using FindClose,
so we were holding on to open directory HANDLEs,
so we couldn't rename the installation directory.
See https://github.com/bazelbuild/bazel/issues/2107
--
Change-Id: If6c1b3c99cf386d82e829dbee2323e6bce5f6b46
Reviewed-on: https://cr.bazel.build/8952
PiperOrigin-RevId: 147734165
MOS_MIGRATED_REVID=147734165
-rw-r--r-- | src/main/cpp/util/BUILD | 4 | ||||
-rw-r--r-- | src/main/cpp/util/file_posix.cc | 6 | ||||
-rw-r--r-- | src/main/cpp/util/file_windows.cc | 40 | ||||
-rw-r--r-- | src/test/cpp/util/file_test.cc | 53 |
4 files changed, 88 insertions, 15 deletions
diff --git a/src/main/cpp/util/BUILD b/src/main/cpp/util/BUILD index b044d19537..b5c7b79d54 100644 --- a/src/main/cpp/util/BUILD +++ b/src/main/cpp/util/BUILD @@ -27,10 +27,6 @@ cc_library( name = "file", srcs = ["file.cc"] + select({ "//src:windows": [ - # TODO(bazel-team): implement functions in file_windows.cc and use - # more and more of them under MSYS until we can completely stop - # using file_posix.cc at which point remove it from this list. - "file_posix.cc", "file_windows.cc", ], "//src:windows_msvc": [ diff --git a/src/main/cpp/util/file_posix.cc b/src/main/cpp/util/file_posix.cc index b59618201b..30fd8cd9e3 100644 --- a/src/main/cpp/util/file_posix.cc +++ b/src/main/cpp/util/file_posix.cc @@ -34,11 +34,6 @@ namespace blaze_util { using std::pair; using std::string; -// TODO(bazel-team): implement all functions in file_windows.cc, use them from -// MSYS, remove file_posix.cc from the `srcs` of -// //src/main/cpp/util:file when building for MSYS, and remove all -// #ifndef __CYGWIN__ directives. -#ifndef __CYGWIN__ // Runs "stat" on `path`. Returns -1 and sets errno if stat fails or // `path` isn't a directory. If check_perms is true, this will also // make sure that `path` is owned by the current user and has `mode` @@ -371,7 +366,6 @@ string GetCwd() { bool ChangeDirectory(const string& path) { return chdir(path.c_str()) == 0; } -#endif // not __CYGWIN__ void ForEachDirectoryEntry(const string &path, DirectoryEntryConsumer *consume) { diff --git a/src/main/cpp/util/file_windows.cc b/src/main/cpp/util/file_windows.cc index c92b76ba7c..26cc47df65 100644 --- a/src/main/cpp/util/file_windows.cc +++ b/src/main/cpp/util/file_windows.cc @@ -1074,14 +1074,44 @@ bool ChangeDirectory(const string& path) { return true; } -#ifdef COMPILER_MSVC void ForEachDirectoryEntry(const string &path, DirectoryEntryConsumer *consume) { - // TODO(bazel-team): implement this. - pdie(255, "blaze_util::ForEachDirectoryEntry is not implemented on Windows"); + wstring wpath; + if (path.empty() || IsDevNull(path)) { + return; + } + if (!AsWindowsPath(path, &wpath)) { + pdie(blaze_exit_code::LOCAL_ENVIRONMENTAL_ERROR, + "ForEachDirectoryEntry(%s): AsWindowsPath failed", path.c_str()); + } + + static const wstring kUncPrefix(L"\\\\?\\"); + static const wstring kDot(L"."); + static const wstring kDotDot(L".."); + // Always add an UNC prefix to ensure we can work with long paths. + if (!windows_util::HasUncPrefix(wpath.c_str())) { + wpath = kUncPrefix + wpath; + } + // Unconditionally add a trailing backslash. We know `wpath` has no trailing + // backslash because it comes from AsWindowsPath whose output is always + // normalized (see NormalizeWindowsPath). + wpath.append(L"\\"); + WIN32_FIND_DATAW metadata; + HANDLE handle = ::FindFirstFileW((wpath + L"*").c_str(), &metadata); + if (handle == INVALID_HANDLE_VALUE) { + return; // directory does not exist or is empty + } + + do { + if (kDot != metadata.cFileName && kDotDot != metadata.cFileName) { + wstring wname = wpath + metadata.cFileName; + string name(WstringToCstring(/* omit prefix */ 4 + wname.c_str()).get()); + bool is_dir = (metadata.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0; + consume->Consume(name, is_dir); + } + } while (::FindNextFileW(handle, &metadata)); + ::FindClose(handle); } -#else // not COMPILER_MSVC -#endif // COMPILER_MSVC string NormalizeWindowsPath(string path) { if (path.empty()) { diff --git a/src/test/cpp/util/file_test.cc b/src/test/cpp/util/file_test.cc index ab9ce916d1..4173954559 100644 --- a/src/test/cpp/util/file_test.cc +++ b/src/test/cpp/util/file_test.cc @@ -14,8 +14,11 @@ #include <stdio.h> #include <string.h> +#include <algorithm> +#include <map> #include <memory> // unique_ptr #include <thread> // NOLINT (to silence Google-internal linter) +#include <vector> #include "src/main/cpp/util/file.h" #include "src/main/cpp/util/file_platform.h" @@ -164,4 +167,54 @@ TEST(FileTest, TestRenameDirectory) { ASSERT_EQ(RenameDirectory(dir2, dir1), kRenameDirectoryFailureNotEmpty); } +class CollectingDirectoryEntryConsumer : public DirectoryEntryConsumer { + public: + CollectingDirectoryEntryConsumer(const string& _rootname) + : rootname(_rootname) {} + + void Consume(const string& name, bool is_directory) override { + // Strip the path prefix up to the `rootname` to ease testing on all + // platforms. + int index = name.rfind(rootname); + string key = (index == string::npos) ? name : name.substr(index); + // Replace backslashes with forward slashes (necessary on Windows only). + std::replace(key.begin(), key.end(), '\\', '/'); + entries[key] = is_directory; + } + + const string rootname; + std::map<string, bool> entries; +}; + +TEST(FileTest, ForEachDirectoryEntryTest) { + string tmpdir(getenv("TEST_TMPDIR")); + EXPECT_FALSE(tmpdir.empty()); + // Create a directory structure: + // $TEST_TMPDIR/ + // foo/ + // bar/ + // file3.txt + // file1.txt + // file2.txt + string rootdir(JoinPath(tmpdir, "foo")); + string file1(JoinPath(rootdir, "file1.txt")); + string file2(JoinPath(rootdir, "file2.txt")); + string subdir(JoinPath(rootdir, "bar")); + string file3(JoinPath(subdir, "file3.txt")); + + EXPECT_TRUE(MakeDirectories(subdir, 0700)); + EXPECT_TRUE(WriteFile("hello", 5, file1)); + EXPECT_TRUE(WriteFile("hello", 5, file2)); + EXPECT_TRUE(WriteFile("hello", 5, file3)); + + std::map<string, bool> expected; + expected["foo/file1.txt"] = false; + expected["foo/file2.txt"] = false; + expected["foo/bar"] = true; + + CollectingDirectoryEntryConsumer consumer("foo"); + ForEachDirectoryEntry(rootdir, &consumer); + ASSERT_EQ(consumer.entries, expected); +} + } // namespace blaze_util |