Refactor EdenMount to use proper absolute and relative paths

Summary:
Instead of just `Path`.

To make it clear which paths are which.

Reviewed By: xavierd

fbshipit-source-id: 18b6c26eafaaea6fb74dbeb3f74a53a0ec9d0433

Author: stepancheg

src/com/facebook/buck/edenfs/EdenMount.java

@@ -16,6 +16,8 @@
 
 package com.facebook.buck.edenfs;
 
+import com.facebook.buck.core.filesystems.AbsPath;
+import com.facebook.buck.core.filesystems.ForwardRelPath;
 import com.facebook.buck.util.sha1.Sha1HashCode;
 import com.facebook.eden.thrift.EdenError;
 import com.facebook.eden.thrift.SHA1Result;
@@ -38,23 +40,23 @@ public class EdenMount {
   private final EdenClientResourcePool pool;
 
   /** Value of the mountPoint argument to use when communicating with Eden via the Thrift API. */
-  private final Path mountPoint;
+  private final AbsPath mountPoint;
 
   /** Root of the Buck project of interest that is contained by this {@link EdenMount}. */
-  private final Path projectRoot;
+  private final AbsPath projectRoot;
 
   /**
    * Relative path used to resolve paths under the {@link #projectRoot} in the context of the {@link
    * #mountPoint}.
    */
-  private final Path prefix;
+  private final ForwardRelPath prefix;
 
   /**
    * Creates a new object for communicating with Eden that is bound to the specified (Eden mount
    * point, Buck project root) pair. It must be the case that {@code
    * projectRoot.startsWith(mountPoint)}.
    */
-  EdenMount(EdenClientResourcePool pool, Path mountPoint, Path projectRoot) {
+  EdenMount(EdenClientResourcePool pool, AbsPath mountPoint, AbsPath projectRoot) {
     Preconditions.checkArgument(
         projectRoot.startsWith(mountPoint),
         "Eden mount point %s must contain the Buck project at %s.",
@@ -63,32 +65,32 @@ public class EdenMount {
     this.pool = pool;
     this.mountPoint = mountPoint;
     this.projectRoot = projectRoot;
-    this.prefix = mountPoint.relativize(projectRoot);
+    this.prefix = ForwardRelPath.ofRelPath(mountPoint.relativize(projectRoot));
   }
 
   /** @return an Eden mount point if {@code projectRoot} is backed by Eden or {@code null}. */
   public static Optional<EdenMount> createEdenMountForProjectRoot(
-      Path projectRoot, EdenClientResourcePool pool) {
-    Optional<Path> rootPath = EdenUtil.getPathFromEdenConfig(projectRoot, "root");
+      AbsPath projectRoot, EdenClientResourcePool pool) {
+    Optional<Path> rootPath = EdenUtil.getPathFromEdenConfig(projectRoot.getPath(), "root");
     if (!rootPath.isPresent()) {
       return Optional.empty();
     }
 
-    return Optional.of(new EdenMount(pool, rootPath.get(), projectRoot));
+    return Optional.of(new EdenMount(pool, AbsPath.of(rootPath.get()), projectRoot));
   }
 
   /** @return The root to the Buck project that this {@link EdenMount} represents. */
-  public Path getProjectRoot() {
+  public AbsPath getProjectRoot() {
     return projectRoot;
   }
 
   @VisibleForTesting
-  Path getPrefix() {
+  ForwardRelPath getPrefix() {
     return prefix;
   }
 
   /** @param entry is a path that is relative to {@link #getProjectRoot()}. */
-  public Sha1HashCode getSha1(Path entry) throws EdenError, IOException, TException {
+  public Sha1HashCode getSha1(ForwardRelPath entry) throws EdenError, IOException, TException {
     try (EdenClientResource client = pool.openClient()) {
       List<SHA1Result> results =
           client
@@ -109,24 +111,28 @@ public Sha1HashCode getSha1(Path entry) throws EdenError, IOException, TExceptio
    * Returns the path relative to {@link #getProjectRoot()} if {@code path} is contained by {@link
    * #getProjectRoot()}; otherwise, returns {@link Optional#empty()}.
    */
-  Optional<Path> getPathRelativeToProjectRoot(Path path) {
+  Optional<ForwardRelPath> getPathRelativeToProjectRoot(Path path) {
     if (path.isAbsolute()) {
-      if (path.startsWith(projectRoot)) {
-        return Optional.of(projectRoot.relativize(path));
+      if (AbsPath.of(path).startsWith(projectRoot)) {
+        return Optional.of(ForwardRelPath.ofRelPath(projectRoot.relativize(path)));
       } else {
         return Optional.empty();
       }
     } else {
-      return Optional.of(path);
+      return Optional.of(ForwardRelPath.ofPath(path));
     }
   }
 
   /**
    * @param entry is a path that is relative to {@link #getProjectRoot()}.
    * @return a path that is relative to {@link #mountPoint}.
    */
-  private byte[] normalizePathArg(Path entry) {
-    return prefix.resolve(entry).toString().getBytes(StandardCharsets.UTF_8);
+  private byte[] normalizePathArg(ForwardRelPath entry) {
+    return prefix
+        .toRelPath(projectRoot.getFileSystem())
+        .resolve(entry)
+        .toString()
+        .getBytes(StandardCharsets.UTF_8);
   }
 
   @Override

src/com/facebook/buck/edenfs/EdenProjectFilesystemDelegate.java

@@ -232,7 +232,7 @@ private Optional<Sha1HashCode> computeSha1ViaXAttr(AbsPath path) throws IOExcept
 
   private Optional<Sha1HashCode> computeSha1ViaThrift(
       AbsPath path, boolean retryWithRealPathIfEdenError) throws IOException {
-    Optional<Path> entry = mount.getPathRelativeToProjectRoot(path.getPath());
+    Optional<ForwardRelPath> entry = mount.getPathRelativeToProjectRoot(path.getPath());
     if (entry.isPresent()) {
       try {
         return Optional.of(mount.getSha1(entry.get()));

src/com/facebook/buck/io/filesystem/impl/DefaultProjectFilesystemFactory.java

@@ -67,7 +67,7 @@ public DefaultProjectFilesystem createProjectFilesystem(
             embeddedCellBuckOutInfo,
             buckOutIncludeTargetConfigHash);
     ProjectFilesystemDelegatePair delegatePair =
-        ProjectFilesystemDelegateFactory.newInstance(root.getPath(), config, watchman);
+        ProjectFilesystemDelegateFactory.newInstance(root, config, watchman);
     return new DefaultProjectFilesystem(
         root,
         extractIgnorePaths(root.getPath(), config, buckPaths, embeddedCellBuckOutInfo),

src/com/facebook/buck/io/filesystem/impl/ProjectFilesystemDelegateFactory.java

@@ -26,7 +26,6 @@
 import com.facebook.buck.io.filesystem.ProjectFilesystemDelegatePair;
 import com.facebook.buck.io.watchman.Watchman;
 import com.facebook.buck.util.config.Config;
-import java.nio.file.Path;
 import java.util.Optional;
 
 /**
@@ -46,15 +45,16 @@ private static Optional<Long> getSHA1LoggingThresholdMicroseconds(Config config)
 
   /** Must always create a new delegate for the specified {@code root}. */
   public static ProjectFilesystemDelegatePair newInstance(
-      Path root, Config config, Watchman watchman) {
+      AbsPath root, Config config, Watchman watchman) {
     return new ProjectFilesystemDelegatePair(
         getGeneralDelegate(root, config, watchman),
         new DefaultProjectFilesystemDelegate(root, getSHA1LoggingThresholdMicroseconds(config)));
   }
 
   private static ProjectFilesystemDelegate getGeneralDelegate(
-      Path root, Config config, Watchman watchman) {
-    Optional<EdenClientResourcePool> pool = EdenClientResourcePool.tryToCreateEdenClientPool(root);
+      AbsPath root, Config config, Watchman watchman) {
+    Optional<EdenClientResourcePool> pool =
+        EdenClientResourcePool.tryToCreateEdenClientPool(root.getPath());
 
     if (pool.isPresent()) {
       Optional<EdenMount> mount = EdenMount.createEdenMountForProjectRoot(root, pool.get());
@@ -66,7 +66,7 @@ private static ProjectFilesystemDelegate getGeneralDelegate(
                 mount.get().getProjectRoot(), getSHA1LoggingThresholdMicroseconds(config)),
             config,
             watchman,
-            AbsPath.of(root));
+            root);
       } else {
         LOG.error("Failed to find Eden client for %s.", root);
       }

test/com/facebook/buck/edenfs/EdenClientTest.java

@@ -24,6 +24,8 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import com.facebook.buck.core.filesystems.AbsPath;
+import com.facebook.buck.core.filesystems.ForwardRelPath;
 import com.facebook.buck.testutil.PathNormalizer;
 import com.facebook.buck.util.environment.Platform;
 import com.facebook.eden.thrift.EdenError;
@@ -80,52 +82,59 @@ public void getMountInfosDelegatesToThriftClient() throws EdenError, IOException
 
   @Test
   public void getMountForMatchesProjectRootEqualToMount() throws IOException {
-    Path projectRoot = PathNormalizer.toWindowsPathIfNeeded(fs.getPath("/home/mbolin/src/eden"));
-    Files.createDirectories(projectRoot);
+    AbsPath projectRoot =
+        AbsPath.of(PathNormalizer.toWindowsPathIfNeeded(fs.getPath("/home/mbolin/src/eden")));
+    Files.createDirectories(projectRoot.getPath());
     if (Platform.detect() == Platform.WINDOWS) {
-      List<String> config = Arrays.asList("[Config]", "root=" + projectRoot.toString());
-      Files.createDirectories(PathNormalizer.toWindowsPathIfNeeded(projectRoot.resolve(".eden")));
+      List<String> config = Arrays.asList("[Config]", "root=" + projectRoot);
+      Files.createDirectories(
+          PathNormalizer.toWindowsPathIfNeeded(projectRoot.resolve(".eden").getPath()));
       Path configFile =
           Files.createFile(
-              PathNormalizer.toWindowsPathIfNeeded(projectRoot.resolve(".eden").resolve("config")));
+              PathNormalizer.toWindowsPathIfNeeded(
+                  projectRoot.resolve(".eden").resolve("config").getPath()));
       Files.write(configFile, config);
     } else {
-      Files.createDirectories(projectRoot.resolve(".eden"));
-      Files.createSymbolicLink(projectRoot.resolve(".eden").resolve("root"), projectRoot);
+      Files.createDirectories(projectRoot.resolve(".eden").getPath());
+      Files.createSymbolicLink(
+          projectRoot.resolve(".eden").resolve("root").getPath(), projectRoot.getPath());
     }
 
     Optional<EdenMount> mount = EdenMount.createEdenMountForProjectRoot(projectRoot, pool);
     assertTrue("Should find mount for path.", mount.isPresent());
     assertEquals(
-        PathNormalizer.toWindowsPathIfNeeded(fs.getPath("/home/mbolin/src/eden")),
+        AbsPath.of(PathNormalizer.toWindowsPathIfNeeded(fs.getPath("/home/mbolin/src/eden"))),
         mount.get().getProjectRoot());
-    assertEquals(fs.getPath(""), mount.get().getPrefix());
+    assertEquals(ForwardRelPath.EMPTY, mount.get().getPrefix());
   }
 
   @Test
   public void getMountForMatchesProjectRootUnderMount() throws IOException {
-    Path edenMountRoot = fs.getPath("/home/mbolin/src/eden");
-    Path projectRoot = fs.getPath("/home/mbolin/src/eden/deep/project");
-    Files.createDirectories(projectRoot);
+    AbsPath edenMountRoot = AbsPath.of(fs.getPath("/home/mbolin/src/eden").toAbsolutePath());
+    AbsPath projectRoot =
+        AbsPath.of(fs.getPath("/home/mbolin/src/eden/deep/project").toAbsolutePath());
+    Files.createDirectories(projectRoot.getPath());
     if (Platform.detect() == Platform.WINDOWS) {
       List<String> config = Arrays.asList("[Config]", "root=/home/mbolin/src/eden");
-      Files.createDirectories(edenMountRoot.resolve(".eden"));
-      Path configFile = Files.createFile(edenMountRoot.resolve(".eden").resolve("config"));
+      Files.createDirectories(edenMountRoot.resolve(".eden").getPath());
+      Path configFile =
+          Files.createFile(edenMountRoot.resolve(".eden").resolve("config").getPath());
       Files.write(configFile, config);
     } else {
-      Files.createDirectories(projectRoot.resolve(".eden"));
-      Files.createSymbolicLink(projectRoot.resolve(".eden").resolve("root"), edenMountRoot);
+      Files.createDirectories(projectRoot.resolve(".eden").getPath());
+      Files.createSymbolicLink(
+          projectRoot.resolve(".eden").resolve("root").getPath(), edenMountRoot.getPath());
     }
 
     Optional<EdenMount> mount = EdenMount.createEdenMountForProjectRoot(projectRoot, pool);
     assertTrue("Should find mount for path.", mount.isPresent());
     assertEquals(projectRoot, mount.get().getProjectRoot());
-    assertEquals(fs.getPath("deep/project"), mount.get().getPrefix());
+    assertEquals(ForwardRelPath.of("deep/project"), mount.get().getPrefix());
   }
 
   @Test
   public void getMountForReturnsNullWhenMissingMountPoint() {
-    Path projectRoot = Paths.get("/home/mbolin/src/other_project");
+    AbsPath projectRoot = AbsPath.of(Paths.get("/home/mbolin/src/other_project").toAbsolutePath());
     Optional<EdenMount> mount = EdenMount.createEdenMountForProjectRoot(projectRoot, pool);
     assertFalse(mount.isPresent());
   }

test/com/facebook/buck/edenfs/EdenMountTest.java

@@ -23,6 +23,8 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
+import com.facebook.buck.core.filesystems.AbsPath;
+import com.facebook.buck.core.filesystems.ForwardRelPath;
 import com.facebook.buck.testutil.DeepMatcher;
 import com.facebook.buck.testutil.PathNormalizer;
 import com.facebook.buck.util.environment.Platform;
@@ -52,7 +54,7 @@ public void getSha1DelegatesToThriftClient() throws EdenError, IOException, TExc
     EdenClient thriftClient = createMock(EdenClient.class);
 
     FileSystem fs = Jimfs.newFileSystem(Configuration.unix());
-    Path entry = fs.getPath("LICENSE");
+    ForwardRelPath entry = ForwardRelPath.of("LICENSE");
     HashCode hash = HashCode.fromString("2b8b815229aa8a61e483fb4ba0588b8b6c491890");
     SHA1Result sha1Result = new SHA1Result();
     sha1Result.setSha1(hash.asBytes());
@@ -67,17 +69,21 @@ public void getSha1DelegatesToThriftClient() throws EdenError, IOException, TExc
 
     EdenClientResourcePool pool =
         new EdenClientResourcePool(() -> new TestEdenClientResource(thriftClient));
-    Path pathToBuck = PathNormalizer.toWindowsPathIfNeeded(fs.getPath("/home/mbolin/src/buck"));
+    AbsPath pathToBuck =
+        AbsPath.of(PathNormalizer.toWindowsPathIfNeeded(fs.getPath("/home/mbolin/src/buck")));
     if (Platform.detect() == Platform.WINDOWS) {
-      List<String> config = Arrays.asList("[Config]", "root=" + pathToBuck.toString());
-      Files.createDirectories(PathNormalizer.toWindowsPathIfNeeded(pathToBuck.resolve(".eden")));
+      List<String> config = Arrays.asList("[Config]", "root=" + pathToBuck);
+      Files.createDirectories(
+          PathNormalizer.toWindowsPathIfNeeded(pathToBuck.resolve(".eden").getPath()));
       Path configFile =
           Files.createFile(
-              PathNormalizer.toWindowsPathIfNeeded(pathToBuck.resolve(".eden").resolve("config")));
+              PathNormalizer.toWindowsPathIfNeeded(
+                  pathToBuck.resolve(".eden").resolve("config").getPath()));
       Files.write(configFile, config);
     } else {
-      Files.createDirectories(pathToBuck.resolve(".eden"));
-      Files.createSymbolicLink(pathToBuck.resolve(".eden").resolve("root"), pathToBuck);
+      Files.createDirectories(pathToBuck.resolve(".eden").getPath());
+      Files.createSymbolicLink(
+          pathToBuck.resolve(".eden").resolve("root").getPath(), pathToBuck.getPath());
     }
 
     Optional<EdenMount> mount = EdenMount.createEdenMountForProjectRoot(pathToBuck, pool);
@@ -91,7 +97,7 @@ public void getMountPointReturnsValuePassedToConstructor() {
     EdenClient thriftClient = createMock(EdenClient.class);
     EdenClientResourcePool pool =
         new EdenClientResourcePool(() -> new TestEdenClientResource(thriftClient));
-    Path mountPoint = Paths.get("/home/mbolin/src/buck");
+    AbsPath mountPoint = AbsPath.of(Paths.get("/home/mbolin/src/buck").toAbsolutePath());
     replay(thriftClient);
 
     EdenMount mount = new EdenMount(pool, mountPoint, mountPoint);
@@ -105,7 +111,7 @@ public void toStringHasExpectedFormatting() {
     EdenClient thriftClient = createMock(EdenClient.class);
     EdenClientResourcePool pool =
         new EdenClientResourcePool(() -> new TestEdenClientResource(thriftClient));
-    Path mountPoint = Paths.get("/home/mbolin/src/buck");
+    AbsPath mountPoint = AbsPath.of(Paths.get("/home/mbolin/src/buck").toAbsolutePath());
     replay(thriftClient);
 
     EdenMount mount = new EdenMount(pool, mountPoint, mountPoint);

test/com/facebook/buck/edenfs/EdenProjectFilesystemDelegateTest.java

@@ -27,6 +27,7 @@
 
 import com.facebook.buck.cli.TestWithBuckd;
 import com.facebook.buck.core.filesystems.AbsPath;
+import com.facebook.buck.core.filesystems.ForwardRelPath;
 import com.facebook.buck.event.console.TestEventConsole;
 import com.facebook.buck.io.filesystem.ProjectFilesystem;
 import com.facebook.buck.io.filesystem.ProjectFilesystemDelegate;
@@ -105,14 +106,15 @@ public void computeSha1ForOrdinaryFileUnderMount() throws IOException, EdenError
         new DefaultProjectFilesystemDelegate(root, Optional.empty());
 
     EdenMount mount = createMock(EdenMount.class);
-    Path path = fs.getPath("foo/bar");
-    expect(mount.getPathRelativeToProjectRoot(root.resolve(path))).andReturn(Optional.of(path));
+    ForwardRelPath path = ForwardRelPath.of("foo/bar");
+    expect(mount.getPathRelativeToProjectRoot(root.resolve(fs.getPath(path.toString()))))
+        .andReturn(Optional.of(path));
     expect(mount.getSha1(path)).andReturn(DUMMY_SHA1);
     replay(mount);
 
     EdenProjectFilesystemDelegate edenDelegate =
         new EdenProjectFilesystemDelegate(mount, delegate, AbsPath.of(root));
-    assertEquals(DUMMY_SHA1, edenDelegate.computeSha1(path));
+    assertEquals(DUMMY_SHA1, edenDelegate.computeSha1(path.toPath(fs)));
 
     verify(mount);
   }
@@ -134,10 +136,12 @@ public void computeSha1ForSymlinkUnderMountThatPointsToFileUnderMount()
     // Eden will throw when the SHA-1 for the link is requested, but return a SHA-1 when the target
     // is requested.
     EdenMount mount = createMock(EdenMount.class);
-    expect(mount.getPathRelativeToProjectRoot(link)).andReturn(Optional.of(fs.getPath("link")));
-    expect(mount.getPathRelativeToProjectRoot(target)).andReturn(Optional.of(fs.getPath("target")));
-    expect(mount.getSha1(fs.getPath("link"))).andThrow(new EdenError());
-    expect(mount.getSha1(fs.getPath("target"))).andReturn(DUMMY_SHA1);
+    expect(mount.getPathRelativeToProjectRoot(link))
+        .andReturn(Optional.of(ForwardRelPath.of("link")));
+    expect(mount.getPathRelativeToProjectRoot(target))
+        .andReturn(Optional.of(ForwardRelPath.of("target")));
+    expect(mount.getSha1(ForwardRelPath.of("link"))).andThrow(new EdenError());
+    expect(mount.getSha1(ForwardRelPath.of("target"))).andReturn(DUMMY_SHA1);
     replay(mount);
 
     EdenProjectFilesystemDelegate edenDelegate =
@@ -166,9 +170,10 @@ public void computeSha1ForSymlinkUnderMountThatPointsToFileOutsideMount()
     // Eden will throw when the SHA-1 for the link is requested, but return a SHA-1 when the target
     // is requested.
     EdenMount mount = createMock(EdenMount.class);
-    expect(mount.getPathRelativeToProjectRoot(link)).andReturn(Optional.of(fs.getPath("link")));
+    expect(mount.getPathRelativeToProjectRoot(link))
+        .andReturn(Optional.of(ForwardRelPath.of("link")));
     expect(mount.getPathRelativeToProjectRoot(target)).andReturn(Optional.empty());
-    expect(mount.getSha1(fs.getPath("link"))).andThrow(new EdenError());
+    expect(mount.getSha1(ForwardRelPath.of("link"))).andThrow(new EdenError());
     replay(mount);
 
     EdenProjectFilesystemDelegate edenDelegate =
@@ -346,8 +351,9 @@ public void computeSha1ViaWatchmanFailedWatchmanNotSetup()
             "[eden]", "use_watchman_content_sha1 = true", "[eden]", "use_xattr = false");
 
     EdenMount mount = createMock(EdenMount.class);
-    Path path = fs.getPath("foo/bar");
-    expect(mount.getPathRelativeToProjectRoot(root.resolve(path))).andReturn(Optional.of(path));
+    ForwardRelPath path = ForwardRelPath.of("foo/bar");
+    expect(mount.getPathRelativeToProjectRoot(root.resolve(path.toPath(fs))))
+        .andReturn(Optional.of(path));
     expect(mount.getSha1(path)).andReturn(DUMMY_SHA1);
     replay(mount);
 
@@ -362,7 +368,7 @@ public void computeSha1ViaWatchmanFailedWatchmanNotSetup()
             new WatchmanFactory.NullWatchman(
                 "EdenProjectFilesystemDelegateTest", WatchmanError.TEST),
             AbsPath.of(root));
-    edenDelegate.computeSha1(path);
+    edenDelegate.computeSha1(path.toPath(fs));
   }
 
   @Test