Propagate the tool bit to logged directory inputs.

PiperOrigin-RevId: 588834575 Change-Id: I119063c6c06353bf71a12d076d98f3d2a9a76f33
davido · Dec 7, 2023 · 8b55ad2 · 8b55ad2
1 parent d9edb1d
commit 8b55ad2
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 10 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContext.java
@@ -159,24 +159,30 @@ public void logSpawn(
       for (Map.Entry<PathFragment, ActionInput> e : inputMap.entrySet()) {
         PathFragment displayPath = e.getKey();
         ActionInput input = e.getValue();
+
         if (input instanceof VirtualActionInput.EmptyActionInput) {
           // Do not include a digest, as it's a waste of space.
           builder.addInputsBuilder().setPath(displayPath.getPathString());
           continue;
         }
+
+        boolean isTool =
+            toolFiles.contains(input)
+                || (input instanceof TreeFileArtifact
+                    && toolFiles.contains(((TreeFileArtifact) input).getParent()));
+
         Path contentPath = fileSystem.getPath(execRoot.getRelative(input.getExecPathString()));
+
         if (contentPath.isDirectory()) {
           listDirectoryContents(
-              displayPath, contentPath, builder::addInputs, inputMetadataProvider);
+              displayPath, contentPath, builder::addInputs, inputMetadataProvider, isTool);
           continue;
         }
+
         Digest digest =
             computeDigest(
                 input, contentPath, inputMetadataProvider, xattrProvider, digestHashFunction);
-        boolean isTool =
-            toolFiles.contains(input)
-                || (input instanceof TreeFileArtifact
-                    && toolFiles.contains(((TreeFileArtifact) input).getParent()));
+
         builder
             .addInputsBuilder()
             .setPath(displayPath.getPathString())
@@ -199,7 +205,11 @@ public void logSpawn(
           ActionInput output = e.getValue();
           if (path.isDirectory()) {
             listDirectoryContents(
-                output.getExecPath(), path, builder::addActualOutputs, inputMetadataProvider);
+                output.getExecPath(),
+                path,
+                builder::addActualOutputs,
+                inputMetadataProvider,
+                /* isTool= */ false);
             continue;
           }
           builder
@@ -306,7 +316,8 @@ private void listDirectoryContents(
       PathFragment displayPath,
       Path contentPath,
       Consumer<File> addFile,
-      InputMetadataProvider inputMetadataProvider)
+      InputMetadataProvider inputMetadataProvider,
+      boolean isTool)
       throws IOException {
     // TODO(olaola): once symlink API proposal is implemented, report symlinks here.
     List<Dirent> sortedDirent = new ArrayList<>(contentPath.readdir(Symlinks.NOFOLLOW));
@@ -318,7 +329,8 @@ private void listDirectoryContents(
       Path childContentPath = contentPath.getChild(name);
 
       if (dirent.getType() == Dirent.Type.DIRECTORY) {
-        listDirectoryContents(childDisplayPath, childContentPath, addFile, inputMetadataProvider);
+        listDirectoryContents(
+            childDisplayPath, childContentPath, addFile, inputMetadataProvider, isTool);
         continue;
       }
 
@@ -332,6 +344,7 @@ private void listDirectoryContents(
                       inputMetadataProvider,
                       xattrProvider,
                       digestHashFunction))
+              .setIsTool(isTool)
               .build());
     }
   }

diff --git a/src/test/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContextTest.java b/src/test/java/com/google/devtools/build/lib/exec/ExpandedSpawnLogContextTest.java
@@ -190,15 +190,18 @@ public void testDirectoryInput(
         defaultTimeout(),
         defaultSpawnResult());
 
-    // TODO(tjgq): Propagate tool bit to files inside source directories.
     closeAndAssertLog(
         context,
         defaultSpawnExecBuilder()
             .addAllInputs(
                 dirContents.isEmpty()
                     ? ImmutableList.of()
                     : ImmutableList.of(
-                        File.newBuilder().setPath("dir/file").setDigest(getDigest("abc")).build()))
+                        File.newBuilder()
+                            .setPath("dir/file")
+                            .setDigest(getDigest("abc"))
+                            .setIsTool(inputsMode.isTool())
+                            .build()))
             .build());
   }