aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar Laszlo Csomor <laszlocsomor@google.com>2017-02-16 18:11:19 +0000
committerGravatar Dmitry Lomov <dslomov@google.com>2017-02-17 14:51:57 +0000
commit748f8af16b2648faa9569ae7bc77d93a7a2fbd67 (patch)
treea0038e8dd57bf1f0134e156f399476f5c55a6c0d
parent1210a2053fd44497bc04bba0bbd434d907cfe486 (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/BUILD4
-rw-r--r--src/main/cpp/util/file_posix.cc6
-rw-r--r--src/main/cpp/util/file_windows.cc40
-rw-r--r--src/test/cpp/util/file_test.cc53
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