From 37bd5f665aa614c6dc640c9d19852dd8d5efb0d8 Mon Sep 17 00:00:00 2001 From: felly Date: Tue, 14 Aug 2018 09:26:34 -0700 Subject: Automated rollback of commit 3bace1b937934fb2cea6260067ecc1cdbe526847. *** Reason for rollback *** b/112583337 RELNOTES: None *** Original change description *** Track Fileset in artifact expansion. PiperOrigin-RevId: 208658921 --- .../lib/skyframe/ActionExecutionFunction.java | 73 ++++++++-------------- 1 file changed, 25 insertions(+), 48 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java') 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 a199feb4f4..0905cf1bef 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,6 +28,7 @@ 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; @@ -47,6 +48,7 @@ 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; @@ -155,7 +157,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver env.getValues(state.allInputs.keysRequested); Preconditions.checkState(!env.valuesMissing(), "%s %s", action, state); } - CheckInputResults checkedInputs = null; + Pair>> 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. @@ -197,14 +199,13 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver if (checkedInputs != null) { Preconditions.checkState(!state.hasArtifactData(), "%s %s", state, action); - state.inputArtifactData = checkedInputs.actionInputMap; - state.expandedArtifacts = checkedInputs.expandedArtifacts; - state.expandedFilesets = checkedInputs.expandedFilesets; + state.inputArtifactData = checkedInputs.first; + state.expandedArtifacts = checkedInputs.second; if (skyframeActionExecutor.usesActionFileSystem()) { state.actionFileSystem = skyframeActionExecutor.createActionFileSystem( directories.getRelativeOutputPath(), - checkedInputs.actionInputMap, + checkedInputs.first, action.getOutputs()); } } @@ -468,24 +469,26 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver continue; } - ImmutableList mapping = - ActionInputMapHelper.getFilesets(env, actionInput); - if (mapping == null) { + 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. return null; } - filesetMappings.put(actionInput.getExecPath(), mapping); + filesetMappings.put(actionInput.getExecPath(), filesetValue.getOutputSymlinks()); } - ImmutableMap> topLevelFilesets = + ImmutableMap> filesets = filesetMappings.build(); - - // 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, - filesetMappings.build()); + state.updateFileSystemContext(skyframeActionExecutor, env, metadataHandler, filesets); } catch (IOException e) { throw new ActionExecutionException( "Failed to update filesystem context: ", e, action, /*catastrophe=*/ false); @@ -495,8 +498,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver perActionFileCache, metadataHandler, Collections.unmodifiableMap(state.expandedArtifacts), - Collections.unmodifiableMap(state.expandedFilesets), - topLevelFilesets, + filesets, state.actionFileSystem, skyframeDepsResult)) { if (!state.hasExecutedAction()) { @@ -647,32 +649,15 @@ 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> expandedArtifacts; - /** Artifact expansion mapping for Filesets embedded in Runfiles. */ - private final Map> expandedFilesets; - - public CheckInputResults(ActionInputMap actionInputMap, - Map> expandedArtifacts, - Map> 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 CheckInputResults checkInputs( + private Pair>> checkInputs( Environment env, Action action, Map> inputDeps) - throws ActionExecutionException, InterruptedException { + throws ActionExecutionException { int missingCount = 0; int actionFailures = 0; // Only populate input data if we have the input values, otherwise they'll just go unused. @@ -684,7 +669,6 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver ActionInputMap inputArtifactData = new ActionInputMap(populateInputData ? inputDeps.size() : 0); Map> expandedArtifacts = new HashMap<>(populateInputData ? 128 : 0); - Map> expandedFilesets = new HashMap<>(); ActionExecutionException firstActionExecutionException = null; for (Map.Entry> @@ -693,13 +677,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver try { SkyValue value = depsEntry.getValue().get(); if (populateInputData) { - ActionInputMapHelper.addToMap( - inputArtifactData, - expandedArtifacts, - expandedFilesets, - input, - value, - env); + ActionInputMapHelper.addToMap(inputArtifactData, expandedArtifacts, input, value); } } catch (MissingInputFileException e) { missingCount++; @@ -748,7 +726,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver rootCauses.build(), /*catastrophe=*/ false); } - return new CheckInputResults(inputArtifactData, expandedArtifacts, expandedFilesets); + return Pair.of(inputArtifactData, expandedArtifacts); } private static Iterable filterKnownInputs( @@ -804,7 +782,6 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver ActionInputMap inputArtifactData = null; Map> expandedArtifacts = null; - Map> expandedFilesets = null; Token token = null; Iterable discoveredInputs = null; ActionExecutionValue value = null; -- cgit v1.2.3