diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java')
| -rw-r--r-- | src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java | 80 |
1 files changed, 52 insertions, 28 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 0905cf1bef..f465201da3 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 @@ -28,7 +28,6 @@ import com.google.devtools.build.lib.actions.ActionExecutionException; import com.google.devtools.build.lib.actions.ActionInputMap; import com.google.devtools.build.lib.actions.ActionLookupData; import com.google.devtools.build.lib.actions.ActionLookupValue; -import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey; import com.google.devtools.build.lib.actions.AlreadyReportedActionExecutionException; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactPathResolver; @@ -48,7 +47,6 @@ import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.rules.cpp.IncludeScannable; -import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.PathFragment; @@ -157,7 +155,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver env.getValues(state.allInputs.keysRequested); Preconditions.checkState(!env.valuesMissing(), "%s %s", action, state); } - Pair<ActionInputMap, Map<Artifact, Collection<Artifact>>> checkedInputs = null; + CheckInputResults checkedInputs = null; try { // Declare deps on known inputs to action. We do this unconditionally to maintain our // invariant of asking for the same deps each build. @@ -199,13 +197,14 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver if (checkedInputs != null) { Preconditions.checkState(!state.hasArtifactData(), "%s %s", state, action); - state.inputArtifactData = checkedInputs.first; - state.expandedArtifacts = checkedInputs.second; + state.inputArtifactData = checkedInputs.actionInputMap; + state.expandedArtifacts = checkedInputs.expandedArtifacts; + state.expandedFilesets = checkedInputs.expandedFilesets; if (skyframeActionExecutor.usesActionFileSystem()) { state.actionFileSystem = skyframeActionExecutor.createActionFileSystem( directories.getRelativeOutputPath(), - checkedInputs.first, + checkedInputs.actionInputMap, action.getOutputs()); } } @@ -462,33 +461,32 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver metadataHandler.discardOutputMetadata(); } - ImmutableMap.Builder<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings = - ImmutableMap.builder(); + // Make sure this is a regular HashMap rather than ImmutableMapBuilder so that we are safe + // in case of collisions. + Map<PathFragment, ImmutableList<FilesetOutputSymlink>> filesetMappings = new HashMap<>(); for (Artifact actionInput : action.getInputs()) { if (!actionInput.isFileset()) { continue; } - ActionLookupKey filesetActionLookupKey = (ActionLookupKey) actionInput.getArtifactOwner(); - // Index 0 for the Fileset ConfiguredTarget indicates the SkyframeFilesetManifestAction where - // we compute the fileset's outputSymlinks. - SkyKey filesetActionKey = ActionExecutionValue.key(filesetActionLookupKey, 0); - ActionExecutionValue filesetValue = (ActionExecutionValue) env.getValue(filesetActionKey); - if (filesetValue == null) { - // At this point skyframe does not guarantee that the filesetValue will be ready, since - // the current action does not directly depend on the outputs of the - // SkyframeFilesetManifestAction whose ActionExecutionValue (filesetValue) is needed here. - // TODO(kush): Get rid of this hack by making the outputSymlinks available in the Fileset - // artifact, which this action depends on, so its value will be guaranteed to be present. + ImmutableList<FilesetOutputSymlink> mapping = + ActionInputMapHelper.getFilesets(env, actionInput); + if (mapping == null) { return null; } - filesetMappings.put(actionInput.getExecPath(), filesetValue.getOutputSymlinks()); + filesetMappings.put(actionInput.getExecPath(), mapping); } - ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> filesets = - filesetMappings.build(); + ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> topLevelFilesets = + ImmutableMap.copyOf(filesetMappings); + + // Aggregate top-level Filesets with Filesets nested in Runfiles. Both should be used to update + // the FileSystem context. + state.expandedFilesets + .forEach((artifact, links) -> filesetMappings.put(artifact.getExecPath(), links)); try { - state.updateFileSystemContext(skyframeActionExecutor, env, metadataHandler, filesets); + state.updateFileSystemContext(skyframeActionExecutor, env, metadataHandler, + ImmutableMap.copyOf(filesetMappings)); } catch (IOException e) { throw new ActionExecutionException( "Failed to update filesystem context: ", e, action, /*catastrophe=*/ false); @@ -498,7 +496,8 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver perActionFileCache, metadataHandler, Collections.unmodifiableMap(state.expandedArtifacts), - filesets, + Collections.unmodifiableMap(state.expandedFilesets), + topLevelFilesets, state.actionFileSystem, skyframeDepsResult)) { if (!state.hasExecutedAction()) { @@ -649,15 +648,32 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver } } + private static class CheckInputResults { + /** Metadata about Artifacts consumed by this Action. */ + private final ActionInputMap actionInputMap; + /** Artifact expansion mapping for Runfiles tree and tree artifacts. */ + private final Map<Artifact, Collection<Artifact>> expandedArtifacts; + /** Artifact expansion mapping for Filesets embedded in Runfiles. */ + private final Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets; + + public CheckInputResults(ActionInputMap actionInputMap, + Map<Artifact, Collection<Artifact>> expandedArtifacts, + Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets) { + this.actionInputMap = actionInputMap; + this.expandedArtifacts = expandedArtifacts; + this.expandedFilesets = expandedFilesets; + } + } + /** * Declare dependency on all known inputs of action. Throws exception if any are known to be * missing. Some inputs may not yet be in the graph, in which case the builder should abort. */ - private Pair<ActionInputMap, Map<Artifact, Collection<Artifact>>> checkInputs( + private CheckInputResults checkInputs( Environment env, Action action, Map<SkyKey, ValueOrException2<MissingInputFileException, ActionExecutionException>> inputDeps) - throws ActionExecutionException { + throws ActionExecutionException, InterruptedException { int missingCount = 0; int actionFailures = 0; // Only populate input data if we have the input values, otherwise they'll just go unused. @@ -669,6 +685,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver ActionInputMap inputArtifactData = new ActionInputMap(populateInputData ? inputDeps.size() : 0); Map<Artifact, Collection<Artifact>> expandedArtifacts = new HashMap<>(populateInputData ? 128 : 0); + Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets = new HashMap<>(); ActionExecutionException firstActionExecutionException = null; for (Map.Entry<SkyKey, ValueOrException2<MissingInputFileException, ActionExecutionException>> @@ -677,7 +694,13 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver try { SkyValue value = depsEntry.getValue().get(); if (populateInputData) { - ActionInputMapHelper.addToMap(inputArtifactData, expandedArtifacts, input, value); + ActionInputMapHelper.addToMap( + inputArtifactData, + expandedArtifacts, + expandedFilesets, + input, + value, + env); } } catch (MissingInputFileException e) { missingCount++; @@ -726,7 +749,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver rootCauses.build(), /*catastrophe=*/ false); } - return Pair.of(inputArtifactData, expandedArtifacts); + return new CheckInputResults(inputArtifactData, expandedArtifacts, expandedFilesets); } private static Iterable<Artifact> filterKnownInputs( @@ -782,6 +805,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver ActionInputMap inputArtifactData = null; Map<Artifact, Collection<Artifact>> expandedArtifacts = null; + Map<Artifact, ImmutableList<FilesetOutputSymlink>> expandedFilesets = null; Token token = null; Iterable<Artifact> discoveredInputs = null; ActionExecutionValue value = null; |