Fix file counting in merkletree.DirectoryTreeBuilder

The DirectoryTreeBuilder did not check if files already existed in the resulting map, so the file counter got wrong and an assertion failed. Fixes bazelbuild#14286.
moroten · Nov 19, 2021 · 8c927cd · 8c927cd
1 parent a729f63
commit 8c927cd
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 9 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTree.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTree.java
@@ -148,8 +148,8 @@ static class DirectoryNode extends Node {
       super(pathSegment);
     }
 
-    void addChild(Node child) {
-      children.add(Preconditions.checkNotNull(child, "child"));
+    boolean addChild(Node child) {
+      return children.add(Preconditions.checkNotNull(child, "child"));
     }
 
     @Override

diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java
@@ -101,8 +101,9 @@ private static int buildFromPaths(
             throw new IOException(String.format("Input '%s' is not a file.", input));
           }
           Digest d = digestUtil.compute(input);
-          currDir.addChild(FileNode.createExecutable(path.getBaseName(), input, d));
-          return 1;
+          boolean childAdded = currDir.addChild(
+              FileNode.createExecutable(path.getBaseName(), input, d));
+          return childAdded ? 1 : 0;
         });
   }
 
@@ -120,16 +121,16 @@ private static int buildFromActionInputs(
       DigestUtil digestUtil,
       Map<PathFragment, DirectoryNode> tree)
       throws IOException {
-    return build(
+    int ret = build(
         inputs,
         tree,
         (input, path, currDir) -> {
           if (input instanceof VirtualActionInput) {
             VirtualActionInput virtualActionInput = (VirtualActionInput) input;
             Digest d = digestUtil.compute(virtualActionInput);
-            currDir.addChild(
+            boolean childAdded = currDir.addChild(
                 FileNode.createExecutable(path.getBaseName(), virtualActionInput.getBytes(), d));
-            return 1;
+            return childAdded ? 1 : 0;
           }
 
           FileArtifactValue metadata =
@@ -141,8 +142,9 @@ private static int buildFromActionInputs(
             case REGULAR_FILE:
               Digest d = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize());
               Path inputPath = ActionInputHelper.toInputPath(input, execRoot);
-              currDir.addChild(FileNode.createExecutable(path.getBaseName(), inputPath, d));
-              return 1;
+              boolean childAdded = currDir.addChild(
+                  FileNode.createExecutable(path.getBaseName(), inputPath, d));
+              return childAdded ? 1 : 0;
 
             case DIRECTORY:
               SortedMap<PathFragment, ActionInput> directoryInputs =
@@ -170,6 +172,7 @@ private static int buildFromActionInputs(
 
           return 0;
         });
+    return ret;
   }
 
   private static <T> int build(