PathFragment comparisons are now platform-aware
PathFragment's `equals`, `hashCode`, `compareTo`,
`startsWith`, `endsWith`, and `relativeTo` are now
aware of case-insensitivity when running on
Windows.
This approach is better than
https://bazel-review.googlesource.com/c/9124/
because it preserves path casing, which is
important when computing action output paths.
This change contains two additional bugfixes:
- `compareTo` now takes `driveLetter` into account
- the `InMemoryFileSystem` in `PathWindowsTest` is
not case-insensitive
Fixes https://github.com/bazelbuild/bazel/issues/2613
--
Change-Id: I1a4250a373fff03fa02a6d8360457450b47a42a8
Reviewed-on: https://cr.bazel.build/9126
PiperOrigin-RevId: 149106930
MOS_MIGRATED_REVID=149106930
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/PathFragment.java b/src/main/java/com/google/devtools/build/lib/vfs/PathFragment.java
index 2bed67d..b466fc9 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/PathFragment.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/PathFragment.java
@@ -518,11 +518,9 @@
+ " is not beneath " + ancestorDirectory);
}
- for (int index = 0; index < ancestorLength; index++) {
- if (!segments[index].equals(ancestorSegments[index])) {
- throw new IllegalArgumentException("PathFragment " + this
- + " is not beneath " + ancestorDirectory);
- }
+ if (!segmentsEqual(ancestorLength, segments, 0, ancestorSegments)) {
+ throw new IllegalArgumentException(
+ "PathFragment " + this + " is not beneath " + ancestorDirectory);
}
int length = segments.length - ancestorLength;
@@ -573,12 +571,7 @@
|| (isAbsolute && this.driveLetter != prefix.driveLetter)) {
return false;
}
- for (int i = 0, len = prefix.segments.length; i < len; i++) {
- if (!this.segments[i].equals(prefix.segments[i])) {
- return false;
- }
- }
- return true;
+ return segmentsEqual(prefix.segments.length, segments, 0, prefix.segments);
}
/**
@@ -594,12 +587,7 @@
return false;
}
int offset = this.segments.length - suffix.segments.length;
- for (int i = 0; i < suffix.segments.length; i++) {
- if (!this.segments[offset + i].equals(suffix.segments[i])) {
- return false;
- }
- }
- return true;
+ return segmentsEqual(suffix.segments.length, segments, offset, suffix.segments);
}
private static String[] subarray(String[] array, int start, int length) {
@@ -736,10 +724,12 @@
// overhead from synchronization, at the cost of potentially computing the hash code more than
// once.
int h = hashCode;
+ boolean isWindows = OS.getCurrent() == OS.WINDOWS;
if (h == 0) {
h = Boolean.valueOf(isAbsolute).hashCode();
for (String segment : segments) {
- h = h * 31 + segment.hashCode();
+ int segmentHash = isWindows ? segment.toLowerCase().hashCode() : segment.hashCode();
+ h = h * 31 + segmentHash;
}
if (!isEmpty()) {
h = h * 31 + Character.valueOf(driveLetter).hashCode();
@@ -763,10 +753,37 @@
} else {
return isAbsolute == otherPath.isAbsolute
&& driveLetter == otherPath.driveLetter
- && Arrays.equals(otherPath.segments, segments);
+ && segments.length == otherPath.segments.length
+ && segmentsEqual(segments.length, segments, 0, otherPath.segments);
}
}
+ private static boolean segmentsEqual(
+ int length, String[] segments1, int offset1, String[] segments2) {
+ if ((segments1.length - offset1) < length || segments2.length < length) {
+ return false;
+ }
+ boolean isWindows = OS.getCurrent() == OS.WINDOWS;
+ for (int i = 0; i < length; ++i) {
+ String seg1 = segments1[i + offset1];
+ String seg2 = segments2[i];
+ if ((seg1 == null) != (seg2 == null)) {
+ return false;
+ }
+ if (seg1 == null) {
+ continue;
+ }
+ if (isWindows) {
+ seg1 = seg1.toLowerCase();
+ seg2 = seg2.toLowerCase();
+ }
+ if (!seg1.equals(seg2)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
/**
* Compares two PathFragments using the lexicographical order.
*/
@@ -775,17 +792,27 @@
if (isAbsolute != p2.isAbsolute) {
return isAbsolute ? -1 : 1;
}
+ int cmp = Character.compare(driveLetter, p2.driveLetter);
+ if (cmp != 0) {
+ return cmp;
+ }
PathFragment p1 = this;
String[] segments1 = p1.segments;
String[] segments2 = p2.segments;
int len1 = segments1.length;
int len2 = segments2.length;
int n = Math.min(len1, len2);
+ boolean isWindows = OS.getCurrent() == OS.WINDOWS;
for (int i = 0; i < n; i++) {
- String segment1 = segments1[i];
- String segment2 = segments2[i];
- if (!segment1.equals(segment2)) {
- return segment1.compareTo(segment2);
+ String seg1 = segments1[i];
+ String seg2 = segments2[i];
+ if (isWindows) {
+ seg1 = seg1.toLowerCase();
+ seg2 = seg2.toLowerCase();
+ }
+ cmp = seg1.compareTo(seg2);
+ if (cmp != 0) {
+ return cmp;
}
}
return len1 - len2;
diff --git a/src/test/java/com/google/devtools/build/lib/windows/PathWindowsTest.java b/src/test/java/com/google/devtools/build/lib/windows/PathWindowsTest.java
index 388c3fd..04015dd 100644
--- a/src/test/java/com/google/devtools/build/lib/windows/PathWindowsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/windows/PathWindowsTest.java
@@ -24,6 +24,7 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.Path.PathFactory;
import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
import com.google.devtools.build.lib.windows.WindowsFileSystem.WindowsPath;
import java.util.ArrayList;
@@ -65,6 +66,11 @@
protected PathFactory getPathFactory() {
return WindowsFileSystem.getPathFactoryForTesting(shortPathResolver);
}
+
+ @Override
+ public boolean isFilePathCaseSensitive() {
+ return false;
+ }
};
root = (WindowsPath) filesystem.getRootDirectory().getRelative("C:/");
root.createDirectory();
@@ -280,4 +286,53 @@
"D:/program files/microsoft something/foo/~bar_hello",
"D:/program files/microsoft something/foo/will.exist");
}
+
+ @Test
+ public void testCaseInsensitivePathFragment() {
+ // equals
+ assertThat(new PathFragment("c:/FOO/BAR")).isEqualTo(new PathFragment("c:\\foo\\bar"));
+ assertThat(new PathFragment("c:/FOO/BAR")).isNotEqualTo(new PathFragment("d:\\foo\\bar"));
+ assertThat(new PathFragment("c:/FOO/BAR")).isNotEqualTo(new PathFragment("/foo/bar"));
+ // equals for the string representation
+ assertThat(new PathFragment("c:/FOO/BAR").toString())
+ .isNotEqualTo(new PathFragment("c:/foo/bar").toString());
+ // hashCode
+ assertThat(new PathFragment("c:/FOO/BAR").hashCode())
+ .isEqualTo(new PathFragment("c:\\foo\\bar").hashCode());
+ assertThat(new PathFragment("c:/FOO/BAR").hashCode())
+ .isNotEqualTo(new PathFragment("d:\\foo\\bar").hashCode());
+ assertThat(new PathFragment("c:/FOO/BAR").hashCode())
+ .isNotEqualTo(new PathFragment("/foo/bar").hashCode());
+ // compareTo
+ assertThat(new PathFragment("c:/FOO/BAR").compareTo(new PathFragment("c:\\foo\\bar")))
+ .isEqualTo(0);
+ assertThat(new PathFragment("c:/FOO/BAR").compareTo(new PathFragment("d:\\foo\\bar")))
+ .isLessThan(0);
+ assertThat(new PathFragment("c:/FOO/BAR").compareTo(new PathFragment("/foo/bar")))
+ .isGreaterThan(0);
+ // startsWith
+ assertThat(new PathFragment("c:/FOO/BAR").startsWith(new PathFragment("c:\\foo"))).isTrue();
+ assertThat(new PathFragment("c:/FOO/BAR").startsWith(new PathFragment("d:\\foo"))).isFalse();
+ // endsWith
+ assertThat(new PathFragment("c:/FOO/BAR/BAZ").endsWith(new PathFragment("bar\\baz"))).isTrue();
+ assertThat(new PathFragment("c:/FOO/BAR/BAZ").endsWith(new PathFragment("/bar/baz"))).isFalse();
+ assertThat(new PathFragment("c:/FOO/BAR/BAZ").endsWith(new PathFragment("d:\\bar\\baz")))
+ .isFalse();
+ // relativeTo
+ assertThat(new PathFragment("c:/FOO/BAR/BAZ/QUX").relativeTo(new PathFragment("c:\\foo\\bar")))
+ .isEqualTo(new PathFragment("Baz/Qux"));
+ }
+
+ @Test
+ public void testCaseInsensitiveRootedPath() {
+ Path ancestor = filesystem.getPath("C:\\foo\\bar");
+ assertThat(ancestor).isInstanceOf(WindowsPath.class);
+ Path child = filesystem.getPath("C:\\FOO\\Bar\\baz");
+ assertThat(child).isInstanceOf(WindowsPath.class);
+ assertThat(child.startsWith(ancestor)).isTrue();
+ assertThat(child.relativeTo(ancestor)).isEqualTo(new PathFragment("baz"));
+ RootedPath actual = RootedPath.toRootedPath(ancestor, child);
+ assertThat(actual.getRoot()).isEqualTo(ancestor);
+ assertThat(actual.getRelativePath()).isEqualTo(new PathFragment("baz"));
+ }
}
