Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Commit

Permalink
Refactor EdenMount to use proper absolute and relative paths
Browse files Browse the repository at this point in the history 
Summary:
Instead of just `Path`.

To make it clear which paths are which.

Reviewed By: xavierd

fbshipit-source-id: 18b6c26eafaaea6fb74dbeb3f74a53a0ec9d0433
  • Loading branch information
@stepancheg @facebook-github-bot
stepancheg authored and facebook-github-bot committed Sep 14, 2021
1 parent 95b90ef commit d687248
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 67 deletions.
40 changes: 23 additions & 17 deletions src/com/facebook/buck/edenfs/EdenMount.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.",
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/com/facebook/buck/edenfs/EdenProjectFilesystemDelegate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down
2 changes: 1 addition & 1 deletion src/com/facebook/buck/io/filesystem/impl/DefaultProjectFilesystemFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
10 changes: 5 additions & 5 deletions src/com/facebook/buck/io/filesystem/impl/ProjectFilesystemDelegateFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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());
Expand All @@ -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);
}
Expand Down
45 changes: 27 additions & 18 deletions test/com/facebook/buck/edenfs/EdenClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down
24 changes: 15 additions & 9 deletions test/com/facebook/buck/edenfs/EdenMountTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
30 changes: 18 additions & 12 deletions test/com/facebook/buck/edenfs/EdenProjectFilesystemDelegateTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand All @@ -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 =
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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);

Expand All @@ -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
Expand Down
Loading

0 comments on commit d687248

Please sign in to comment.