aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java8
-rw-r--r--src/main/native/windows/file-jni.cc2
-rw-r--r--src/main/native/windows/file.cc89
-rw-r--r--src/main/native/windows/file.h14
-rw-r--r--src/test/native/windows/file_test.cc27
5 files changed, 89 insertions, 51 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java
index 229c207e91..4b5d74f701 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsFileOperations.java
@@ -61,10 +61,10 @@ public class WindowsFileOperations {
// Keep DELETE_PATH_* values in sync with src/main/native/windows/file.cc.
private static final int DELETE_PATH_SUCCESS = 0;
- private static final int DELETE_PATH_DOES_NOT_EXIST = 1;
- private static final int DELETE_PATH_DIRECTORY_NOT_EMPTY = 2;
- private static final int DELETE_PATH_ACCESS_DENIED = 3;
- private static final int DELETE_PATH_ERROR = 4;
+ private static final int DELETE_PATH_ERROR = 1;
+ private static final int DELETE_PATH_DOES_NOT_EXIST = 2;
+ private static final int DELETE_PATH_DIRECTORY_NOT_EMPTY = 3;
+ private static final int DELETE_PATH_ACCESS_DENIED = 4;
private static native int nativeIsJunction(String path, String[] error);
diff --git a/src/main/native/windows/file-jni.cc b/src/main/native/windows/file-jni.cc
index 0f3334c517..0dc4fbd7ab 100644
--- a/src/main/native/windows/file-jni.cc
+++ b/src/main/native/windows/file-jni.cc
@@ -98,7 +98,7 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsFileOperations_nativeDelet
std::wstring wpath(bazel::windows::GetJavaWstring(env, path));
std::wstring error;
int result = bazel::windows::DeletePath(wpath, &error);
- if (result != bazel::windows::DELETE_PATH_SUCCESS && !error.empty() &&
+ if (result != bazel::windows::DeletePathResult::kSuccess && !error.empty() &&
CanReportError(env, error_msg_holder)) {
ReportLastError(
bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__,
diff --git a/src/main/native/windows/file.cc b/src/main/native/windows/file.cc
index c020eebd24..c46744a2c2 100644
--- a/src/main/native/windows/file.cc
+++ b/src/main/native/windows/file.cc
@@ -154,22 +154,25 @@ int CreateJunction(const wstring& junction_name, const wstring& junction_target,
}
AutoHandle handle(CreateFileW(
- name.c_str(), GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING,
- FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS, NULL));
+ name.c_str(), GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ, NULL,
+ OPEN_EXISTING, FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS,
+ NULL));
if (!handle.IsValid()) {
DWORD err = GetLastError();
// We can't open the directory for writing: either it disappeared, or turned
// into a file, or another process holds it open without write-sharing.
- // Either way, don't try to create the junction, just try opening it for
- // reading and check its value.
+ // Either way, don't try to create the junction, just try opening it without
+ // any read or write access (we can still read its metadata) and maximum
+ // sharing, and check its target.
create = false;
handle = CreateFileW(
- name.c_str(), GENERIC_READ, 0, NULL, OPEN_EXISTING,
+ name.c_str(), 0, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+ NULL, OPEN_EXISTING,
FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS, NULL);
if (!handle.IsValid()) {
- // We can't open the directory even for reading: either it disappeared, or
- // it turned into a file, or another process holds it open without
- // read-sharing. Give up.
+ // We can't open the directory at all: either it disappeared, or it turned
+ // into a file, or another process holds it open without any sharing.
+ // Give up.
DWORD err = GetLastError();
if (err == ERROR_SHARING_VIOLATION) {
// The junction is held open by another process.
@@ -226,7 +229,23 @@ int CreateJunction(const wstring& junction_name, const wstring& junction_target,
create = false;
if (!(info.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)) {
// The path is no longer a directory, and it's not a junction either.
- return CreateJunctionResult::kAlreadyExistsButNotJunction;
+ // Though this is a case for kAlreadyExistsButNotJunction, let's instead
+ // print the attributes and return kError, to give more information to
+ // the user.
+ if (error) {
+ // Print the attributes as a 32-bit hexadecimal number.
+ WCHAR attr_str[9];
+ DWORD attr_value = info.dwFileAttributes;
+ attr_str[8] = 0;
+ for (int i = 0; i < 8; ++i) {
+ attr_str[7 - i] = L"0123456789abcdef"[attr_value & 0xF];
+ attr_value >>= 4;
+ }
+ *error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
+ L"GetFileInformationByHandle", name,
+ wstring(L"attrs=0x") + attr_str);
+ }
+ return CreateJunctionResult::kError;
}
}
}
@@ -334,18 +353,18 @@ int DeletePath(const wstring& path, wstring* error) {
DWORD err = GetLastError();
if (err == ERROR_SHARING_VIOLATION) {
// The file or directory is in use by some process.
- return DELETE_PATH_ACCESS_DENIED;
+ return DeletePathResult::kAccessDenied;
} else if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) {
// The file or directory does not exist, or a parent directory does not
// exist, or a parent directory is actually a file.
- return DELETE_PATH_DOES_NOT_EXIST;
+ return DeletePathResult::kDoesNotExist;
} else if (err != ERROR_ACCESS_DENIED) {
// Some unknown error occurred.
if (error) {
*error = MakeErrorMessage(WSTR(__FILE__), __LINE__, L"DeleteFileW",
path, err);
}
- return DELETE_PATH_ERROR;
+ return DeletePathResult::kError;
}
// DeleteFileW failed with access denied, because the file is read-only or
@@ -356,15 +375,31 @@ int DeletePath(const wstring& path, wstring* error) {
if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) {
// The file disappeared, or one of its parent directories disappeared,
// or one of its parent directories is no longer a directory.
- return DELETE_PATH_DOES_NOT_EXIST;
+ return DeletePathResult::kDoesNotExist;
+ } else if (err != ERROR_ACCESS_DENIED) {
+ // Some unknown error occurred.
+ if (error) {
+ *error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
+ L"GetFileAttributesW", path, err);
+ }
+ return DeletePathResult::kError;
}
- // Some unknown error occurred.
- if (error) {
- *error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
- L"GetFileAttributesW", path, err);
+ // According to a comment in .NET CoreFX [1] (which is the only relevant
+ // information we found as of 2018-07-13) GetFileAttributesW may fail with
+ // ERROR_ACCESS_DENIED if the file is marked for deletion but not yet
+ // actually deleted, but FindFirstFileW should succeed even then.
+ //
+ // [1]
+ // https://github.com/dotnet/corefx/blob/f25eb288a449010574a6e95fe298f3ad880ada1e/src/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs#L205-L208
+ WIN32_FIND_DATAW find_data;
+ HANDLE find = FindFirstFileW(wpath, &find_data);
+ if (find == INVALID_HANDLE_VALUE) {
+ // The path is already deleted.
+ return DeletePathResult::kDoesNotExist;
}
- return DELETE_PATH_ERROR;
+ FindClose(find);
+ attr = find_data.dwFileAttributes;
}
if (attr & FILE_ATTRIBUTE_DIRECTORY) {
@@ -374,14 +409,14 @@ int DeletePath(const wstring& path, wstring* error) {
err = GetLastError();
if (err == ERROR_SHARING_VIOLATION) {
// The junction or directory is in use by another process.
- return DELETE_PATH_ACCESS_DENIED;
+ return DeletePathResult::kAccessDenied;
} else if (err == ERROR_DIR_NOT_EMPTY) {
// The directory is not empty.
- return DELETE_PATH_DIRECTORY_NOT_EMPTY;
+ return DeletePathResult::kDirectoryNotEmpty;
} else if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) {
// The directory or one of its directories disappeared or is no longer
// a directory.
- return DELETE_PATH_DOES_NOT_EXIST;
+ return DeletePathResult::kDoesNotExist;
}
// Some unknown error occurred.
@@ -389,7 +424,7 @@ int DeletePath(const wstring& path, wstring* error) {
*error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
L"DeleteDirectoryW", path, err);
}
- return DELETE_PATH_ERROR;
+ return DeletePathResult::kError;
}
} else {
// It's a file and it's probably read-only.
@@ -400,14 +435,14 @@ int DeletePath(const wstring& path, wstring* error) {
if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) {
// The file disappeared, or one of its parent directories disappeared,
// or one of its parent directories is no longer a directory.
- return DELETE_PATH_DOES_NOT_EXIST;
+ return DeletePathResult::kDoesNotExist;
}
// Some unknown error occurred.
if (error) {
*error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
L"SetFileAttributesW", path, err);
}
- return DELETE_PATH_ERROR;
+ return DeletePathResult::kError;
}
if (!DeleteFileW(wpath)) {
@@ -416,7 +451,7 @@ int DeletePath(const wstring& path, wstring* error) {
if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) {
// The file disappeared, or one of its parent directories disappeared,
// or one of its parent directories is no longer a directory.
- return DELETE_PATH_DOES_NOT_EXIST;
+ return DeletePathResult::kDoesNotExist;
}
// Some unknown error occurred.
@@ -424,11 +459,11 @@ int DeletePath(const wstring& path, wstring* error) {
*error = MakeErrorMessage(WSTR(__FILE__), __LINE__, L"DeleteFileW",
path, err);
}
- return DELETE_PATH_ERROR;
+ return DeletePathResult::kError;
}
}
}
- return DELETE_PATH_SUCCESS;
+ return DeletePathResult::kSuccess;
}
} // namespace windows
diff --git a/src/main/native/windows/file.h b/src/main/native/windows/file.h
index ce13908140..e16b03bd23 100644
--- a/src/main/native/windows/file.h
+++ b/src/main/native/windows/file.h
@@ -42,12 +42,14 @@ enum {
};
// Keep in sync with j.c.g.devtools.build.lib.windows.WindowsFileOperations
-enum {
- DELETE_PATH_SUCCESS = 0,
- DELETE_PATH_DOES_NOT_EXIST = 1,
- DELETE_PATH_DIRECTORY_NOT_EMPTY = 2,
- DELETE_PATH_ACCESS_DENIED = 3,
- DELETE_PATH_ERROR = 4,
+struct DeletePathResult {
+ enum {
+ kSuccess = 0,
+ kError = 1,
+ kDoesNotExist = 2,
+ kDirectoryNotEmpty = 3,
+ kAccessDenied = 4,
+ };
};
struct CreateJunctionResult {
diff --git a/src/test/native/windows/file_test.cc b/src/test/native/windows/file_test.cc
index 4624fec39c..4792aaae61 100644
--- a/src/test/native/windows/file_test.cc
+++ b/src/test/native/windows/file_test.cc
@@ -177,7 +177,7 @@ TEST_F(WindowsFileOperationsTest, TestCannotCreateJunctionFromExistingFile) {
CreateJunctionResult::kAlreadyExistsButNotJunction);
}
-TEST_F(WindowsFileOperationsTest, TestCannotCreateJunctionIfNameIsBusy) {
+TEST_F(WindowsFileOperationsTest, TestCannotCreateButCanCheckIfNameIsBusy) {
wstring tmp(kUncPrefix + GetTestTmpDirW());
wstring name = tmp + L"\\junc" WLINE;
wstring target = tmp + L"\\target" WLINE;
@@ -188,7 +188,7 @@ TEST_F(WindowsFileOperationsTest, TestCannotCreateJunctionIfNameIsBusy) {
EXPECT_NE(h, INVALID_HANDLE_VALUE);
int actual = CreateJunction(name, target, nullptr);
CloseHandle(h);
- ASSERT_EQ(actual, CreateJunctionResult::kAccessDenied);
+ ASSERT_EQ(actual, CreateJunctionResult::kAlreadyExistsButNotJunction);
}
TEST_F(WindowsFileOperationsTest, TestCanCreateJunctionIfTargetIsBusy) {
@@ -208,14 +208,14 @@ TEST_F(WindowsFileOperationsTest, TestCanDeleteExistingFile) {
wstring tmp(kUncPrefix + GetTestTmpDirW());
wstring path = tmp + L"\\file" WLINE;
EXPECT_TRUE(blaze_util::CreateDummyFile(path));
- ASSERT_EQ(DeletePath(path.c_str(), nullptr), DELETE_PATH_SUCCESS);
+ ASSERT_EQ(DeletePath(path.c_str(), nullptr), DeletePathResult::kSuccess);
}
TEST_F(WindowsFileOperationsTest, TestCanDeleteExistingDirectory) {
wstring tmp(kUncPrefix + GetTestTmpDirW());
wstring path = tmp + L"\\dir" WLINE;
EXPECT_TRUE(CreateDirectoryW(path.c_str(), NULL));
- ASSERT_EQ(DeletePath(path.c_str(), nullptr), DELETE_PATH_SUCCESS);
+ ASSERT_EQ(DeletePath(path.c_str(), nullptr), DeletePathResult::kSuccess);
}
TEST_F(WindowsFileOperationsTest, TestCanDeleteExistingJunction) {
@@ -225,7 +225,7 @@ TEST_F(WindowsFileOperationsTest, TestCanDeleteExistingJunction) {
EXPECT_TRUE(CreateDirectoryW(target.c_str(), NULL));
EXPECT_EQ(CreateJunction(name, target, nullptr),
CreateJunctionResult::kSuccess);
- ASSERT_EQ(DeletePath(name.c_str(), nullptr), DELETE_PATH_SUCCESS);
+ ASSERT_EQ(DeletePath(name.c_str(), nullptr), DeletePathResult::kSuccess);
}
TEST_F(WindowsFileOperationsTest, TestCanDeleteExistingJunctionWithoutTarget) {
@@ -240,14 +240,14 @@ TEST_F(WindowsFileOperationsTest, TestCanDeleteExistingJunctionWithoutTarget) {
EXPECT_NE(GetFileAttributesW(name.c_str()), INVALID_FILE_ATTRIBUTES);
EXPECT_EQ(GetFileAttributesW(target.c_str()), INVALID_FILE_ATTRIBUTES);
// We can delete the dangling junction.
- ASSERT_EQ(DeletePath(name.c_str(), nullptr), DELETE_PATH_SUCCESS);
+ ASSERT_EQ(DeletePath(name.c_str(), nullptr), DeletePathResult::kSuccess);
}
TEST_F(WindowsFileOperationsTest, TestCannotDeleteNonExistentPath) {
wstring tmp(kUncPrefix + GetTestTmpDirW());
wstring path = tmp + L"\\dummy" WLINE;
EXPECT_EQ(GetFileAttributesW(path.c_str()), INVALID_FILE_ATTRIBUTES);
- ASSERT_EQ(DeletePath(path.c_str(), nullptr), DELETE_PATH_DOES_NOT_EXIST);
+ ASSERT_EQ(DeletePath(path.c_str(), nullptr), DeletePathResult::kDoesNotExist);
}
TEST_F(WindowsFileOperationsTest, TestCannotDeletePathWhereParentIsFile) {
@@ -255,7 +255,8 @@ TEST_F(WindowsFileOperationsTest, TestCannotDeletePathWhereParentIsFile) {
wstring parent = tmp + L"\\file" WLINE;
wstring child = parent + L"\\file" WLINE;
EXPECT_TRUE(blaze_util::CreateDummyFile(parent));
- ASSERT_EQ(DeletePath(child.c_str(), nullptr), DELETE_PATH_DOES_NOT_EXIST);
+ ASSERT_EQ(DeletePath(child.c_str(), nullptr),
+ DeletePathResult::kDoesNotExist);
}
TEST_F(WindowsFileOperationsTest, TestCannotDeleteNonEmptyDirectory) {
@@ -265,7 +266,7 @@ TEST_F(WindowsFileOperationsTest, TestCannotDeleteNonEmptyDirectory) {
EXPECT_TRUE(CreateDirectoryW(parent.c_str(), NULL));
EXPECT_TRUE(blaze_util::CreateDummyFile(child));
ASSERT_EQ(DeletePath(parent.c_str(), nullptr),
- DELETE_PATH_DIRECTORY_NOT_EMPTY);
+ DeletePathResult::kDirectoryNotEmpty);
}
TEST_F(WindowsFileOperationsTest, TestCannotDeleteBusyFile) {
@@ -277,7 +278,7 @@ TEST_F(WindowsFileOperationsTest, TestCannotDeleteBusyFile) {
EXPECT_NE(h, INVALID_HANDLE_VALUE);
int actual = DeletePath(path.c_str(), nullptr);
CloseHandle(h);
- ASSERT_EQ(actual, DELETE_PATH_ACCESS_DENIED);
+ ASSERT_EQ(actual, DeletePathResult::kAccessDenied);
}
TEST_F(WindowsFileOperationsTest, TestCannotDeleteBusyDirectory) {
@@ -289,7 +290,7 @@ TEST_F(WindowsFileOperationsTest, TestCannotDeleteBusyDirectory) {
EXPECT_NE(h, INVALID_HANDLE_VALUE);
int actual = DeletePath(path.c_str(), nullptr);
CloseHandle(h);
- ASSERT_EQ(actual, DELETE_PATH_ACCESS_DENIED);
+ ASSERT_EQ(actual, DeletePathResult::kAccessDenied);
}
TEST_F(WindowsFileOperationsTest, TestCannotDeleteBusyJunction) {
@@ -306,7 +307,7 @@ TEST_F(WindowsFileOperationsTest, TestCannotDeleteBusyJunction) {
EXPECT_NE(h, INVALID_HANDLE_VALUE);
int actual = DeletePath(name.c_str(), nullptr);
CloseHandle(h);
- ASSERT_EQ(actual, DELETE_PATH_ACCESS_DENIED);
+ ASSERT_EQ(actual, DeletePathResult::kAccessDenied);
}
TEST_F(WindowsFileOperationsTest, TestCanDeleteJunctionWhoseTargetIsBusy) {
@@ -322,7 +323,7 @@ TEST_F(WindowsFileOperationsTest, TestCanDeleteJunctionWhoseTargetIsBusy) {
EXPECT_NE(h, INVALID_HANDLE_VALUE);
int actual = DeletePath(name.c_str(), nullptr);
CloseHandle(h);
- ASSERT_EQ(actual, DELETE_PATH_SUCCESS);
+ ASSERT_EQ(actual, DeletePathResult::kSuccess);
}
#undef TOSTRING1
generated by cgit v1.2.3 (git 2.39.1) at 2024-07-10 07:36:26 +0000