Always create TreeArtifactValue through a builder.
Ensures that no unnecessary (unsorted) temporary maps are created prior to the ImmutableSortedMap being created for the final TreeArtifactValue.
MetadataInjector becomes aware of TreeArtifactValue now that the dependency cycle is broken after work in b/161911915, so it can take the TreeArtifactValue instead of a map of its children. The method is renamed to injectTree.
RELNOTES: None.
PiperOrigin-RevId: 323080321
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD
index 932a8bb..db5e4d7 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD
@@ -85,6 +85,7 @@
"//src/main/java/com/google/devtools/build/lib/skyframe:sane_analysis_exception",
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
"//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_aware_action",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi",
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java
index 8bfcd77..19c214d 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java
@@ -33,8 +33,8 @@
* <p>Freshly created output files (i.e. from an action that just executed) that require a stat to
* obtain the metadata will first be set read-only and executable during this call. This ensures
* that the returned metadata has an appropriate ctime, which is affected by chmod. Note that this
- * does not apply to outputs injected via {@link #injectFile} or {@link #injectDirectory} since a
- * stat is not required for them.
+ * does not apply to outputs injected via {@link #injectFile} or {@link #injectTree} since a stat
+ * is not required for them.
*/
@Override
@Nullable
@@ -47,7 +47,7 @@
* Constructs a {@link FileArtifactValue} for the given output whose digest is known.
*
* <p>This call does not inject the returned metadata. It should be injected with a followup call
- * to {@link #injectFile} or {@link #injectDirectory} as appropriate.
+ * to {@link #injectFile} or {@link #injectTree} as appropriate.
*
* <p>chmod will not be called on the output.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataInjector.java b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataInjector.java
index ccea7b3..a425ee8 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataInjector.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataInjector.java
@@ -14,13 +14,11 @@
package com.google.devtools.build.lib.actions.cache;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
-import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.FileArtifactValue;
-import java.util.Map;
+import com.google.devtools.build.lib.skyframe.TreeArtifactInjector;
/** Supports metadata injection of action outputs into skyframe. */
-public interface MetadataInjector {
+public interface MetadataInjector extends TreeArtifactInjector {
/**
* Injects the metadata of a file.
@@ -29,20 +27,10 @@
*
* <p>{@linkplain Artifact#isTreeArtifact Tree artifacts} and their {@linkplain
* Artifact#isChildOfDeclaredDirectory children} must not be passed here. Instead, they should be
- * passed to {@link #injectDirectory}.
+ * passed to {@link #injectTree}.
*
* @param output a regular output file
* @param metadata the file metadata
*/
void injectFile(Artifact output, FileArtifactValue metadata);
-
- /**
- * Injects the metadata of a tree artifact.
- *
- * <p>This can be used to save filesystem operations when the metadata is already known.
- *
- * @param output an output directory {@linkplain Artifact#isTreeArtifact tree artifact}
- * @param children the metadata of the files stored in the directory
- */
- void injectDirectory(SpecialArtifact output, Map<TreeFileArtifact, FileArtifactValue> children);
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD
index 04e0f2e..9cf2c60 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD
@@ -78,6 +78,7 @@
"//src/main/java/com/google/devtools/build/lib/remote/options",
"//src/main/java/com/google/devtools/build/lib/remote/util",
"//src/main/java/com/google/devtools/build/lib/skyframe:mutable_supplier",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
"//src/main/java/com/google/devtools/build/lib/util:exit_code",
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
index afc1562..7c5bdfd 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.remote;
+import static com.google.common.util.concurrent.Futures.immediateFuture;
import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture;
@@ -44,7 +45,6 @@
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.ExecException;
-import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue;
import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.actions.cache.MetadataInjector;
@@ -63,6 +63,7 @@
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution;
import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution.Code;
+import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.util.io.OutErr;
import com.google.devtools.build.lib.vfs.Dirent;
@@ -98,13 +99,8 @@
void lock() throws InterruptedException, IOException;
}
- private static final ListenableFuture<Void> COMPLETED_SUCCESS = SettableFuture.create();
- private static final ListenableFuture<byte[]> EMPTY_BYTES = SettableFuture.create();
-
- static {
- ((SettableFuture<Void>) COMPLETED_SUCCESS).set(null);
- ((SettableFuture<byte[]>) EMPTY_BYTES).set(new byte[0]);
- }
+ private static final ListenableFuture<Void> COMPLETED_SUCCESS = immediateFuture(null);
+ private static final ListenableFuture<byte[]> EMPTY_BYTES = immediateFuture(new byte[0]);
protected final RemoteCacheClient cacheProtocol;
protected final RemoteOptions options;
@@ -440,7 +436,7 @@
}
}
- /** Download a file (that is not a directory). The content is fetched from the digest. */
+ /** Downloads a file (that is not a directory). The content is fetched from the digest. */
public ListenableFuture<Void> downloadFile(Path path, Digest digest) throws IOException {
Preconditions.checkNotNull(path.getParentDirectory()).createDirectoryAndParents();
if (digest.getSizeBytes() == 0) {
@@ -621,8 +617,7 @@
+ "--experimental_remote_download_outputs=minimal");
}
SpecialArtifact parent = (SpecialArtifact) output;
- ImmutableMap.Builder<TreeFileArtifact, FileArtifactValue> childMetadata =
- ImmutableMap.builderWithExpectedSize(directory.files.size());
+ TreeArtifactValue.Builder tree = TreeArtifactValue.newBuilder(parent);
for (FileMetadata file : directory.files()) {
TreeFileArtifact child =
TreeFileArtifact.createTreeOutput(parent, file.path().relativeTo(parent.getPath()));
@@ -632,9 +627,9 @@
file.digest().getSizeBytes(),
/*locationIndex=*/ 1,
actionId);
- childMetadata.put(child, value);
+ tree.putChild(child, value);
}
- metadataInjector.injectDirectory(parent, childMetadata.build());
+ metadataInjector.injectTree(parent, tree.build());
} else {
FileMetadata outputMetadata = metadata.file(execRoot.getRelative(output.getExecPathString()));
if (outputMetadata == null) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
index 50a9ee2..e021d3bb 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionValue.java
@@ -271,16 +271,15 @@
}
SpecialArtifact newParent = (SpecialArtifact) newArtifact;
+ TreeArtifactValue.Builder newTree = TreeArtifactValue.newBuilder(newParent);
- Map<TreeFileArtifact, FileArtifactValue> newChildren =
- Maps.newHashMapWithExpectedSize(tree.getChildValues().size());
for (Map.Entry<TreeFileArtifact, FileArtifactValue> child : tree.getChildValues().entrySet()) {
- newChildren.put(
+ newTree.putChild(
TreeFileArtifact.createTreeOutput(newParent, child.getKey().getParentRelativePath()),
child.getValue());
}
- return TreeArtifactValue.create(newChildren);
+ return newTree.build();
}
ActionExecutionValue transformForSharedAction(ImmutableSet<Artifact> outputs) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
index ef44f38..3112097 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -20,7 +20,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.Sets;
import com.google.common.flogger.GoogleLogger;
import com.google.common.io.BaseEncoding;
@@ -322,8 +321,7 @@
setPathReadOnlyAndExecutable(treeDir);
}
- ImmutableSortedMap.Builder<TreeFileArtifact, FileArtifactValue> values =
- ImmutableSortedMap.naturalOrder();
+ TreeArtifactValue.Builder tree = TreeArtifactValue.newBuilder(parent);
TreeArtifactValue.visitTree(
treeDir,
@@ -347,10 +345,10 @@
throw new IOException(errorMessage, e);
}
- values.put(child, metadata);
+ tree.putChild(child, metadata);
});
- return TreeArtifactValue.create(values.build());
+ return tree.build();
}
@Override
@@ -379,7 +377,7 @@
checkArgument(isKnownOutput(output), "%s is not a declared output of this action", output);
checkArgument(
!output.isTreeArtifact() && !output.isChildOfDeclaredDirectory(),
- "Tree artifacts and their children must be injected via injectDirectory: %s",
+ "Tree artifacts and their children must be injected via injectTree: %s",
output);
checkState(executionMode.get(), "Tried to inject %s outside of execution", output);
@@ -387,13 +385,12 @@
}
@Override
- public void injectDirectory(
- SpecialArtifact output, Map<TreeFileArtifact, FileArtifactValue> children) {
+ public void injectTree(SpecialArtifact output, TreeArtifactValue tree) {
checkArgument(isKnownOutput(output), "%s is not a declared output of this action", output);
checkArgument(output.isTreeArtifact(), "Output must be a tree artifact: %s", output);
checkState(executionMode.get(), "Tried to inject %s outside of execution", output);
- store.putTreeArtifactData(output, TreeArtifactValue.create(children));
+ store.putTreeArtifactData(output, tree);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
index d4696b9..98a749f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
@@ -16,8 +16,6 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionExecutionException;
@@ -27,6 +25,7 @@
import com.google.devtools.build.lib.actions.ActionTemplate;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
+import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactOwner;
import com.google.devtools.build.lib.actions.FileArtifactValue;
@@ -189,11 +188,14 @@
return null;
}
- // Aggregate the ArtifactValues for individual TreeFileArtifacts into a TreeArtifactValue for
- // the parent TreeArtifact.
- ImmutableMap.Builder<TreeFileArtifact, FileArtifactValue> map = ImmutableMap.builder();
+ // Aggregate the metadata for individual TreeFileArtifacts into a TreeArtifactValue for the
+ // parent TreeArtifact.
+ SpecialArtifact parent = (SpecialArtifact) artifactDependencies.artifact;
+ TreeArtifactValue.Builder treeBuilder = TreeArtifactValue.newBuilder(parent);
boolean omitted = false;
+
for (ActionLookupData actionKey : expandedActionExecutionKeys) {
+ boolean sawTreeChild = false;
ActionExecutionValue actionExecutionValue =
(ActionExecutionValue)
Preconditions.checkNotNull(
@@ -202,48 +204,47 @@
artifactDependencies,
expansionValue,
expandedActionValueMap);
- Iterable<TreeFileArtifact> treeFileArtifacts =
- Iterables.transform(
- Iterables.filter(
- actionExecutionValue.getAllFileValues().keySet(),
- artifact -> {
- Preconditions.checkState(
- artifact.hasParent(),
- "No parent: %s %s %s",
- artifact,
- actionExecutionValue,
- artifactDependencies);
- return artifact.getParent().equals(artifactDependencies.artifact);
- }),
- artifact -> (TreeFileArtifact) artifact);
- Preconditions.checkState(
- !Iterables.isEmpty(treeFileArtifacts),
- "Action denoted by %s does not output TreeFileArtifact from %s",
- actionKey,
- artifactDependencies);
+ for (Map.Entry<Artifact, FileArtifactValue> entry :
+ actionExecutionValue.getAllFileValues().entrySet()) {
+ Artifact artifact = entry.getKey();
+ Preconditions.checkState(
+ artifact.hasParent(),
+ "Parentless artifact %s found in ActionExecutionValue for %s: %s %s",
+ artifact,
+ actionKey,
+ actionExecutionValue,
+ artifactDependencies);
- for (TreeFileArtifact treeFileArtifact : treeFileArtifacts) {
- FileArtifactValue value =
- actionExecutionValue.getExistingFileArtifactValue(treeFileArtifact);
- if (FileArtifactValue.OMITTED_FILE_MARKER.equals(value)) {
- omitted = true;
- } else {
- map.put(treeFileArtifact, value);
+ if (artifact.getParent().equals(parent)) {
+ sawTreeChild = true;
+ if (FileArtifactValue.OMITTED_FILE_MARKER.equals(entry.getValue())) {
+ omitted = true;
+ } else {
+ treeBuilder.putChild((TreeFileArtifact) artifact, entry.getValue());
+ }
}
}
+
+ Preconditions.checkState(
+ sawTreeChild,
+ "Action denoted by %s does not output any TreeFileArtifacts from %s",
+ actionKey,
+ artifactDependencies);
}
- ImmutableMap<TreeFileArtifact, FileArtifactValue> children = map.build();
+ TreeArtifactValue tree = treeBuilder.build();
if (omitted) {
Preconditions.checkState(
- children.isEmpty(),
+ tree.getChildValues().isEmpty(),
"Action template expansion has some but not all outputs omitted, present outputs: %s",
- children);
+ artifactDependencies,
+ tree.getChildValues());
return TreeArtifactValue.OMITTED_TREE_MARKER;
}
- return TreeArtifactValue.create(children);
+
+ return tree;
}
private static SkyValue createSourceValue(Artifact artifact, Environment env)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index 09603fa..b3473b4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -2674,11 +2674,14 @@
java_library(
name = "tree_artifact_value",
- srcs = ["TreeArtifactValue.java"],
+ srcs = [
+ "TreeArtifactInjector.java",
+ "TreeArtifactValue.java",
+ ],
deps = [
- "//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
+ "//src/main/java/com/google/devtools/build/lib/actions:has_digest",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
"//src/main/java/com/google/devtools/build/lib/util",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactInjector.java b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactInjector.java
new file mode 100644
index 0000000..22a7df5
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactInjector.java
@@ -0,0 +1,31 @@
+// Copyright 2020 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
+
+/** Supports metadata injection of action outputs into skyframe. */
+public interface TreeArtifactInjector {
+
+ /**
+ * Injects the metadata of a tree artifact.
+ *
+ * <p>This can be used to save filesystem operations when the metadata is already known.
+ *
+ * @param output an output directory {@linkplain Artifact#isTreeArtifact tree artifact}
+ * @param tree a {@link TreeArtifactValue} with the metadata of the files stored in the directory
+ */
+ void injectTree(SpecialArtifact output, TreeArtifactValue tree);
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
index 1f570d4..6727ecf 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
@@ -13,10 +13,11 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import com.google.common.base.MoreObjects;
-import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedMap;
@@ -25,7 +26,6 @@
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.HasDigest;
import com.google.devtools.build.lib.actions.cache.DigestUtils;
-import com.google.devtools.build.lib.actions.cache.MetadataInjector;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
import com.google.devtools.build.lib.util.Fingerprint;
@@ -49,19 +49,36 @@
*/
public class TreeArtifactValue implements HasDigest, SkyValue {
+ /** Returns an empty {@link TreeArtifactValue}. */
+ public static TreeArtifactValue empty() {
+ return EMPTY;
+ }
+
+ /**
+ * Returns a new {@link Builder} for the given parent tree artifact.
+ *
+ * <p>The returned builder only supports adding children under this parent. To build multiple tree
+ * artifacts at once, use {@link MultiBuilder}.
+ */
+ public static Builder newBuilder(SpecialArtifact parent) {
+ return new Builder(parent);
+ }
+
/** Builder for constructing multiple instances of {@link TreeArtifactValue} at once. */
public interface MultiBuilder {
/**
* Puts a child tree file into this builder under its {@linkplain TreeFileArtifact#getParent
* parent}.
+ *
+ * @return {@code this} for convenience
*/
- void putChild(TreeFileArtifact child, FileArtifactValue metadata);
+ MultiBuilder putChild(TreeFileArtifact child, FileArtifactValue metadata);
/**
* For each unique parent seen by this builder, passes the aggregated metadata to {@link
- * MetadataInjector#injectDirectory}.
+ * TreeArtifactInjector#injectTree}.
*/
- void injectTo(MetadataInjector metadataInjector);
+ void injectTo(TreeArtifactInjector treeInjector);
}
/** Returns a new {@link MultiBuilder}. */
@@ -85,8 +102,7 @@
private final ImmutableSortedMap<TreeFileArtifact, FileArtifactValue> childData;
private final boolean entirelyRemote;
- @AutoCodec.VisibleForSerialization
- TreeArtifactValue(
+ private TreeArtifactValue(
byte[] digest,
ImmutableSortedMap<TreeFileArtifact, FileArtifactValue> childData,
boolean entirelyRemote) {
@@ -95,37 +111,6 @@
this.entirelyRemote = entirelyRemote;
}
- /**
- * Returns a TreeArtifactValue out of the given Artifact-relative path fragments and their
- * corresponding FileArtifactValues.
- */
- static TreeArtifactValue create(Map<TreeFileArtifact, FileArtifactValue> childData) {
- if (childData.isEmpty()) {
- return EMPTY;
- }
-
- // Sort the children to ensure a deterministic TreeArtifactValue (including digest).
- ImmutableSortedMap<TreeFileArtifact, FileArtifactValue> sortedChildData =
- ImmutableSortedMap.copyOf(childData);
- Fingerprint fingerprint = new Fingerprint();
- boolean entirelyRemote = true;
-
- for (Map.Entry<TreeFileArtifact, FileArtifactValue> e : sortedChildData.entrySet()) {
- TreeFileArtifact child = e.getKey();
- FileArtifactValue value = e.getValue();
- Preconditions.checkState(
- !FileArtifactValue.OMITTED_FILE_MARKER.equals(value),
- "Cannot construct TreeArtifactValue because child %s was omitted",
- child);
- // Tolerate a tree artifact having a mix of local and remote children (b/152496153#comment80).
- entirelyRemote &= value.isRemote();
- fingerprint.addPath(child.getParentRelativePath());
- value.addTo(fingerprint);
- }
-
- return new TreeArtifactValue(fingerprint.digestAndReset(), sortedChildData, entirelyRemote);
- }
-
FileArtifactValue getMetadata() {
return FileArtifactValue.createProxy(digest);
}
@@ -136,7 +121,6 @@
.collect(toImmutableSet());
}
- @Nullable
@Override
public byte[] getDigest() {
return digest.clone();
@@ -180,7 +164,7 @@
@Override
public String toString() {
- return MoreObjects.toStringHelper(TreeArtifactValue.class)
+ return MoreObjects.toStringHelper(this)
.add("digest", digest)
.add("childData", childData)
.toString();
@@ -277,7 +261,7 @@
* artifact directory, or if {@link TreeArtifactVisitor#visit} throws {@link IOException}
*/
public static void visitTree(Path parentDir, TreeArtifactVisitor visitor) throws IOException {
- visitTree(parentDir, PathFragment.EMPTY_FRAGMENT, Preconditions.checkNotNull(visitor));
+ visitTree(parentDir, PathFragment.EMPTY_FRAGMENT, checkNotNull(visitor));
}
private static void visitTree(Path parentDir, PathFragment subdir, TreeArtifactVisitor visitor)
@@ -330,20 +314,82 @@
}
}
+ /** Builder for a {@link TreeArtifactValue}. */
+ public static final class Builder {
+ private final ImmutableSortedMap.Builder<TreeFileArtifact, FileArtifactValue> childData =
+ ImmutableSortedMap.naturalOrder();
+ private final SpecialArtifact parent;
+
+ Builder(SpecialArtifact parent) {
+ checkArgument(parent.isTreeArtifact(), "%s is not a tree artifact", parent);
+ this.parent = parent;
+ }
+
+ /**
+ * Adds a child to this builder.
+ *
+ * <p>The child's {@linkplain TreeFileArtifact#getParent parent} <em>must</em> match the parent
+ * with which this builder was initialized.
+ *
+ * <p>Children may be added in any order. The children are sorted prior to constructing the
+ * final {@link TreeArtifactValue}.
+ *
+ * <p>It is illegal to call this method with {@link FileArtifactValue.OMITTED_FILE_MARKER}. When
+ * children are omitted, use {@link TreeArtifactValue#OMITTED_TREE_MARKER}.
+ *
+ * @return {@code this} for convenience
+ */
+ public Builder putChild(TreeFileArtifact child, FileArtifactValue metadata) {
+ checkArgument(
+ child.getParent().equals(parent),
+ "While building TreeArtifactValue for %s, got %s with parent %s",
+ parent,
+ child,
+ child.getParent());
+ checkArgument(
+ !FileArtifactValue.OMITTED_FILE_MARKER.equals(metadata),
+ "Cannot construct TreeArtifactValue for %s because child %s was omitted",
+ parent,
+ child);
+ childData.put(child, metadata);
+ return this;
+ }
+
+ /** Builds the final {@link TreeArtifactValue}. */
+ public TreeArtifactValue build() {
+ ImmutableSortedMap<TreeFileArtifact, FileArtifactValue> finalChildData = childData.build();
+ if (finalChildData.isEmpty()) {
+ return EMPTY;
+ }
+
+ Fingerprint fingerprint = new Fingerprint();
+ boolean entirelyRemote = true;
+
+ for (Map.Entry<TreeFileArtifact, FileArtifactValue> childData : finalChildData.entrySet()) {
+ // Digest will be deterministic because children are sorted.
+ fingerprint.addPath(childData.getKey().getParentRelativePath());
+ childData.getValue().addTo(fingerprint);
+
+ // Tolerate a mix of local and remote children(b/152496153#comment80).
+ entirelyRemote &= childData.getValue().isRemote();
+ }
+
+ return new TreeArtifactValue(fingerprint.digestAndReset(), finalChildData, entirelyRemote);
+ }
+ }
+
private static final class BasicMultiBuilder implements MultiBuilder {
- private final Map<
- SpecialArtifact, ImmutableSortedMap.Builder<TreeFileArtifact, FileArtifactValue>>
- map = new HashMap<>();
+ private final Map<SpecialArtifact, Builder> map = new HashMap<>();
@Override
- public void putChild(TreeFileArtifact child, FileArtifactValue metadata) {
- map.computeIfAbsent(child.getParent(), parent -> ImmutableSortedMap.naturalOrder())
- .put(child, metadata);
+ public MultiBuilder putChild(TreeFileArtifact child, FileArtifactValue metadata) {
+ map.computeIfAbsent(child.getParent(), Builder::new).putChild(child, metadata);
+ return this;
}
@Override
- public void injectTo(MetadataInjector metadataInjector) {
- map.forEach((parent, builder) -> metadataInjector.injectDirectory(parent, builder.build()));
+ public void injectTo(TreeArtifactInjector treeInjector) {
+ map.forEach((parent, builder) -> treeInjector.injectTree(parent, builder.build()));
}
}
@@ -353,14 +399,20 @@
map = new ConcurrentHashMap<>();
@Override
- public void putChild(TreeFileArtifact child, FileArtifactValue metadata) {
+ public MultiBuilder putChild(TreeFileArtifact child, FileArtifactValue metadata) {
map.computeIfAbsent(child.getParent(), parent -> new ConcurrentHashMap<>())
.put(child, metadata);
+ return this;
}
@Override
- public void injectTo(MetadataInjector metadataInjector) {
- map.forEach(metadataInjector::injectDirectory);
+ public void injectTo(TreeArtifactInjector treeInjector) {
+ map.forEach(
+ (parent, children) -> {
+ Builder builder = new Builder(parent);
+ children.forEach(builder::putChild);
+ treeInjector.injectTree(parent, builder.build());
+ });
}
}
}
