Add support to completion function to create a path resolver from its looked-up artifact values.
RELNOTES: None
PiperOrigin-RevId: 207295716
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index e183366..8cefd9c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -659,42 +659,7 @@
try {
SkyValue value = depsEntry.getValue().get();
if (populateInputData) {
- if (value instanceof AggregatingArtifactValue) {
- AggregatingArtifactValue aggregatingValue = (AggregatingArtifactValue) value;
- for (Pair<Artifact, FileArtifactValue> entry : aggregatingValue.getFileArtifacts()) {
- inputArtifactData.put(entry.first, entry.second);
- }
- for (Pair<Artifact, TreeArtifactValue> entry : aggregatingValue.getTreeArtifacts()) {
- expandTreeArtifactAndPopulateArtifactData(
- entry.getFirst(),
- Preconditions.checkNotNull(entry.getSecond()),
- expandedArtifacts,
- inputArtifactData);
- }
- // We have to cache the "digest" of the aggregating value itself,
- // because the action cache checker may want it.
- inputArtifactData.put(input, aggregatingValue.getSelfData());
- // While not obvious at all this code exists to ensure that we don't expand the
- // .runfiles/MANIFEST file into the inputs. The reason for that being that the MANIFEST
- // file contains absolute paths that don't work with remote execution.
- // Instead, the way the SpawnInputExpander expands runfiles is via the Runfiles class
- // which contains all artifacts in the runfiles tree minus the MANIFEST file.
- // TODO(buchgr): Clean this up and get rid of the RunfilesArtifactValue type.
- if (!(value instanceof RunfilesArtifactValue)) {
- ImmutableList.Builder<Artifact> expansionBuilder = ImmutableList.builder();
- for (Pair<Artifact, FileArtifactValue> pair : aggregatingValue.getFileArtifacts()) {
- expansionBuilder.add(Preconditions.checkNotNull(pair.getFirst()));
- }
- expandedArtifacts.put(input, expansionBuilder.build());
- }
- } else if (value instanceof TreeArtifactValue) {
- expandTreeArtifactAndPopulateArtifactData(
- input, (TreeArtifactValue) value, expandedArtifacts, inputArtifactData);
-
- } else {
- Preconditions.checkState(value instanceof FileArtifactValue, depsEntry);
- inputArtifactData.put(input, (FileArtifactValue) value);
- }
+ ActionInputMapHelper.addToMap(inputArtifactData, expandedArtifacts, input, value);
}
} catch (MissingInputFileException e) {
missingCount++;
@@ -746,22 +711,6 @@
return Pair.of(inputArtifactData, expandedArtifacts);
}
- private static void expandTreeArtifactAndPopulateArtifactData(
- Artifact treeArtifact,
- TreeArtifactValue value,
- Map<Artifact, Collection<Artifact>> expandedArtifacts,
- ActionInputMap inputArtifactData) {
- ImmutableSet.Builder<Artifact> children = ImmutableSet.builder();
- for (Map.Entry<Artifact.TreeFileArtifact, FileArtifactValue> child :
- value.getChildValues().entrySet()) {
- children.add(child.getKey());
- inputArtifactData.put(child.getKey(), child.getValue());
- }
- expandedArtifacts.put(treeArtifact, children.build());
- // Again, we cache the "digest" of the value for cache checking.
- inputArtifactData.put(treeArtifact, value.getSelfData());
- }
-
private static Iterable<Artifact> filterKnownInputs(
Iterable<Artifact> newInputs, ActionInputMap inputArtifactData) {
return Iterables.filter(newInputs, input -> inputArtifactData.getMetadata(input) == null);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java
new file mode 100644
index 0000000..072c019
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java
@@ -0,0 +1,88 @@
+// Copyright 2018 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.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.actions.ActionInputMap;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.FileArtifactValue;
+import com.google.devtools.build.lib.util.Pair;
+import com.google.devtools.build.skyframe.SkyValue;
+import java.util.Collection;
+import java.util.Map;
+
+class ActionInputMapHelper {
+
+ // Adds a value obtained by an Artifact skyvalue lookup to the action input map
+ static void addToMap(
+ ActionInputMap inputMap,
+ Map<Artifact, Collection<Artifact>> expandedArtifacts,
+ Artifact key,
+ SkyValue value) {
+ if (value instanceof AggregatingArtifactValue) {
+ AggregatingArtifactValue aggregatingValue = (AggregatingArtifactValue) value;
+ for (Pair<Artifact, FileArtifactValue> entry : aggregatingValue.getFileArtifacts()) {
+ inputMap.put(entry.first, entry.second);
+ }
+ for (Pair<Artifact, TreeArtifactValue> entry : aggregatingValue.getTreeArtifacts()) {
+ expandTreeArtifactAndPopulateArtifactData(
+ entry.getFirst(),
+ Preconditions.checkNotNull(entry.getSecond()),
+ expandedArtifacts,
+ inputMap);
+ }
+ // We have to cache the "digest" of the aggregating value itself,
+ // because the action cache checker may want it.
+ inputMap.put(key, aggregatingValue.getSelfData());
+ // While not obvious at all this code exists to ensure that we don't expand the
+ // .runfiles/MANIFEST file into the inputs. The reason for that being that the MANIFEST
+ // file contains absolute paths that don't work with remote execution.
+ // Instead, the way the SpawnInputExpander expands runfiles is via the Runfiles class
+ // which contains all artifacts in the runfiles tree minus the MANIFEST file.
+ // TODO(buchgr): Clean this up and get rid of the RunfilesArtifactValue type.
+ if (!(value instanceof RunfilesArtifactValue)) {
+ ImmutableList.Builder<Artifact> expansionBuilder = ImmutableList.builder();
+ for (Pair<Artifact, FileArtifactValue> pair : aggregatingValue.getFileArtifacts()) {
+ expansionBuilder.add(Preconditions.checkNotNull(pair.getFirst()));
+ }
+ expandedArtifacts.put(key, expansionBuilder.build());
+ }
+ } else if (value instanceof TreeArtifactValue) {
+ expandTreeArtifactAndPopulateArtifactData(
+ key, (TreeArtifactValue) value, expandedArtifacts, inputMap);
+
+ } else {
+ Preconditions.checkState(value instanceof FileArtifactValue);
+ inputMap.put(key, (FileArtifactValue) value);
+ }
+ }
+
+ private static void expandTreeArtifactAndPopulateArtifactData(
+ Artifact treeArtifact,
+ TreeArtifactValue value,
+ Map<Artifact, Collection<Artifact>> expandedArtifacts,
+ ActionInputMap inputMap) {
+ ImmutableSet.Builder<Artifact> children = ImmutableSet.builder();
+ for (Map.Entry<Artifact.TreeFileArtifact, FileArtifactValue> child :
+ value.getChildValues().entrySet()) {
+ children.add(child.getKey());
+ inputMap.put(child.getKey(), child.getValue());
+ }
+ expandedArtifacts.put(treeArtifact, children.build());
+ // Again, we cache the "digest" of the value for cache checking.
+ inputMap.put(treeArtifact, value.getSelfData());
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
index 5d1fa1c..57a9d47 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.skyframe;
import com.google.devtools.build.lib.actions.ActionExecutionException;
+import com.google.devtools.build.lib.actions.ActionInputMap;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.ArtifactSkyKey;
@@ -39,6 +40,8 @@
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.ValueOrException2;
+import java.util.Collection;
+import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;
@@ -48,6 +51,12 @@
public final class CompletionFunction<TValue extends SkyValue, TResult extends SkyValue>
implements SkyFunction {
+ interface PathResolverFactory {
+ ArtifactPathResolver createPathResolverForArtifactValues(ActionInputMap actionInputMap);
+
+ boolean shouldCreatePathResolverForArtifactValues();
+ }
+
/** A strategy for completing the build. */
interface Completor<TValue, TResult extends SkyValue> {
@@ -281,17 +290,20 @@
}
}
- public static SkyFunction targetCompletionFunction() {
- return new CompletionFunction<>(new TargetCompletor());
+ public static SkyFunction targetCompletionFunction(PathResolverFactory pathResolverFactory) {
+ return new CompletionFunction<>(pathResolverFactory, new TargetCompletor());
}
- public static SkyFunction aspectCompletionFunction() {
- return new CompletionFunction<>(new AspectCompletor());
+ public static SkyFunction aspectCompletionFunction(PathResolverFactory pathResolverFactory) {
+ return new CompletionFunction<>(pathResolverFactory, new AspectCompletor());
}
+ private final PathResolverFactory pathResolverFactory;
private final Completor<TValue, TResult> completor;
- private CompletionFunction(Completor<TValue, TResult> completor) {
+ private CompletionFunction(
+ PathResolverFactory pathResolverFactory, Completor<TValue, TResult> completor) {
+ this.pathResolverFactory = pathResolverFactory;
this.completor = completor;
}
@@ -311,6 +323,14 @@
MissingInputFileException.class,
ActionExecutionException.class);
+ boolean createPathResolver = pathResolverFactory.shouldCreatePathResolverForArtifactValues();
+ ActionInputMap inputMap = null;
+ Map<Artifact, Collection<Artifact>> expandedArtifacts = null;
+ if (createPathResolver) {
+ inputMap = new ActionInputMap(inputDeps.size());
+ expandedArtifacts = new HashMap<>();
+ }
+
int missingCount = 0;
ActionExecutionException firstActionExecutionException = null;
MissingInputFileException missingInputException = null;
@@ -319,7 +339,10 @@
depsEntry : inputDeps.entrySet()) {
Artifact input = ArtifactSkyKey.artifact(depsEntry.getKey());
try {
- depsEntry.getValue().get();
+ SkyValue artifactValue = depsEntry.getValue().get();
+ if (createPathResolver && artifactValue != null) {
+ ActionInputMapHelper.addToMap(inputMap, expandedArtifacts, input, artifactValue);
+ }
} catch (MissingInputFileException e) {
missingCount++;
final Label inputOwner = input.getOwner();
@@ -365,9 +388,14 @@
if (env.valuesMissing()) {
return null;
}
+
+ ArtifactPathResolver pathResolver =
+ createPathResolver
+ ? pathResolverFactory.createPathResolverForArtifactValues(inputMap)
+ : ArtifactPathResolver.IDENTITY;
+
ExtendedEventHandler.Postable postable =
- completor.createSucceeded(
- skyKey, value, ArtifactPathResolver.IDENTITY, topLevelContext, env);
+ completor.createSucceeded(skyKey, value, pathResolver, topLevelContext, env);
if (postable == null) {
return null;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 4a9c7bf..36ad359 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -39,6 +39,7 @@
import com.google.devtools.build.lib.actions.ActionCacheChecker;
import com.google.devtools.build.lib.actions.ActionExecutionStatusReporter;
import com.google.devtools.build.lib.actions.ActionGraph;
+import com.google.devtools.build.lib.actions.ActionInputMap;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLogBufferPathGenerator;
@@ -46,6 +47,7 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactFactory;
import com.google.devtools.build.lib.actions.ArtifactOwner;
+import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.ArtifactResolver.ArtifactResolverSupplier;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
@@ -116,6 +118,7 @@
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.rules.repository.ResolvedHashesFunction;
import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey;
+import com.google.devtools.build.lib.skyframe.CompletionFunction.PathResolverFactory;
import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.FileDirtinessChecker;
import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile;
@@ -183,9 +186,9 @@
/**
* A helper object to support Skyframe-driven execution.
*
- * <p>This object is mostly used to inject external state, such as the executor engine or
- * some additional artifacts (workspace status and build info artifacts) into SkyFunctions
- * for use during the build.
+ * <p>This object is mostly used to inject external state, such as the executor engine or some
+ * additional artifacts (workspace status and build info artifacts) into SkyFunctions for use during
+ * the build.
*/
public abstract class SkyframeExecutor implements WalkableGraphFactory {
private final EvaluatorSupplier evaluatorSupplier;
@@ -304,6 +307,8 @@
private static final Logger logger = Logger.getLogger(SkyframeExecutor.class.getName());
+ private final PathResolverFactory pathResolverFactory = new PathResolverFactoryImpl();
+
/** An {@link ArtifactResolverSupplier} that supports setting of an {@link ArtifactFactory}. */
public static class MutableArtifactFactorySupplier implements ArtifactResolverSupplier {
@@ -319,6 +324,20 @@
}
}
+ class PathResolverFactoryImpl implements PathResolverFactory {
+ @Override
+ public boolean shouldCreatePathResolverForArtifactValues() {
+ return outputService != null && outputService.supportsPathResolverForArtifactValues();
+ }
+
+ @Override
+ public ArtifactPathResolver createPathResolverForArtifactValues(ActionInputMap actionInputMap) {
+ Preconditions.checkState(shouldCreatePathResolverForArtifactValues());
+ return outputService.createPathResolverForArtifactValues(
+ directories.getExecRoot().asFragment(), fileSystem, getPathEntries(), actionInputMap);
+ }
+ }
+
protected SkyframeExecutor(
EvaluatorSupplier evaluatorSupplier,
PackageFactory pkgFactory,
@@ -491,8 +510,12 @@
SkyFunctions.WORKSPACE_FILE,
new WorkspaceFileFunction(ruleClassProvider, pkgFactory, directories));
map.put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction());
- map.put(SkyFunctions.TARGET_COMPLETION, CompletionFunction.targetCompletionFunction());
- map.put(SkyFunctions.ASPECT_COMPLETION, CompletionFunction.aspectCompletionFunction());
+ map.put(
+ SkyFunctions.TARGET_COMPLETION,
+ CompletionFunction.targetCompletionFunction(pathResolverFactory));
+ map.put(
+ SkyFunctions.ASPECT_COMPLETION,
+ CompletionFunction.aspectCompletionFunction(pathResolverFactory));
map.put(SkyFunctions.TEST_COMPLETION, new TestCompletionFunction());
map.put(Artifact.ARTIFACT, new ArtifactFunction());
map.put(
