Canonicalize the parent path in readSymbolicLink/createSymbolicLink a…

…nd use the ActionInputMap as a source of information for readSymbolicLink.

Even though readSymbolicLink/createSymbolic link don't canonicalize the full path, they still need to canonicalize the parent path to be correct in the presence of cross-filesystem symlinks.

Files in the ActionInputMap must participate in the resolveSymbolicLinks algorithm even when they're not present in the local filesystem.

PiperOrigin-RevId: 559400472
Change-Id: Ia0354b57d0ef543e64301fdacadcb3b91363ea4d

src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java

@@ -36,6 +36,7 @@
 import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
 import com.google.devtools.build.lib.actions.FileArtifactValue;
 import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
+import com.google.devtools.build.lib.actions.FileArtifactValue.UnresolvedSymlinkArtifactValue;
 import com.google.devtools.build.lib.actions.FileStatusWithMetadata;
 import com.google.devtools.build.lib.actions.InputMetadataProvider;
 import com.google.devtools.build.lib.actions.cache.MetadataInjector;
@@ -549,19 +550,46 @@ 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());
+    }
+
+    if (path.startsWith(execRoot)) {
+      var execPath = path.relativeTo(execRoot);
+      var metadata = inputArtifactData.getMetadata(execPath);
+      if (metadata instanceof UnresolvedSymlinkArtifactValue) {
+        return PathFragment.create(((UnresolvedSymlinkArtifactValue) metadata).getSymlinkTarget());
+      }
+      if (metadata != null) {
+        // Other input artifacts are never symlinks.
+        throw new NotASymlinkException(path);
+      }
+      if (inputTreeArtifactDirectoryCache.get(execPath) != null) {
+        // Tree artifacts never contain symlinks.
+        throw new NotASymlinkException(path);
+      }
+    }
+
     if (isOutput(path)) {
       try {
         return remoteOutputTree.getPath(path).readSymbolicLink();
       } catch (FileNotFoundException e) {
         // Intentionally ignored.
       }
     }
+
     return localFs.getPath(path).readSymbolicLink();
   }
 
   @Override
   protected void createSymbolicLink(PathFragment linkPath, PathFragment targetFragment)
       throws IOException {
+    PathFragment parentPath = linkPath.getParentDirectory();
+    if (parentPath != null) {
+      linkPath = resolveSymbolicLinks(parentPath).asFragment().getChild(linkPath.getBaseName());
+    }
+
     if (isOutput(linkPath)) {
       remoteOutputTree.getPath(linkPath).createSymbolicLink(targetFragment);
     }

src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java

@@ -681,6 +681,50 @@ public void readSymbolicLink_fromRemoteFilesystem() throws Exception {
     assertThrows(NotASymlinkException.class, () -> actionFs.readSymbolicLink(filePath));
   }
 
+  @Test
+  public void readSymbolicLink_fromInputArtifactData_regularFile() throws Exception {
+    ActionInputMap inputs = new ActionInputMap(1);
+    Artifact artifact = createRemoteArtifact("file", "contents", inputs);
+    RemoteActionFileSystem actionFs = (RemoteActionFileSystem) createActionFileSystem(inputs);
+
+    assertThrows(
+        NotASymlinkException.class,
+        () -> actionFs.readSymbolicLink(artifact.getPath().asFragment()));
+  }
+
+  @Test
+  public void readSymbolicLink_fromInputArtifactData_treeSubDir() throws Exception {
+    ActionInputMap inputs = new ActionInputMap(1);
+    SpecialArtifact tree =
+        createRemoteTreeArtifact("tree", ImmutableMap.of("subdir/file", ""), inputs);
+    RemoteActionFileSystem actionFs = (RemoteActionFileSystem) createActionFileSystem(inputs);
+
+    assertThrows(
+        NotASymlinkException.class, () -> actionFs.readSymbolicLink(tree.getPath().asFragment()));
+
+    assertThrows(
+        NotASymlinkException.class,
+        () -> actionFs.readSymbolicLink(tree.getPath().getRelative("subdir").asFragment()));
+  }
+
+  @Test
+  public void readSymbolicLink_fromInputArtifactData_unresolvedSymlink() throws Exception {
+    ActionInputMap inputs = new ActionInputMap(1);
+    RemoteActionFileSystem actionFs = (RemoteActionFileSystem) createActionFileSystem(inputs);
+
+    Artifact symlink = ActionsTestUtil.createUnresolvedSymlinkArtifact(outputRoot, "symlink");
+    PathFragment targetPath = PathFragment.create("/some/path");
+    // Create symlink on the filesystem so we can digest it, then delete it to verify that its
+    // presence in the ActionInputMap is sufficient for readSymbolicLink to work. Note that this is
+    // an unrealistic scenario, as symlinks are always materialized even when produced remotely.
+    Path symlinkPath = getLocalFileSystem(actionFs).getPath(symlink.getPath().getPathString());
+    symlinkPath.createSymbolicLink(targetPath);
+    inputs.putWithNoDepOwner(symlink, FileArtifactValue.createForUnresolvedSymlink(symlinkPath));
+    symlinkPath.delete();
+
+    assertThat(actionFs.readSymbolicLink(getOutputPath("symlink"))).isEqualTo(targetPath);
+  }
+
   @Test
   public void readSymbolicLink_notFound() throws Exception {
     RemoteActionFileSystem actionFs = (RemoteActionFileSystem) createActionFileSystem();
@@ -857,6 +901,28 @@ public void createSymbolicLink_unresolvedSymlink() throws Exception {
     verifyNoInteractions(metadataInjector);
   }
 
+  @Test
+  public void createAndReadSymbolicLink_followSymlinks(@TestParameter FilesystemTestParam from)
+      throws Exception {
+    // createSymbolicLink writes to both the local and remote filesystem, so it makes no sense to
+    // parameterize on the symlink's destination filesystem.
+    RemoteActionFileSystem actionFs = (RemoteActionFileSystem) createActionFileSystem();
+    FileSystem fromFs = from.getFilesystem(actionFs);
+
+    PathFragment parentLinkPath = getOutputPath("parent_link");
+    PathFragment parentTargetPath = getOutputPath("parent_target");
+    fromFs
+        .getPath(parentLinkPath)
+        .createSymbolicLink(execRoot.getRelative(parentTargetPath).asFragment());
+    actionFs.getPath(parentTargetPath).createDirectoryAndParents();
+
+    PathFragment linkPath = getOutputPath("parent_target/link");
+    PathFragment targetPath = PathFragment.create("/some/path");
+    actionFs.getPath(linkPath).createSymbolicLink(targetPath);
+
+    assertThat(actionFs.getPath(linkPath).readSymbolicLink()).isEqualTo(targetPath);
+  }
+
   @Test
   public void resolveSymbolicLinks(
       @TestParameter FilesystemTestParam a, @TestParameter FilesystemTestParam b) throws Exception {