[7.1.0] Correctly handle unresolved symlinks when they appear in the …

…inputs. (#21181)

This should have been a part of
9fd9aa5.

PiperOrigin-RevId: 602999711
Change-Id: Ic817e5682d8fbbb477e03a0a7e217c61735cc90b

src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java

@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.actions;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static java.nio.charset.StandardCharsets.ISO_8859_1;
 
@@ -282,6 +283,11 @@ public static FileArtifactValue createForVirtualActionInput(byte[] digest, long
     return new RegularFileArtifactValue(digest, /* proxy= */ null, size);
   }
 
+  public static FileArtifactValue createForUnresolvedSymlink(Artifact artifact) throws IOException {
+    checkArgument(artifact.isSymlink());
+    return createForUnresolvedSymlink(artifact.getPath());
+  }
+
   public static FileArtifactValue createForUnresolvedSymlink(Path symlink) throws IOException {
     return new UnresolvedSymlinkArtifactValue(symlink);
   }

src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java

@@ -293,6 +293,8 @@ private int logNestedSet(
             Path path = fileSystem.getPath(execRoot.getRelative(input.getExecPath()));
             if (isInputDirectory(input, inputMetadataProvider)) {
               builder.addDirectoryIds(logDirectory(input, path, inputMetadataProvider));
+            } else if (input.isSymlink()) {
+              builder.addUnresolvedSymlinkIds(logUnresolvedSymlink(input, path));
             } else {
               builder.addFileIds(logFile(input, path, inputMetadataProvider));
             }
@@ -306,7 +308,8 @@ private int logNestedSet(
    * Logs a file.
    *
    * @param input the input representing the file.
-   * @param path the path to the file
+   * @param path the path to the file, which must have already been verified to be of the correct
+   *     type.
    * @return the entry ID of the {@link ExecLogEntry.File} describing the file.
    */
   private int logFile(ActionInput input, Path path, InputMetadataProvider inputMetadataProvider)

src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java

@@ -180,7 +180,8 @@ public void logSpawn(
             builder
                 .addInputsBuilder()
                 .setPath(displayPath.getPathString())
-                .setSymlinkTargetPath(metadata.getSymlinkTarget());
+                .setSymlinkTargetPath(metadata.getSymlinkTarget())
+                .setIsTool(isTool);
             continue;
           }
 

src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java

@@ -115,7 +115,7 @@ static boolean isInputDirectory(ActionInput input, InputMetadataProvider inputMe
     if (input.isDirectory()) {
       return true;
     }
-    if (!(input instanceof SourceArtifact)) {
+    if (input.isSymlink() || !(input instanceof SourceArtifact)) {
       return false;
     }
     // A source artifact may be a directory in spite of claiming to be a file. Avoid unnecessary I/O

src/main/protobuf/spawn.proto

@@ -248,17 +248,19 @@ message ExecLogEntry {
   }
 
   // A set of spawn inputs.
-  // The contents of the set are the directly referenced files and directories
-  // in addition to the contents of all transitively referenced sets. Sets are
-  // not canonical: two sets with different structure may yield the same
-  // contents.
+  // The contents of the set are the directly referenced files, directories and
+  // symlinks in addition to the contents of all transitively referenced sets.
+  // Sets are not canonical: two sets with different structure may yield the
+  // same contents.
   message InputSet {
     // Entry IDs of files belonging to this set.
     repeated int32 file_ids = 1;
     // Entry IDs of directories belonging to this set.
     repeated int32 directory_ids = 2;
+    // Entry IDs of unresolved symlinks belonging to this set.
+    repeated int32 unresolved_symlink_ids = 3;
     // Entry IDs of other sets contained in this set.
-    repeated int32 transitive_set_ids = 3;
+    repeated int32 transitive_set_ids = 4;
   }
 
   // A spawn output.

src/test/java/com/google/devtools/build/lib/exec/CompactSpawnLogContextTest.java

@@ -240,6 +240,11 @@ private SortedMap<String, File> reconstructInputs(
           inputs.put(dirFile.getPath(), reconstructFile(dir, dirFile, hashFunctionName));
         }
       }
+      for (int symlinkId : set.getUnresolvedSymlinkIdsList()) {
+        ExecLogEntry.UnresolvedSymlink symlink =
+            checkNotNull(entryMap.get(symlinkId)).getUnresolvedSymlink();
+        inputs.put(symlink.getPath(), reconstructSymlink(symlink));
+      }
       setsToVisit.addAll(set.getTransitiveSetIdsList());
     }
     return inputs;

src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java

@@ -249,6 +249,39 @@ public void testTreeInput(
             .build());
   }
 
+  @Test
+  public void testUnresolvedSymlinkInput(@TestParameter InputsMode inputsMode) throws Exception {
+    Artifact symlinkInput = ActionsTestUtil.createUnresolvedSymlinkArtifact(outputDir, "symlink");
+
+    symlinkInput.getPath().getParentDirectory().createDirectoryAndParents();
+    symlinkInput.getPath().createSymbolicLink(PathFragment.create("/some/path"));
+
+    SpawnBuilder spawn = defaultSpawnBuilder().withInputs(symlinkInput);
+    if (inputsMode.isTool()) {
+      spawn.withTools(symlinkInput);
+    }
+
+    SpawnLogContext context = createSpawnLogContext();
+
+    context.logSpawn(
+        spawn.build(),
+        createInputMetadataProvider(symlinkInput),
+        createInputMap(symlinkInput),
+        fs,
+        defaultTimeout(),
+        defaultSpawnResult());
+
+    closeAndAssertLog(
+        context,
+        defaultSpawnExecBuilder()
+            .addInputs(
+                File.newBuilder()
+                    .setPath("out/symlink")
+                    .setSymlinkTargetPath("/some/path")
+                    .setIsTool(inputsMode.isTool()))
+            .build());
+  }
+
   @Test
   public void testRunfilesFileInput() throws Exception {
     Artifact runfilesInput = ActionsTestUtil.createArtifact(rootDir, "data.txt");
@@ -862,6 +895,8 @@ protected static InputMetadataProvider createInputMetadataProvider(Artifact... a
             treeMetadata.getChildValues().entrySet()) {
           builder.put(entry.getKey(), entry.getValue());
         }
+      } else if (artifact.isSymlink()) {
+        builder.put(artifact, FileArtifactValue.createForUnresolvedSymlink(artifact));
       } else {
         builder.put(artifact, FileArtifactValue.createForTesting(artifact));
       }