VFS: implement a Windows-specific Path subclass

The new subclass WindowsFileSystem.WindowsPath is
aware of Windows drives.

This change:
- introduces a new factory for Path objects so
  FileSystems can return a custom implementation
  that instantiates filesystem-specific Paths
- implements the WindowsPath subclass of Path that
  is aware of Windows drives
- introduces the bazel.windows_unix_root JVM
  argument that defines the MSYS root, which
  defines the absolute Windows path that is the
  root of all Unix paths that Bazel creates (e.g.
  "/usr/lib" -> "C:/tools/msys64/usr/lib") except
  if the path is of the form "/c/foo" which is
  treated as "C:/foo"
- removes all Windows-specific logic from Path

PathFragment is still aware of drive letters and
it has to remain so because it is unaware of file
systems.

WindowsPath restricts the allowed path strings to
absolute Unix paths where the first segment, if
any, is a volume specifier. From now on if Bazel
attempts to create a WindowsPath from an absolute
Unix path, Bazel will make it relative to
WindowsPath.UNIX_ROOT, unless the first component
is a single-letter name (e.g. "/c/foo" which is
"C:/foo").

Subclassing Path is necessary because a Unix-style
absolute path doesn't sufficiently define a full
Windows path, as it may be relative to any drive.

Fixes https://github.com/bazelbuild/bazel/issues/1463

--
MOS_MIGRATED_REVID=136350304

5 files changed, 189 insertions, 32 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index 942d9a337d..0556e41c5a 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -169,7 +169,10 @@ java_test(
 java_test(
     name = "windows_test",
     srcs = CROSS_PLATFORM_WINDOWS_TESTS,
-    jvm_flags = ["-Dblaze.os=Windows"],
+    jvm_flags = [
+        "-Dblaze.os=Windows",
+        "-Dbazel.windows_unix_root=C:/fake/msys",
+    ],
     test_class = "com.google.devtools.build.lib.AllTests",
     deps = [
         ":foundations_testutil",
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/PathFragmentTest.java b/src/test/java/com/google/devtools/build/lib/vfs/PathFragmentTest.java
index ac34ec035a..71b7decf9e 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/PathFragmentTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/PathFragmentTest.java
@@ -26,14 +26,12 @@ import com.google.common.collect.Lists;
 import com.google.common.testing.EqualsTester;
 import com.google.devtools.build.lib.testutil.TestUtils;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
-
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
 import java.io.File;
 import java.util.Collections;
 import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
 
 /**
  * This class tests the functionality of the PathFragment.
@@ -69,15 +67,16 @@ public class PathFragmentTest {
     InMemoryFileSystem filesystem = new InMemoryFileSystem();
 
     new EqualsTester()
-        .addEqualityGroup(new PathFragment("../relative/path"),
-                          new PathFragment("../relative/path"),
-                          new PathFragment(new File("../relative/path")))
+        .addEqualityGroup(
+            new PathFragment("../relative/path"),
+            new PathFragment("..").getRelative("relative").getRelative("path"),
+            new PathFragment('\0', false, new String[] {"..", "relative", "path"}),
+            new PathFragment(new File("../relative/path")))
         .addEqualityGroup(new PathFragment("something/else"))
         .addEqualityGroup(new PathFragment("/something/else"))
-        .addEqualityGroup(new PathFragment("/"),
-                          new PathFragment("//////"))
-        .addEqualityGroup(new PathFragment(""))
-        .addEqualityGroup(filesystem.getRootDirectory())  // A Path object.
+        .addEqualityGroup(new PathFragment("/"), new PathFragment("//////"))
+        .addEqualityGroup(new PathFragment(""), PathFragment.EMPTY_FRAGMENT)
+        .addEqualityGroup(filesystem.getRootDirectory()) // A Path object.
         .testEquals();
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/PathFragmentWindowsTest.java b/src/test/java/com/google/devtools/build/lib/vfs/PathFragmentWindowsTest.java
index 48a63e34dc..df2c770d98 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/PathFragmentWindowsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/PathFragmentWindowsTest.java
@@ -14,23 +14,24 @@
 package com.google.devtools.build.lib.vfs;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
+import java.io.File;
+import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
-import java.io.File;
-
 /**
  * This class tests the functionality of the PathFragment.
  */
 @RunWith(JUnit4.class)
 public class PathFragmentWindowsTest {
-  
+
   @Test
   public void testWindowsSeparator() {
     assertEquals("bar/baz", new PathFragment("bar\\baz").toString());
@@ -52,6 +53,26 @@ public class PathFragmentWindowsTest {
   }
 
   @Test
+  public void testAbsolutePathsWithDrive() {
+    PathFragment p1 = new PathFragment("/c");
+    assertThat(p1.isAbsolute()).isTrue();
+    assertThat(p1.getDriveLetter()).isEqualTo('C');
+
+    PathFragment p2 = new PathFragment("/c/");
+    assertThat(p2.isAbsolute()).isTrue();
+    assertThat(p2.getDriveLetter()).isEqualTo('C');
+
+    assertThat(p1).isEqualTo(p2);
+
+    try {
+      new PathFragment("/c:");
+      Assert.fail("expected failure");
+    } catch (IllegalArgumentException e) {
+      assertThat(e.getMessage()).contains("Illegal path string \"/c:\"");
+    }
+  }
+
+  @Test
   public void testIsAbsoluteWindowsBackslash() {
     assertTrue(new PathFragment(new File("C:\\blah")).isAbsolute());
     assertTrue(new PathFragment(new File("C:\\")).isAbsolute());
@@ -83,10 +104,75 @@ public class PathFragmentWindowsTest {
     assertEquals("C:/c/d", new PathFragment("a/b").getRelative("C:/c/d").getPathString());
   }
 
+  private void assertGetRelative(String path, String relative, PathFragment expected)
+      throws Exception {
+    PathFragment actual = new PathFragment(path).getRelative(relative);
+    assertThat(actual.getPathString()).isEqualTo(expected.getPathString());
+    assertThat(actual).isEqualTo(expected);
+    assertThat(actual.getDriveLetter()).isEqualTo(expected.getDriveLetter());
+    assertThat(actual.hashCode()).isEqualTo(expected.hashCode());
+  }
+
+  private void assertRelativeTo(String path, String relativeTo, String... expectedPathSegments)
+      throws Exception {
+    PathFragment expected = new PathFragment('\0', false, expectedPathSegments);
+    PathFragment actual = new PathFragment(path).relativeTo(relativeTo);
+    assertThat(actual.getPathString()).isEqualTo(expected.getPathString());
+    assertThat(actual).isEqualTo(expected);
+    assertThat(actual.getDriveLetter()).isEqualTo(expected.getDriveLetter());
+    assertThat(actual.hashCode()).isEqualTo(expected.hashCode());
+  }
+
+  private void assertCantComputeRelativeTo(String path, String relativeTo) throws Exception {
+    try {
+      new PathFragment(path).relativeTo(relativeTo);
+      Assert.fail("expected failure");
+    } catch (Exception e) {
+      assertThat(e.getMessage()).contains("is not beneath");
+    }
+  }
+
+  private static PathFragment makePath(char drive, boolean absolute, String... segments) {
+    return new PathFragment(drive, absolute, segments);
+  }
+
   @Test
-  public void testGetRelativeMixed() {
-    assertEquals("/b", new PathFragment("C:/a").getRelative("/b").getPathString());
-    assertEquals("C:/b", new PathFragment("/a").getRelative("C:/b").getPathString());
+  public void testGetRelativeMixed() throws Exception {
+    assertGetRelative("a", "b", makePath('\0', false, "a", "b"));
+    assertGetRelative("a", "/b", makePath('B', true));
+    assertGetRelative("a", "E:b", makePath('\0', false, "a", "b"));
+    assertGetRelative("a", "E:/b", makePath('E', true, "b"));
+
+    assertGetRelative("/a", "b", makePath('A', true, "b"));
+    assertGetRelative("/a", "/b", makePath('B', true));
+    assertGetRelative("/a", "E:b", makePath('A', true, "b"));
+    assertGetRelative("/a", "E:/b", makePath('E', true, "b"));
+
+    assertGetRelative("D:a", "b", makePath('D', false, "a", "b"));
+    assertGetRelative("D:a", "/b", makePath('B', true));
+    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('B', true));
+    assertGetRelative("D:/a", "E:b", makePath('D', true, "a", "b"));
+    assertGetRelative("D:/a", "E:/b", makePath('E', true, "b"));
+  }
+
+  @Test
+  public void testRelativeTo() throws Exception {
+    assertRelativeTo("", "");
+    assertCantComputeRelativeTo("", "a");
+
+    assertRelativeTo("a", "", "a");
+    assertRelativeTo("a", "a");
+    assertCantComputeRelativeTo("a", "b");
+    assertRelativeTo("a/b", "a", "b");
+
+    assertRelativeTo("C:", "");
+    assertRelativeTo("C:", "C:");
+    assertCantComputeRelativeTo("C:/", "");
+    assertRelativeTo("C:/", "C:/");
   }
 
   @Test
@@ -97,8 +183,12 @@ public class PathFragmentWindowsTest {
 
   // Tests after here test the canonicalization
   private void assertRegular(String expected, String actual) {
-    assertEquals(expected, new PathFragment(actual).getPathString()); // compare string forms
-    assertEquals(new PathFragment(expected), new PathFragment(actual)); // compare fragment forms
+    PathFragment exp = new PathFragment(expected);
+    PathFragment act = new PathFragment(actual);
+    assertThat(exp.getPathString()).isEqualTo(expected);
+    assertThat(act.getPathString()).isEqualTo(expected);
+    assertThat(act).isEqualTo(exp);
+    assertThat(act.hashCode()).isEqualTo(exp.hashCode());
   }
 
   @Test
@@ -106,9 +196,38 @@ public class PathFragmentWindowsTest {
     assertRegular("C:/", "C:/");
   }
 
+  private void assertAllEqual(PathFragment... ps) {
+    assertThat(ps.length).isGreaterThan(1);
+    for (int i = 1; i < ps.length; i++) {
+      String msg = "comparing items 0 and " + i;
+      assertWithMessage(msg + " for getPathString")
+          .that(ps[i].getPathString())
+          .isEqualTo(ps[0].getPathString());
+      assertWithMessage(msg + " for equals").that(ps[0]).isEqualTo(ps[i]);
+      assertWithMessage(msg + " for hashCode").that(ps[0].hashCode()).isEqualTo(ps[i].hashCode());
+    }
+  }
+
   @Test
   public void testEmptyRelativePathToEmptyPathWindows() {
-    assertRegular("C:", "C:");
+    // Surprising but correct behavior: a PathFragment made of just a drive identifier (and not the
+    // absolute path "C:/") is equal not only to the empty fragment, but (therefore) also to other
+    // drive identifiers.
+    // This makes sense if you consider that these are still empty paths, the drive letter adds no
+    // information to the path itself.
+    assertAllEqual(
+        PathFragment.EMPTY_FRAGMENT,
+        new PathFragment("C:"),
+        new PathFragment("D:"),
+        new PathFragment('\0', false, new String[0]),
+        new PathFragment('C', false, new String[0]),
+        new PathFragment('D', false, new String[0]));
+    assertAllEqual(new PathFragment("C:/"), new PathFragment("/c"), new PathFragment("/c/"));
+    assertAllEqual(new PathFragment("C:/foo"), new PathFragment("/c/foo"));
+
+    assertThat(new PathFragment("C:/")).isNotEqualTo(new PathFragment("C:"));
+    assertThat(new PathFragment("C:/").getPathString())
+        .isNotEqualTo(new PathFragment("C:").getPathString());
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/PathWindowsTest.java b/src/test/java/com/google/devtools/build/lib/vfs/PathWindowsTest.java
index cb8b23b000..864a483536 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/PathWindowsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/PathWindowsTest.java
@@ -18,8 +18,10 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertSame;
 
 import com.google.devtools.build.lib.util.BlazeClock;
+import com.google.devtools.build.lib.vfs.Path.PathFactory;
+import com.google.devtools.build.lib.vfs.WindowsFileSystem.WindowsPathFactory;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
-
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -35,8 +37,16 @@ public class PathWindowsTest {
 
   @Before
   public final void initializeFileSystem() throws Exception  {
-    filesystem = new InMemoryFileSystem(BlazeClock.instance());
-    root = filesystem.getRootDirectory();
+    filesystem =
+        new InMemoryFileSystem(BlazeClock.instance()) {
+          @Override
+          protected PathFactory getPathFactory() {
+            return WindowsPathFactory.INSTANCE;
+          }
+        };
+    root = filesystem.getRootDirectory().getRelative("C:/");
+    root.createDirectory();
+
     Path first = root.getChild("first");
     first.createDirectory();
   }
@@ -98,10 +108,38 @@ public class PathWindowsTest {
   }
 
   @Test
+  public void testAbsoluteUnixPathIsRelativeToWindowsUnixRoot() {
+    Path actual = root.getRelative("/foo/bar");
+    Path expected = root.getRelative("C:/fake/msys/foo/bar");
+    assertThat(actual.getPathString()).isEqualTo(expected.getPathString());
+    assertThat(actual).isEqualTo(expected);
+  }
+
+  @Test
+  public void testAbsoluteUnixPathReferringToDriveIsRecognized() {
+    Path actual = root.getRelative("/c/foo");
+    Path expected = root.getRelative("C:/foo");
+    assertThat(actual.getPathString()).isEqualTo(expected.getPathString());
+    assertThat(actual).isEqualTo(expected);
+
+    // "unexpected" is not a valid MSYS path, we should not be able to create it.
+    try {
+      root.getRelative("/c:");
+      Assert.fail("expected failure");
+    } catch (IllegalArgumentException e) {
+      assertThat(e.getMessage()).contains("Illegal path string \"/c:\"");
+    }
+  }
+
+  @Test
   public void testStartsWithWorksOnWindows() {
     assertStartsWithReturnsOnWindows(true, "C:/first/x", "C:/first/x/y");
     assertStartsWithReturnsOnWindows(true, "c:/first/x", "C:/FIRST/X/Y");
     assertStartsWithReturnsOnWindows(true, "C:/FIRST/X", "c:/first/x/y");
+    assertStartsWithReturnsOnWindows(true, "/", "C:/");
+    assertStartsWithReturnsOnWindows(false, "C:/", "/");
+    assertStartsWithReturnsOnWindows(false, "C:/", "D:/");
+    assertStartsWithReturnsOnWindows(false, "C:/", "D:/foo");
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/UnixPathTest.java b/src/test/java/com/google/devtools/build/lib/vfs/UnixPathTest.java
index 2be505548a..e2317bf0f1 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/UnixPathTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/UnixPathTest.java
@@ -24,12 +24,6 @@ import com.google.common.collect.Lists;
 import com.google.common.testing.EqualsTester;
 import com.google.devtools.build.lib.testutil.TestUtils;
 import com.google.devtools.build.lib.vfs.util.FileSystems;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
 import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
@@ -39,6 +33,10 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
 
 /**
  * Tests for {@link Path}.
@@ -269,7 +267,7 @@ public class UnixPathTest {
 
   @Test
   public void testDerivedSegmentEquality() {
-    Path absoluteSegment = new Path(null);
+    Path absoluteSegment = unixFs.getRootDirectory();
 
     Path derivedNode = absoluteSegment.getChild("derivedSegment");
     Path otherDerivedNode = absoluteSegment.getChild("derivedSegment");