Skip to content

Commit

Permalink
PathFragment comparisons are now platform-aware
Browse files Browse the repository at this point in the history 
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 #2613

--
Change-Id: I1a4250a373fff03fa02a6d8360457450b47a42a8
Reviewed-on: https://cr.bazel.build/9126
PiperOrigin-RevId: 149106930
MOS_MIGRATED_REVID=149106930
  • Loading branch information
@laszlocsomor @hermione521
laszlocsomor authored and hermione521 committed Mar 14, 2017
1 parent 460e3bb commit 18a559b
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 23 deletions.
73 changes: 50 additions & 23 deletions src/main/java/com/google/devtools/build/lib/vfs/PathFragment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -518,11 +518,9 @@ public PathFragment relativeTo(PathFragment ancestorDirectory) {
+ " 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;
Expand Down Expand Up @@ -573,12 +571,7 @@ public boolean startsWith(PathFragment prefix) {
|| (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);
}

/**
Expand All @@ -594,12 +587,7 @@ public boolean endsWith(PathFragment suffix) {
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) {
Expand Down Expand Up @@ -736,10 +724,12 @@ public int hashCode() {
// 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();
Expand All @@ -763,10 +753,37 @@ public boolean equals(Object other) {
} 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.
*/
Expand All @@ -775,17 +792,27 @@ public int compareTo(PathFragment p2) {
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;
Expand Down
56 changes: 56 additions & 0 deletions src/test/java/com/google/devtools/build/lib/vfs/PathWindowsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import com.google.common.base.Predicate;
import com.google.devtools.build.lib.util.BlazeClock;
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 java.util.ArrayList;
import java.util.HashMap;
Expand Down Expand Up @@ -62,6 +64,11 @@ public final void initializeFileSystem() throws Exception {
protected PathFactory getPathFactory() {
return WindowsFileSystem.getPathFactoryForTesting(shortPathResolver);
}

@Override
public boolean isFilePathCaseSensitive() {
return false;
}
};
root = filesystem.getRootDirectory().getRelative("C:/");
root.createDirectory();
Expand Down Expand Up @@ -318,4 +325,53 @@ public boolean apply(Path child) {
"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"));
}
}

0 comments on commit 18a559b

Please sign in to comment.