diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java index 27b162a9027dfc..449faadcacb9fb 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java @@ -291,8 +291,24 @@ public String getFileSystemType(PathFragment path) { return "remoteActionFS"; } + // Like resolveSymbolicLinks(), except that only the parent path is canonicalized. + private PathFragment resolveSymbolicLinksForParent(PathFragment path) throws IOException { + PathFragment parentPath = path.getParentDirectory(); + if (parentPath != null) { + return resolveSymbolicLinks(parentPath).asFragment().getChild(path.getBaseName()); + } + return path; + } + @Override protected boolean delete(PathFragment path) throws IOException { + try { + path = resolveSymbolicLinksForParent(path); + } catch (FileNotFoundException ignored) { + // Failure to delete a nonexistent path is not an error. + return false; + } + boolean deleted = localFs.getPath(path).delete(); if (isOutput(path)) { deleted = remoteOutputTree.getPath(path).delete() || deleted; @@ -462,10 +478,7 @@ protected void chmod(PathFragment path, int mode) throws IOException { @Override protected PathFragment readSymbolicLink(PathFragment path) throws IOException { - PathFragment parentPath = path.getParentDirectory(); - if (parentPath != null) { - path = resolveSymbolicLinks(parentPath).asFragment().getChild(path.getBaseName()); - } + path = resolveSymbolicLinksForParent(path); if (path.startsWith(execRoot)) { var execPath = path.relativeTo(execRoot); @@ -497,10 +510,7 @@ protected PathFragment readSymbolicLink(PathFragment path) throws IOException { @Override protected void createSymbolicLink(PathFragment linkPath, PathFragment targetFragment) throws IOException { - PathFragment parentPath = linkPath.getParentDirectory(); - if (parentPath != null) { - linkPath = resolveSymbolicLinks(parentPath).asFragment().getChild(linkPath.getBaseName()); - } + linkPath = resolveSymbolicLinksForParent(linkPath); if (isOutput(linkPath)) { remoteOutputTree.getPath(linkPath).createSymbolicLink(targetFragment); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java index 711cda52ad4b11..d123c1d5062dcb 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java @@ -460,6 +460,49 @@ public void statAndExists_notFound() throws Exception { FileNotFoundException.class, () -> actionFs.stat(path, /* followSymlinks= */ true)); } + @Test + public void delete_deleteSymlink() throws Exception { + RemoteActionFileSystem actionFs = (RemoteActionFileSystem) createActionFileSystem(); + + PathFragment linkPath = getOutputPath("link"); + PathFragment targetPath = getOutputPath("target"); + actionFs.getPath(linkPath).createSymbolicLink(execRoot.getRelative(targetPath).asFragment()); + writeLocalFile(actionFs, targetPath, "content"); + + assertThat(actionFs.delete(linkPath)).isTrue(); + assertThat(actionFs.exists(linkPath, /* followSymlinks= */ false)).isFalse(); + assertThat(actionFs.exists(targetPath, /* followSymlinks= */ false)).isTrue(); + } + + @Test + public void delete_followSymlinks( + @TestParameter FilesystemTestParam from, @TestParameter FilesystemTestParam to) + throws Exception { + RemoteActionFileSystem actionFs = (RemoteActionFileSystem) createActionFileSystem(); + FileSystem fromFs = from.getFilesystem(actionFs); + FileSystem toFs = to.getFilesystem(actionFs); + + PathFragment dirLinkPath = getOutputPath("dirLink"); + PathFragment dirTargetPath = getOutputPath("dirTarget"); + fromFs + .getPath(dirLinkPath) + .createSymbolicLink(execRoot.getRelative(dirTargetPath).asFragment()); + actionFs.getPath(dirTargetPath).createDirectoryAndParents(); + + PathFragment naivePath = dirLinkPath.getChild("file"); + PathFragment canonicalPath = dirTargetPath.getChild("file"); + + if (toFs.equals(actionFs.getLocalFileSystem())) { + writeLocalFile(actionFs, canonicalPath, "content"); + } else { + injectRemoteFile(actionFs, canonicalPath, "content"); + } + + assertThat(actionFs.delete(naivePath)).isTrue(); + assertThat(actionFs.exists(naivePath, /* followSymlinks= */ false)).isFalse(); + assertThat(actionFs.exists(canonicalPath, /* followSymlinks= */ false)).isFalse(); + } + @Test public void setLastModifiedTime_forRemoteOutputTree() throws Exception { RemoteActionFileSystem actionFs = (RemoteActionFileSystem) createActionFileSystem();