Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remote: Fix file counting in merkletree.DirectoryTreeBuilder #14330

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
}

Expand All @@ -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 =
Expand All @@ -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<PathFragment, ActionInput> directoryInputs =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<PathFragment, ActionInput> sortedInputs = new TreeMap<>();
Map<ActionInput, FileArtifactValue> 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<PathFragment, ActionInput> sortedInputs) {
PathFragment pathFragment = PathFragment.create(path);
Expand Down