Skip to content

Commit

Permalink
Remove support for Windows relative-to-current-drive paths.
Browse files Browse the repository at this point in the history 
Eg. 'C:foo' was previously "the directory 'foo' relative to the current directory of drive 'C:\'". Now it is simple interpreted as "the relative path fragment 'C:foo'".

PiperOrigin-RevId: 180936012
  • Loading branch information
@tomlu
tomlu authored and Copybara-Service committed Jan 5, 2018
1 parent 39a23a0 commit 3ed0159
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 54 deletions.
9 changes: 6 additions & 3 deletions src/main/java/com/google/devtools/build/lib/vfs/WindowsPathFragment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
abstract class WindowsPathFragment extends PathFragment {
static final Helper HELPER = new Helper();

// The drive letter of an absolute path, eg. 'C' for 'C:/foo'.
// We deliberately ignore "C:foo" style paths and treat them like a literal "C:foo" path segment.
protected final char driveLetter;

protected WindowsPathFragment(char driveLetter, String[] segments) {
Expand Down Expand Up @@ -63,13 +65,14 @@ private static boolean isDriveLetter(char c) {
@Override
PathFragment create(String path) {
char driveLetter =
path.length() >= 2 && path.charAt(1) == ':' && isDriveLetter(path.charAt(0))
path.length() >= 3
&& path.charAt(1) == ':'
&& isSeparator(path.charAt(2))
&& isDriveLetter(path.charAt(0))
? Character.toUpperCase(path.charAt(0))
: '\0';
if (driveLetter != '\0') {
path = path.substring(2);
// TODO(bazel-team): Decide what to do about non-absolute paths with a volume name, e.g.
// C:x.
}
boolean isAbsolute = path.length() > 0 && isSeparator(path.charAt(0));
return isAbsolute
Expand Down
62 changes: 11 additions & 51 deletions src/test/java/com/google/devtools/build/lib/vfs/PathFragmentWindowsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ public void testIsAbsoluteWindows() {
assertThat(PathFragment.create("d:/foo/bar").isAbsolute()).isTrue();

assertThat(PathFragment.create("*:/").isAbsolute()).isFalse();

// C: is not an absolute path, it points to the current active directory on drive C:.
assertThat(PathFragment.create("C:").isAbsolute()).isFalse();
assertThat(PathFragment.create("C:foo").isAbsolute()).isFalse();
}

@Test
Expand All @@ -65,23 +61,15 @@ public void testAbsoluteAndAbsoluteLookingPaths() {
assertThat(p3.getDriveLetter()).isEqualTo('C');
assertThat(p3.getSegments()).isEmpty();

PathFragment p4 = PathFragment.create("C:");
assertThat(p4.isAbsolute()).isFalse();
assertThat(p4.getDriveLetter()).isEqualTo('C');
assertThat(p4.getSegments()).isEmpty();

PathFragment p5 = PathFragment.create("/c:");
assertThat(p5.isAbsolute()).isTrue();
assertThat(p5.getDriveLetter()).isEqualTo('\0');
assertThat(p5.getSegments()).containsExactly("c:");

assertThat(p1).isEqualTo(p2);
assertThat(p1).isNotEqualTo(p3);
assertThat(p1).isNotEqualTo(p4);
assertThat(p1).isNotEqualTo(p5);
assertThat(p3).isNotEqualTo(p4);
assertThat(p3).isNotEqualTo(p5);
assertThat(p4).isNotEqualTo(p5);
}

@Test
Expand Down Expand Up @@ -155,22 +143,14 @@ private static PathFragment makePath(char drive, boolean absolute, String... seg
public void testGetRelativeMixed() throws Exception {
assertGetRelative("a", "b", makePath('\0', false, "a", "b"));
assertGetRelative("a", "/b", makePath('\0', true, "b"));
assertGetRelative("a", "E:b", makePath('\0', false, "a", "b"));
assertGetRelative("a", "E:/b", makePath('E', true, "b"));

assertGetRelative("/a", "b", makePath('\0', true, "a", "b"));
assertGetRelative("/a", "/b", makePath('\0', true, "b"));
assertGetRelative("/a", "E:b", makePath('\0', true, "a", "b"));
assertGetRelative("/a", "E:/b", makePath('E', true, "b"));

assertGetRelative("D:a", "b", makePath('D', false, "a", "b"));
assertGetRelative("D:a", "/b", makePath('D', true, "b"));
assertGetRelative("D:a", "E:b", makePath('D', false, "a", "b"));
assertGetRelative("D:a", "E:/b", makePath('E', true, "b"));

assertGetRelative("D:/a", "b", makePath('D', true, "a", "b"));
assertGetRelative("D:/a", "/b", makePath('D', true, "b"));
assertGetRelative("D:/a", "E:b", makePath('D', true, "a", "b"));
assertGetRelative("D:/a", "E:/b", makePath('E', true, "b"));
}

Expand All @@ -184,8 +164,6 @@ public void testRelativeTo() throws Exception {
assertCantComputeRelativeTo("a", "b");
assertRelativeTo("a/b", "a", "b");

assertRelativeTo("C:", "");
assertRelativeTo("C:", "C:");
assertCantComputeRelativeTo("C:/", "");
assertRelativeTo("C:/", "C:/");
}
Expand Down Expand Up @@ -232,8 +210,6 @@ public void testEmptyRelativePathToEmptyPathWindows() {
// information to the path itself.
assertAllEqual(
PathFragment.EMPTY_FRAGMENT,
PathFragment.create("C:"),
PathFragment.create("D:"),
PathFragment.createAlreadyInterned('\0', false, new String[0]),
PathFragment.createAlreadyInterned('C', false, new String[0]),
PathFragment.createAlreadyInterned('D', false, new String[0]));
Expand All @@ -246,31 +222,6 @@ public void testEmptyRelativePathToEmptyPathWindows() {
.isNotEqualTo(PathFragment.create("C:").getPathString());
}

@Test
public void testConfusingSemanticsOfDriveLettersInRelativePaths() {
// This test serves to document the current confusing semantics of non-empty relative windows
// paths that have drive letters. Also note the above testEmptyRelativePathToEmptyPathWindows
// which documents the confusing semantics of empty relative windows paths that have drive
// letters.
//
// TODO(laszlocsomor): Reevaluate the current semantics. Depending on the results of that,
// consider not storing the drive letter in relative windows paths.
PathFragment cColonFoo = PathFragment.create("C:foo");
PathFragment dColonFoo = PathFragment.create("D:foo");
PathFragment foo = PathFragment.create("foo");
assertThat(cColonFoo).isNotEqualTo(dColonFoo);
assertThat(cColonFoo).isNotEqualTo(foo);
assertThat(dColonFoo).isNotEqualTo(foo);
assertThat(cColonFoo.segmentCount()).isEqualTo(dColonFoo.segmentCount());
assertThat(cColonFoo.segmentCount()).isEqualTo(foo.segmentCount());
assertThat(cColonFoo.startsWith(dColonFoo)).isTrue();
assertThat(cColonFoo.startsWith(foo)).isTrue();
assertThat(foo.startsWith(cColonFoo)).isTrue();
assertThat(cColonFoo.getPathString()).isEqualTo("foo");
assertThat(cColonFoo.getPathString()).isEqualTo(dColonFoo.getPathString());
assertThat(cColonFoo.getPathString()).isEqualTo(foo.getPathString());
}

@Test
public void testWindowsVolumeUppercase() {
assertRegular("C:/", "c:/");
Expand Down Expand Up @@ -345,9 +296,7 @@ public void testStartsWithWindows() {
assertThat(PathFragment.create("C:/foo/bar").startsWith(PathFragment.create("C:/foo")))
.isTrue();
assertThat(PathFragment.create("C:/foo/bar").startsWith(PathFragment.create("C:/"))).isTrue();
assertThat(PathFragment.create("C:foo/bar").startsWith(PathFragment.create("C:"))).isTrue();
assertThat(PathFragment.create("C:/").startsWith(PathFragment.create("C:/"))).isTrue();
assertThat(PathFragment.create("C:").startsWith(PathFragment.create("C:"))).isTrue();

// The first path is absolute, the second is not.
assertThat(PathFragment.create("C:/foo/bar").startsWith(PathFragment.create("C:"))).isFalse();
Expand Down Expand Up @@ -379,4 +328,15 @@ public void testNormalizeWindows() {
assertThat(PathFragment.create("C:/../b").normalize())
.isEqualTo(PathFragment.create("C:/../b"));
}

@Test
public void testWindowsDriveRelativePaths() throws Exception {
// On Windows, paths that look like "C:foo" mean "foo relative to the current directory
// of drive C:\".
// Bazel doesn't resolve such paths, and just takes them literally like normal path segments.
// If the user attempts to open files under such paths, the file system API will give an error.
assertThat(PathFragment.create("C:").isAbsolute()).isFalse();
assertThat(PathFragment.create("C:").getDriveLetter()).isEqualTo('\0');
assertThat(PathFragment.create("C:").getSegments()).containsExactly("C:");
}
}

0 comments on commit 3ed0159

Please sign in to comment.