Automated rollback of commit 06d2f6d8ad1a7cde19c064e54e31dad7bd8cfdcf.
*** Reason for rollback ***
We must implement path case fixing on top of Skyframe (by requesting DirectoryListingValue), rather than as a JNI method. That way clients get invalidated when a path is renamed such that only the casing changes.
*** Original change description ***
Windows: implement getCorrectCasing
Implement getCorrectCasing: a function that
normalizes a path and returns the case-correct
version of it.
See https://github.com/bazelbuild/bazel/issues/8799
Closes #9435.
PiperOrigin-RevId: 277688249
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
index e54b923..a80f3d5 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
@@ -241,9 +241,4 @@
return Files.readAttributes(
file.toPath(), DosFileAttributes.class, symlinkOpts(followSymlinks));
}
-
- @VisibleForTesting
- Path getCorrectCasingForTesting(Path p) throws IOException {
- return getPath(WindowsFileOperations.getCorrectCasing(p.getPathString()));
- }
}
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 552c606..340893f 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
@@ -113,8 +113,6 @@
private static native int nativeDeletePath(String path, String[] error);
- private static native void nativeGetCorrectCasing(String path, String[] result);
-
/** Determines whether `path` is a junction point or directory symlink. */
public static boolean isSymlinkOrJunction(String path) throws IOException {
WindowsJniLoader.loadJni();
@@ -261,33 +259,4 @@
throw new IOException(String.format("Cannot delete path '%s': %s", path, error[0]));
}
}
-
- /**
- * Returns the case-corrected copy of <code>absPath</code>
- *
- * <p>Windows paths are case-insensitive, but the filesystem preserves the casing. For example you
- * can create <code>C:\Foo\Bar</code> and access it as <code>c:\FOO\bar</code>, but the correct
- * casing would be <code>C:\Foo\Bar</code>. Sometimes Bazel needs the correct casing (see
- * https://github.com/bazelbuild/bazel/issues/8799 for example).
- *
- * <p>This method fixes paths to have the correct casing, up to the last existing segment of the
- * path.
- *
- * @param absPath an absolute Windows path. May not be normalized (i.e. have <code>./</code> and
- * <code>../</code> segments) and may have <code>/</code> separators instead of <code>\\
- * </code>.
- * @return an absolute, normalized Windows path. The path is case-corrected up to the last
- * existing segment of <code>absPath</code>. The rest of the segments are left unchanged (with
- * regard to casing) but are normalized and copied into the result.
- * @throws IOException if <code>absPath</code> was empty or was not absolute
- */
- public static String getCorrectCasing(String absPath) throws IOException {
- WindowsJniLoader.loadJni();
- String[] result = new String[] {null};
- nativeGetCorrectCasing(absPath, result);
- if (result[0].isEmpty()) {
- throw new IOException(String.format("Path is not absolute: '%s'", absPath));
- }
- return removeUncPrefixAndUseSlashes(result[0]);
- }
}
diff --git a/src/main/native/windows/file-jni.cc b/src/main/native/windows/file-jni.cc
index 47f5f82..63d81d8 100644
--- a/src/main/native/windows/file-jni.cc
+++ b/src/main/native/windows/file-jni.cc
@@ -141,14 +141,3 @@
}
return result;
}
-
-extern "C" JNIEXPORT void JNICALL
-Java_com_google_devtools_build_lib_windows_jni_WindowsFileOperations_nativeGetCorrectCasing(
- JNIEnv* env, jclass clazz, jstring path, jobjectArray result_holder) {
- std::wstring wpath(bazel::windows::GetJavaWstring(env, path));
- std::wstring result(bazel::windows::GetCorrectCasing(wpath));
- env->SetObjectArrayElement(
- result_holder, 0,
- env->NewString(reinterpret_cast<const jchar*>(result.c_str()),
- result.size()));
-}
diff --git a/src/main/native/windows/file.cc b/src/main/native/windows/file.cc
index fa65374..cc79ead 100644
--- a/src/main/native/windows/file.cc
+++ b/src/main/native/windows/file.cc
@@ -781,61 +781,5 @@
}
}
-std::wstring GetCorrectCasing(const std::wstring& abs_path) {
- if (!HasDriveSpecifierPrefix(abs_path.c_str())) {
- return L"";
- }
- std::wstring path = Normalize(abs_path);
- std::unique_ptr<wchar_t[]> result(new wchar_t[4 + path.size() + 1]);
- // Ensure path starts with UNC prefix, so we can use long paths in
- // FindFirstFileW.
- wcscpy(result.get(), L"\\\\?\\");
- // Copy the rest of the normalized path. (Must be normalized and use `\`
- // separators, for the UNC prefix to work.)
- wcscpy(result.get() + 4, path.c_str());
- // Ensure drive letter is upper case.
- result[4] = towupper(result[4]);
- // Start at index 7, which is after the UNC prefix and drive segment (i.e.
- // past `\\?\C:\`).
- wchar_t* start = result.get() + 7;
- // Fix the casing of each segment, from left to right.
- while (true) {
- // Find current segment end.
- wchar_t* seg_end = wcschr(start, L'\\');
- // Pretend the whole path ends at the current segment.
- if (seg_end) {
- *seg_end = 0;
- }
- // Find this path from the filesystem. The lookup is case-insensitive, but
- // the result shows the correct casing. Because we fix the casing from left
- // to right, only the last segment needs fixing, so we look up that
- // particular directory (or file).
- WIN32_FIND_DATAW metadata;
- HANDLE handle = FindFirstFileW(result.get(), &metadata);
- if (handle != INVALID_HANDLE_VALUE) {
- // Found the child. The correct casing is in metadata.cFileName
- wcscpy(start, metadata.cFileName);
- FindClose(handle);
- if (seg_end) {
- // There are more path segments to fix. Restore the `\` separator.
- *seg_end = L'\\';
- start = seg_end + 1;
- } else {
- // This was the last segment.
- break;
- }
- } else {
- // Path does not exist. Restore the `\` separator and leave the rest of
- // the path unchanged.
- if (seg_end) {
- *seg_end = L'\\';
- }
- break;
- }
- }
- // Return the case-corrected path without the `\\?\` prefix.
- return result.get() + 4;
-}
-
} // namespace windows
} // namespace bazel
diff --git a/src/main/native/windows/file.h b/src/main/native/windows/file.h
index 9786687..f8651e6 100644
--- a/src/main/native/windows/file.h
+++ b/src/main/native/windows/file.h
@@ -195,23 +195,6 @@
bool GetCwd(std::wstring* result, DWORD* err_code);
-// Normalizes and corrects the casing of 'abs_path'.
-//
-// 'abs_path' must be absolute, and start with a Windows drive prefix.
-// It may have an UNC prefix, may not be normalized, may use '\' and '/' as
-// directory separators.
-//
-// The result is normalized, uses '\' separators, has no trailing '\', and is
-// case-corrected up to the last existing segment. Non-existent tail segments
-// are kept in their casing, but normalized.
-//
-// For example if C:\Foo\Bar exists, then
-// GetCorrectCasing("\\\\?\\c:/FOO/./bar\\") returns "C:\Foo\Bar" and
-// GetCorrectCasing("c:/foo/qux//../bar/BAZ/quX") returns "C:\Foo\Bar\BAZ\quX".
-//
-// If 'abs_path' is null or empty or not absolute, the result is empty.
-std::wstring GetCorrectCasing(const std::wstring& abs_path);
-
} // namespace windows
} // namespace bazel
diff --git a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java
index 1efa839..dc3275c0 100644
--- a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java
+++ b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileSystemTest.java
@@ -369,25 +369,4 @@
assertThat(juncPath.readSymbolicLink()).isEqualTo(dirPath.asFragment());
}
-
- private static String invertCharacterCasing(String s) {
- char[] a = s.toCharArray();
- for (int i = 0; i < a.length; ++i) {
- char c = a[i];
- a[i] = Character.isUpperCase(c) ? Character.toLowerCase(c) : Character.toUpperCase(c);
- }
- return new String(a);
- }
-
- @Test
- public void testGetCorrectCasing() throws Exception {
- String rootStr = scratchRoot.getPathString();
- String inverseRootStr = invertCharacterCasing(rootStr);
- Path inverseRoot = fs.getPath(inverseRootStr);
- assertThat(inverseRootStr).isNotEqualTo(rootStr);
- assertThat(inverseRoot).isEqualTo(scratchRoot);
- Path correctCasing = fs.getCorrectCasingForTesting(inverseRoot);
- assertThat(correctCasing).isEqualTo(scratchRoot);
- assertThat(correctCasing.getPathString()).isNotEqualTo(inverseRootStr);
- }
}
diff --git a/src/test/native/windows/file_test.cc b/src/test/native/windows/file_test.cc
index de3c8af..49ce133 100644
--- a/src/test/native/windows/file_test.cc
+++ b/src/test/native/windows/file_test.cc
@@ -452,64 +452,8 @@
ASSERT_NORMALIZE("c:\\", "c:\\");
ASSERT_NORMALIZE("c:\\..//foo/./bar/", "c:\\foo\\bar");
ASSERT_NORMALIZE("../foo", "..\\foo");
- ASSERT_NORMALIZE("../foo\\", "..\\foo");
- ASSERT_NORMALIZE("../foo\\\\", "..\\foo");
- ASSERT_NORMALIZE("..//foo\\\\", "..\\foo");
#undef ASSERT_NORMALIZE
}
-static void GetTestTempDirWithCorrectCasing(std::wstring* result,
- std::wstring* result_inverted) {
- static constexpr size_t kMaxPath = 0x8000;
- wchar_t buf[kMaxPath];
- ASSERT_GT(GetEnvironmentVariableW(L"TEST_TMPDIR", buf, kMaxPath), 0);
- HANDLE h = CreateFileW(buf, 0,
- FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
- NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
- ASSERT_NE(h, INVALID_HANDLE_VALUE);
- ASSERT_GT(GetFinalPathNameByHandleW(h, buf, kMaxPath, 0), 0);
- CloseHandle(h);
- *result = RemoveUncPrefixMaybe(buf);
-
- for (wchar_t* c = buf; *c; ++c) {
- if (*c >= L'A' && *c <= L'Z') {
- *c = L'a' + *c - L'A';
- } else if (*c >= L'a' && *c <= L'z') {
- *c = L'A' + *c - L'a';
- }
- }
- *result_inverted = RemoveUncPrefixMaybe(buf);
-}
-
-TEST(FileTests, TestGetCorrectCase) {
- EXPECT_EQ(GetCorrectCasing(L""), L"");
- EXPECT_EQ(GetCorrectCasing(L"d"), L"");
- EXPECT_EQ(GetCorrectCasing(L"d:"), L"");
-
- EXPECT_EQ(GetCorrectCasing(L"d:\\"), L"D:\\");
- EXPECT_EQ(GetCorrectCasing(L"d:/"), L"D:\\");
- EXPECT_EQ(GetCorrectCasing(L"d:\\\\"), L"D:\\");
- EXPECT_EQ(GetCorrectCasing(L"d://"), L"D:\\");
- EXPECT_EQ(GetCorrectCasing(L"d:\\\\\\"), L"D:\\");
- EXPECT_EQ(GetCorrectCasing(L"d:///"), L"D:\\");
-
- EXPECT_EQ(GetCorrectCasing(L"A:/Non:Existent"), L"A:\\Non:Existent");
-
- EXPECT_EQ(GetCorrectCasing(L"A:\\\\Non:Existent"), L"A:\\Non:Existent");
- EXPECT_EQ(GetCorrectCasing(L"A:\\Non:Existent\\."), L"A:\\Non:Existent");
- EXPECT_EQ(GetCorrectCasing(L"A:\\Non:Existent\\..\\.."), L"A:\\");
- EXPECT_EQ(GetCorrectCasing(L"A:\\Non:Existent\\.\\"), L"A:\\Non:Existent");
-
- std::wstring tmp, tmp_inverse;
- GetTestTempDirWithCorrectCasing(&tmp, &tmp_inverse);
- ASSERT_GT(tmp.size(), 0);
- ASSERT_NE(tmp, tmp_inverse);
- EXPECT_EQ(GetCorrectCasing(tmp), tmp);
- EXPECT_EQ(GetCorrectCasing(tmp_inverse), tmp);
- EXPECT_EQ(GetCorrectCasing(tmp_inverse + L"\\"), tmp);
- EXPECT_EQ(GetCorrectCasing(tmp_inverse + L"\\Does\\\\Not\\./Exist"),
- tmp + L"\\Does\\Not\\Exist");
-}
-
} // namespace windows
} // namespace bazel