diff options
Diffstat (limited to 'src/main/java/com')
5 files changed, 169 insertions, 68 deletions
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 e183366c11..8cefd9c0a7 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 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver 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 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver 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 0000000000..072c019be4 --- /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 5d1fa1c7a9..57a9d47206 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.SkyFunctionException; 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 @@ import javax.annotation.Nullable; 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 final class CompletionFunction<TValue extends SkyValue, TResult extends S } } - 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 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S 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 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S 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 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S 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 4a9c7bfc0b..36ad359ba8 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.ActionAnalysisMetadata; 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.ActionLookupValue; 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.Profiler; 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 @@ import javax.annotation.Nullable; /** * 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 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { 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 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { } } + 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 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { 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( diff --git a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java index 9afa82fb53..03503bfed1 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Action; 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.BuildFailedException; import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.ExecException; @@ -116,7 +117,7 @@ public interface OutputService { * com.google.devtools.build.lib.pkgcache.PathPackageLocator}) * @param inputArtifactData information about required inputs to the action * @param outputArtifacts required outputs of the action - * @return an action-scoped filesystem if {@link supportsActionFileSystem} is true + * @return an action-scoped filesystem if {@link #supportsActionFileSystem} is true */ @Nullable default FileSystem createActionFileSystem( @@ -130,12 +131,24 @@ public interface OutputService { } /** - * Updates the context used by the filesystem returned by {@link createActionFileSystem}. + * Updates the context used by the filesystem returned by {@link #createActionFileSystem}. * * <p>Should be called as context changes throughout action execution. * - * @param actionFileSystem must be a filesystem returned by {@link createActionFileSystem}. + * @param actionFileSystem must be a filesystem returned by {@link #createActionFileSystem}. */ default void updateActionFileSystemContext( FileSystem actionFileSystem, SkyFunction.Environment env, MetadataConsumer consumer) {} + + default boolean supportsPathResolverForArtifactValues() { + return false; + } + + default ArtifactPathResolver createPathResolverForArtifactValues( + PathFragment execRoot, + FileSystem fileSystem, + ImmutableList<Root> pathEntries, + ActionInputMap actionInputMap) { + throw new IllegalStateException("Path resolver not supported by this class"); + } } |