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 index 48005f1a9a7733..675622d7e3617d 100644 --- 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 @@ -147,8 +147,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 index 1ddcf963520079..a29304b1ba40cd 100644 --- 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; }); } @@ -127,9 +128,11 @@ private static int buildFromActionInputs( if (input instanceof VirtualActionInput) { VirtualActionInput virtualActionInput = (VirtualActionInput) input; Digest d = digestUtil.compute(virtualActionInput); - currDir.addChild( - FileNode.createExecutable(path.getBaseName(), virtualActionInput.getBytes(), d)); - return 1; + boolean childAdded = + currDir.addChild( + FileNode.createExecutable( + path.getBaseName(), virtualActionInput.getBytes(), d)); + return childAdded ? 1 : 0; } FileArtifactValue metadata = @@ -141,8 +144,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 directoryInputs = diff --git a/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java b/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java index fcca592bb49bc1..393d610d715574 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/merkletree/ActionInputDirectoryTreeTest.java @@ -130,6 +130,43 @@ public void directoryInputShouldBeExpanded() throws Exception { assertThat(fileNodesAtDepth(tree, 3)).containsExactly(expectedBuzzNode); } + @Test + public void filesShouldBeDeduplicated() throws Exception { + // Test that a file specified as part of a directory and as an individual file is not counted + // twice. + + SortedMap sortedInputs = new TreeMap<>(); + Map metadata = new HashMap<>(); + + Path dirPath = execRoot.getRelative("srcs"); + dirPath.createDirectoryAndParents(); + Artifact dir = ActionsTestUtil.createArtifact(artifactRoot, dirPath); + sortedInputs.put(dirPath.relativeTo(execRoot), dir); + metadata.put(dir, FileArtifactValue.createForTesting(dirPath)); + + Path fooPath = dirPath.getRelative("foo.cc"); + FileSystemUtils.writeContentAsLatin1(fooPath, "foo"); + ActionInput foo = ActionInputHelper.fromPath(fooPath.relativeTo(execRoot)); + sortedInputs.put(fooPath.relativeTo(execRoot), foo); + metadata.put(foo, FileArtifactValue.createForTesting(fooPath)); + + DirectoryTree tree = + DirectoryTreeBuilder.fromActionInputs( + sortedInputs, new StaticMetadataProvider(metadata), execRoot, digestUtil); + assertLexicographicalOrder(tree); + + assertThat(directoriesAtDepth(0, tree)).containsExactly("srcs"); + assertThat(directoriesAtDepth(1, tree)).isEmpty(); + + FileNode expectedFooNode = + FileNode.createExecutable( + "foo.cc", execRoot.getRelative(foo.getExecPath()), digestUtil.computeAsUtf8("foo")); + assertThat(fileNodesAtDepth(tree, 0)).isEmpty(); + assertThat(fileNodesAtDepth(tree, 1)).containsExactly(expectedFooNode); + + assertThat(tree.numFiles()).isEqualTo(1); + } + private static VirtualActionInput addVirtualFile( String path, String content, SortedMap sortedInputs) { PathFragment pathFragment = PathFragment.create(path);