diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java index f0ed9d2189d..515802684d4 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java @@ -653,27 +653,21 @@ private ActionExecutionValue checkCacheAndExecuteIfNeeded( return null; } } - switch (addDiscoveredInputs( - state.inputArtifactData, - state.expandedArtifacts, - filterKnownInputs(state.discoveredInputs, state.inputArtifactData), - env)) { - case VALUES_MISSING: - return null; - case NO_DISCOVERED_DATA: - break; - case DISCOVERED_DATA: - metadataHandler = - new ActionMetadataHandler( - state.inputArtifactData, - /*missingArtifactsAllowed=*/ false, - action.getOutputs(), - tsgm.get(), - pathResolver, - newOutputStore(state)); - // Set the MetadataHandler to accept output information. - metadataHandler.discardOutputMetadata(); + addDiscoveredInputs( + state.inputArtifactData, state.expandedArtifacts, state.discoveredInputs, env); + if (env.valuesMissing()) { + return null; } + metadataHandler = + new ActionMetadataHandler( + state.inputArtifactData, + /* missingArtifactsAllowed= */ false, + action.getOutputs(), + tsgm.get(), + pathResolver, + newOutputStore(state)); + // Set the MetadataHandler to accept output information. + metadataHandler.discardOutputMetadata(); } // Make sure this is a regular HashMap rather than ImmutableMapBuilder so that we are safe @@ -763,25 +757,28 @@ public void run( if (action.discoversInputs()) { Iterable newInputs = filterKnownInputs(action.getInputs(), state.inputArtifactData); + Map metadataFoundDuringActionExecution = + env.getValues(toKeys(newInputs, action.getMandatoryInputs())); state.discoveredInputs = newInputs; - switch (addDiscoveredInputs( - state.inputArtifactData, state.expandedArtifacts, newInputs, env)) { - case VALUES_MISSING: - return; - case NO_DISCOVERED_DATA: - break; - case DISCOVERED_DATA: - // We are in the interesting case of an action that discovered its inputs during - // execution, and found some new ones, but the new ones were already present in the - // graph. We must therefore cache the metadata for those new ones. - metadataHandler = - new ActionMetadataHandler( - state.inputArtifactData, - /*missingArtifactsAllowed=*/ false, - action.getOutputs(), - tsgm.get(), - metadataHandler.getArtifactPathResolver(), - metadataHandler.getOutputStore()); + if (env.valuesMissing()) { + return; + } + if (!metadataFoundDuringActionExecution.isEmpty()) { + // We are in the interesting case of an action that discovered its inputs during + // execution, and found some new ones, but the new ones were already present in the graph. + // We must therefore cache the metadata for those new ones. + for (Map.Entry entry : metadataFoundDuringActionExecution.entrySet()) { + state.inputArtifactData.putWithNoDepOwner( + ArtifactSkyKey.artifact(entry.getKey()), (FileArtifactValue) entry.getValue()); + } + metadataHandler = + new ActionMetadataHandler( + state.inputArtifactData, + /*missingArtifactsAllowed=*/ false, + action.getOutputs(), + tsgm.get(), + metadataHandler.getArtifactPathResolver(), + metadataHandler.getOutputStore()); } } Preconditions.checkState(!env.valuesMissing(), action); @@ -790,15 +787,21 @@ public void run( } private static final Function TO_NONMANDATORY_SKYKEY = - artifact -> ArtifactSkyKey.key(artifact, /*isMandatory=*/ false); + new Function() { + @Nullable + @Override + public SkyKey apply(@Nullable Artifact artifact) { + return ArtifactSkyKey.key(artifact, /*isMandatory=*/ false); + } + }; - private enum DiscoveredState { - VALUES_MISSING, - NO_DISCOVERED_DATA, - DISCOVERED_DATA + private static Iterable newlyDiscoveredInputsToSkyKeys( + Iterable discoveredInputs, ActionInputMap inputArtifactData) { + return Iterables.transform( + filterKnownInputs(discoveredInputs, inputArtifactData), TO_NONMANDATORY_SKYKEY); } - private static DiscoveredState addDiscoveredInputs( + private static void addDiscoveredInputs( ActionInputMap inputData, Map> expandedArtifacts, Iterable discoveredInputs, @@ -812,28 +815,23 @@ private static DiscoveredState addDiscoveredInputs( // discovery. // Therefore there is no need to catch and rethrow exceptions as there is with #checkInputs. Map nonMandatoryDiscovered = - env.getValues(Iterables.transform(discoveredInputs, TO_NONMANDATORY_SKYKEY)); - if (env.valuesMissing()) { - return DiscoveredState.VALUES_MISSING; - } - if (nonMandatoryDiscovered.isEmpty()) { - return DiscoveredState.NO_DISCOVERED_DATA; - } - for (Map.Entry entry : nonMandatoryDiscovered.entrySet()) { - Artifact input = ArtifactSkyKey.artifact(entry.getKey()); - if (entry.getValue() instanceof TreeArtifactValue) { - TreeArtifactValue treeValue = (TreeArtifactValue) entry.getValue(); - expandedArtifacts.put(input, ImmutableSet.copyOf(treeValue.getChildren())); - for (Map.Entry child : - treeValue.getChildValues().entrySet()) { - inputData.putWithNoDepOwner(child.getKey(), child.getValue()); + env.getValues(newlyDiscoveredInputsToSkyKeys(discoveredInputs, inputData)); + if (!env.valuesMissing()) { + for (Map.Entry entry : nonMandatoryDiscovered.entrySet()) { + Artifact input = ArtifactSkyKey.artifact(entry.getKey()); + if (entry.getValue() instanceof TreeArtifactValue) { + TreeArtifactValue treeValue = (TreeArtifactValue) entry.getValue(); + expandedArtifacts.put(input, ImmutableSet.copyOf(treeValue.getChildren())); + for (Map.Entry child : + treeValue.getChildValues().entrySet()) { + inputData.putWithNoDepOwner(child.getKey(), child.getValue()); + } + inputData.putWithNoDepOwner(input, treeValue.getSelfData()); + } else { + inputData.putWithNoDepOwner(input, (FileArtifactValue) entry.getValue()); } - inputData.putWithNoDepOwner(input, treeValue.getSelfData()); - } else { - inputData.putWithNoDepOwner(input, (FileArtifactValue) entry.getValue()); } } - return DiscoveredState.DISCOVERED_DATA; } private static Object establishSkyframeDependencies( diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java index ba82e905190..caa83889069 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java @@ -20,6 +20,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ConcurrentHashMultiset; +import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -36,7 +37,6 @@ import com.google.devtools.build.lib.actions.ActionInputDepOwners; import com.google.devtools.build.lib.actions.ActionLookupData; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.ArtifactSkyKey; import com.google.devtools.build.lib.actions.LostInputsExecException; import com.google.devtools.build.lib.actions.LostInputsExecException.LostInputsActionExecutionException; import com.google.devtools.build.lib.bugreport.BugReport; @@ -45,10 +45,8 @@ import com.google.devtools.build.skyframe.SkyFunction.Restart; import com.google.devtools.build.skyframe.SkyKey; import java.util.ArrayDeque; -import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.logging.Logger; @@ -109,17 +107,22 @@ RewindPlan getRewindPlan( // failedAction, which the caller of getRewindPlan already knows must be restarted). ImmutableList.Builder additionalActionsToRestart = ImmutableList.builder(); - Set lostArtifacts = + HashMultimap lostInputsByDepOwners = getLostInputsByDepOwners( lostInputs, lostInputsException.getInputOwners(), runfilesDepOwners, ImmutableSet.copyOf(failedActionDeps), - failedAction, - lostInputsException); + failedAction); - for (Artifact.DerivedArtifact lostArtifact : lostArtifacts) { - SkyKey artifactKey = ArtifactSkyKey.mandatoryKey(lostArtifact); + for (Map.Entry> entry : + lostInputsByDepOwners.asMap().entrySet()) { + Artifact.DerivedArtifact lostArtifact = entry.getKey(); + checkIfLostArtifactIsSource( + lostArtifact, failedAction, lostInputsException, entry.getValue()); + + // Note that this artifact must be restarted. + rewindGraph.putEdge(actionLookupData, lostArtifact); Map actionMap = getActionsForLostArtifact(lostArtifact, env); if (actionMap == null) { @@ -129,10 +132,6 @@ RewindPlan getRewindPlan( } ImmutableList newlyVisitedActions = addArtifactDepsAndGetNewlyVisitedActions(rewindGraph, lostArtifact, actionMap); - // Note that artifactKey must be restarted. We do this after - // addArtifactDepsAndGetNewlyVisitedActions so that it can track if actions are already known - // to be in the graph. - rewindGraph.putEdge(actionLookupData, artifactKey); additionalActionsToRestart.addAll(actions(newlyVisitedActions)); checkActions(newlyVisitedActions, env, rewindGraph, additionalActionsToRestart); } @@ -189,28 +188,40 @@ private void checkIfActionLostInputTooManyTimes( } } - private static Set getLostInputsByDepOwners( + private void checkIfLostArtifactIsSource( + Artifact lostArtifact, + Action failedAction, + LostInputsActionExecutionException lostInputsException, + Collection associatedLostInputs) + throws ActionExecutionException { + if (lostArtifact.isSourceArtifact()) { + // Rewinding source artifacts is not possible. They should not be losable, but we tolerate + // their loss--by failing the build instead of crashing--in case some kind of infrastructure + // failure results in their apparent loss. + logger.info( + String.format( + "lostArtifact unexpectedly source. lostArtifact: %s, lostInputs for artifact: %s, " + + "failedAction: %.10000s", + lostArtifact, associatedLostInputs, failedAction)); + // Launder the LostInputs exception as a plain ActionExecutionException so that it may be + // processed by SkyframeActionExecutor without short-circuiting. + throw new ActionExecutionException(lostInputsException, failedAction, /*catastrophe=*/ false); + } + } + + private HashMultimap getLostInputsByDepOwners( ImmutableList lostInputs, LostInputsExecException.InputOwners inputOwners, ActionInputDepOwners runfilesDepOwners, ImmutableSet failedActionDeps, - Action failedAction, - LostInputsActionExecutionException lostInputsException) - throws ActionExecutionException { + Action failedActionForLogging) { - Set lostArtifacts = new HashSet<>(); + HashMultimap lostInputsByDepOwners = + HashMultimap.create(); for (ActionInput lostInput : lostInputs) { boolean foundLostInputDepOwner = false; Artifact owner = inputOwners.getOwner(lostInput); - if (owner != null && owner.isSourceArtifact()) { - // TODO(mschaller): tighten signatures for InputMappingsSink to make this impossible. - BugReport.sendBugReport( - new IllegalStateException( - "Unexpected source artifact as input owner: " + owner + " " + failedAction)); - throw new ActionExecutionException( - lostInputsException, failedAction, /*catastrophe=*/ false); - } // Rewinding must invalidate all Skyframe paths from the failed action to the action which // generates the lost input. Intermediate nodes not on the shortest path to that action may // have values that depend on the output of that action. If these intermediate nodes are not @@ -220,8 +231,11 @@ private static Set getLostInputsByDepOwners( if (owner != null) { runfilesTransitiveOwners = runfilesDepOwners.getDepOwners(owner); for (Artifact runfilesTransitiveOwner : runfilesTransitiveOwners) { - if (failedActionDeps.contains(ArtifactSkyKey.mandatoryKey(runfilesTransitiveOwner))) { - lostArtifacts.add((Artifact.DerivedArtifact) runfilesTransitiveOwner); + if (failedActionDeps.contains(runfilesTransitiveOwner)) { + // The lost input's owning tree artifact or fileset is included in a runfiles middleman + // that the action directly depends on. + lostInputsByDepOwners.put( + (Artifact.DerivedArtifact) runfilesTransitiveOwner, lostInput); foundLostInputDepOwner = true; } } @@ -229,33 +243,27 @@ private static Set getLostInputsByDepOwners( Collection runfilesOwners = runfilesDepOwners.getDepOwners(lostInput); for (Artifact runfilesOwner : runfilesOwners) { - if (runfilesOwner.isSourceArtifact()) { - // TODO(mschaller): tighten signatures for ActionInputMapSink to make this impossible. - BugReport.sendBugReport( - new IllegalStateException( - "Unexpected source artifact as runfile owner: " - + runfilesOwner - + " " - + failedAction)); - throw new ActionExecutionException( - lostInputsException, failedAction, /*catastrophe=*/ false); - } - if (failedActionDeps.contains(ArtifactSkyKey.mandatoryKey(runfilesOwner))) { - lostArtifacts.add((Artifact.DerivedArtifact) runfilesOwner); + if (failedActionDeps.contains(runfilesOwner)) { + lostInputsByDepOwners.put((Artifact.DerivedArtifact) runfilesOwner, lostInput); foundLostInputDepOwner = true; } } - if (owner != null && failedActionDeps.contains(ArtifactSkyKey.mandatoryKey(owner))) { + if (owner != null && failedActionDeps.contains(owner)) { // The lost input is included in a tree artifact or fileset that the action directly depends // on. - lostArtifacts.add((Artifact.DerivedArtifact) owner); + lostInputsByDepOwners.put((Artifact.DerivedArtifact) owner, lostInput); foundLostInputDepOwner = true; } - if (lostInput instanceof Artifact - && failedActionDeps.contains(ArtifactSkyKey.mandatoryKey((Artifact) lostInput))) { - lostArtifacts.add((Artifact.DerivedArtifact) lostInput); + if (failedActionDeps.contains(lostInput)) { + Preconditions.checkState( + lostInput instanceof Artifact, + "unexpected non-artifact lostInput which is a dep of the current action. " + + "lostInput: %s, failedAction: %.10000s", + lostInput, + failedActionForLogging); + lostInputsByDepOwners.put((Artifact.DerivedArtifact) lostInput, lostInput); foundLostInputDepOwner = true; } @@ -271,14 +279,14 @@ private static Set getLostInputsByDepOwners( // // In other cases, such as with bugs, when the action fails enough it will cause a crash in // checkIfActionLostInputTooManyTimes. We log that this has occurred. - logger.warning( + logger.info( String.format( "lostInput not a dep of the failed action, and can't be associated with such a dep. " + "lostInput: %s, owner: %s, runfilesOwners: %s, runfilesTransitiveOwners:" + " %s, failedAction: %.10000s", - lostInput, owner, runfilesOwners, runfilesTransitiveOwners, failedAction)); + lostInput, owner, runfilesOwners, runfilesTransitiveOwners, failedActionForLogging)); } - return lostArtifacts; + return lostInputsByDepOwners; } /** @@ -298,36 +306,23 @@ private void checkActions( ActionAndLookupData actionAndLookupData = uncheckedActions.removeFirst(); ActionLookupData actionKey = actionAndLookupData.lookupData(); Action action = actionAndLookupData.action(); - ArrayList artifactsToCheck = new ArrayList<>(); - ArrayList newlyDiscoveredActions = new ArrayList<>(); + HashSet artifactsToCheck = new HashSet<>(); if (action instanceof SkyframeAwareAction) { // This action depends on more than just its input artifact values. We need to also restart - // the Skyframe subgraph it depends on, up to and including any artifacts, which may - // aggregate multiple actions. - addSkyframeAwareDepsAndGetNewlyVisitedArtifactsAndActions( - rewindGraph, - actionKey, - (SkyframeAwareAction) action, - artifactsToCheck, - newlyDiscoveredActions); + // the Skyframe subgraph it depends on, up to and including any artifacts. + artifactsToCheck.addAll( + addSkyframeAwareDepsAndGetNewlyVisitedArtifacts( + rewindGraph, actionKey, (SkyframeAwareAction) action)); } if (action.mayInsensitivelyPropagateInputs()) { // Restarting this action won't recreate the missing input. We need to also restart this // action's non-source inputs and the actions which created those inputs. - addPropagatingActionDepsAndGetNewlyVisitedArtifactsAndActions( - rewindGraph, actionKey, action, artifactsToCheck, newlyDiscoveredActions); + artifactsToCheck.addAll( + addPropagatingActionDepsAndGetNewlyVisitedArtifacts(rewindGraph, actionKey, action)); } - for (ActionLookupData actionLookupData : newlyDiscoveredActions) { - Action additionalAction = - Preconditions.checkNotNull( - ActionExecutionFunction.getActionForLookupData(env, actionLookupData), - actionLookupData); - additionalActionsToRestart.add(additionalAction); - uncheckedActions.add(ActionAndLookupData.create(actionLookupData, additionalAction)); - } for (Artifact.DerivedArtifact artifact : artifactsToCheck) { Map actionMap = getActionsForLostArtifact(artifact, env); if (actionMap == null) { @@ -343,67 +338,56 @@ private void checkActions( /** * For a {@link SkyframeAwareAction} {@code action} with key {@code actionKey}, add its Skyframe - * subgraph to {@code rewindGraph}, any {@link Artifact}s to {@code newlyVisitedArtifacts}, and - * any {@link ActionLookupData}s to {@code newlyVisitedActions}. + * subgraph to {@code rewindGraph}. + * + *

Returns a list of artifacts which were newly added to the graph. */ - private static void addSkyframeAwareDepsAndGetNewlyVisitedArtifactsAndActions( - MutableGraph rewindGraph, - ActionLookupData actionKey, - SkyframeAwareAction action, - ArrayList newlyVisitedArtifacts, - ArrayList newlyVisitedActions) { + private ImmutableList addSkyframeAwareDepsAndGetNewlyVisitedArtifacts( + MutableGraph rewindGraph, ActionLookupData actionKey, SkyframeAwareAction action) { ImmutableGraph graph = action.getSkyframeDependenciesForRewinding(actionKey); if (graph.nodes().isEmpty()) { - return; + return ImmutableList.of(); } assertSkyframeAwareRewindingGraph(graph, actionKey); + ImmutableList.Builder newlyVisitedArtifacts = ImmutableList.builder(); Set> edges = graph.edges(); for (EndpointPair edge : edges) { SkyKey target = edge.target(); if (target instanceof Artifact && rewindGraph.addNode(target)) { newlyVisitedArtifacts.add(((Artifact.DerivedArtifact) target)); } - if (target instanceof ActionLookupData && rewindGraph.addNode(target)) { - newlyVisitedActions.add(((ActionLookupData) target)); - } rewindGraph.putEdge(edge.source(), edge.target()); } + return newlyVisitedArtifacts.build(); } /** * For a propagating {@code action} with key {@code actionKey}, add its generated inputs' keys to - * {@code rewindGraph}, add edges from {@code actionKey} to those keys, add any {@link Artifact}s - * to {@code newlyVisitedArtifacts}, and add any {@link ActionLookupData}s to {@code - * newlyVisitedActions}. + * {@code rewindGraph} and add edges from {@code actionKey} to those keys. + * + *

Returns a list of artifacts which were newly added to the graph. */ - private static void addPropagatingActionDepsAndGetNewlyVisitedArtifactsAndActions( - MutableGraph rewindGraph, - ActionLookupData actionKey, - Action action, - ArrayList newlyVisitedArtifacts, - ArrayList newlyVisitedActions) { + private ImmutableList + addPropagatingActionDepsAndGetNewlyVisitedArtifacts( + MutableGraph rewindGraph, ActionLookupData actionKey, Action action) { + ImmutableList.Builder newlyVisitedArtifacts = ImmutableList.builder(); for (Artifact input : action.getInputs()) { if (input.isSourceArtifact()) { continue; } - SkyKey artifactKey = ArtifactSkyKey.mandatoryKey(input); // Restarting all derived inputs of propagating actions is overkill. Preferably, we'd want // to only restart the inputs which correspond to the known lost outputs. The information - // to do this is probably present in the data available to #getRewindPlan. + // to do this is probably present in the ActionInputs contained in getRewindPlan's + // lostInputsByOwners. // // Rewinding is expected to be rare, so refining this may not be necessary. - boolean newlyVisited = rewindGraph.addNode(artifactKey); - if (newlyVisited) { - if (artifactKey instanceof Artifact) { - newlyVisitedArtifacts.add((Artifact.DerivedArtifact) artifactKey); - } else if (artifactKey instanceof ActionLookupData) { - newlyVisitedActions.add((ActionLookupData) artifactKey); - } + if (rewindGraph.addNode(input)) { + newlyVisitedArtifacts.add((Artifact.DerivedArtifact) input); } - rewindGraph.putEdge(actionKey, artifactKey); + rewindGraph.putEdge(actionKey, input); } // Rewinding ignores artifacts returned by Action#getAllowedDerivedInputs because: @@ -418,6 +402,8 @@ private static void addPropagatingActionDepsAndGetNewlyVisitedArtifactsAndAction + "%.10000s", actionKey, action))); } + + return newlyVisitedArtifacts.build(); } /** @@ -434,15 +420,12 @@ private ImmutableList addArtifactDepsAndGetNewlyVisitedActi ImmutableList.Builder newlyVisitedActions = ImmutableList.builderWithExpectedSize(actionMap.size()); - SkyKey artifactKey = ArtifactSkyKey.mandatoryKey(artifact); for (Map.Entry actionEntry : actionMap.entrySet()) { ActionLookupData actionKey = actionEntry.getKey(); if (rewindGraph.addNode(actionKey)) { newlyVisitedActions.add(ActionAndLookupData.create(actionKey, actionEntry.getValue())); } - if (!artifactKey.equals(actionKey)) { - rewindGraph.putEdge(artifactKey, actionKey); - } + rewindGraph.putEdge(artifact, actionKey); } return newlyVisitedActions.build(); } @@ -463,9 +446,7 @@ private Map getActionsForLostArtifact( Map actions = Maps.newHashMapWithExpectedSize(actionExecutionDeps.size()); for (ActionLookupData dep : actionExecutionDeps) { - actions.put( - dep, - Preconditions.checkNotNull(ActionExecutionFunction.getActionForLookupData(env, dep))); + actions.put(dep, ActionExecutionFunction.getActionForLookupData(env, dep)); } return actions; } @@ -477,26 +458,24 @@ private Map getActionsForLostArtifact( @Nullable private Set getActionExecutionDeps( Artifact.DerivedArtifact lostInput, Environment env) throws InterruptedException { - if (!lostInput.isTreeArtifact()) { - return ImmutableSet.of(lostInput.getGeneratingActionKey()); - } ArtifactFunction.ArtifactDependencies artifactDependencies = ArtifactFunction.ArtifactDependencies.discoverDependencies(lostInput, env); if (artifactDependencies == null) { return null; } - if (!artifactDependencies.isTemplateActionForTreeArtifact()) { - return ImmutableSet.of(lostInput.getGeneratingActionKey()); - } - ArtifactFunction.ActionTemplateExpansion actionTemplateExpansion = - artifactDependencies.getActionTemplateExpansion(env); - if (actionTemplateExpansion == null) { - return null; + if (artifactDependencies.isTemplateActionForTreeArtifact()) { + ArtifactFunction.ActionTemplateExpansion actionTemplateExpansion = + artifactDependencies.getActionTemplateExpansion(env); + if (actionTemplateExpansion == null) { + return null; + } + // This ignores the ActionTemplateExpansionKey dependency of the template artifact because we + // expect to never need to rewind that. + return ImmutableSet.copyOf(actionTemplateExpansion.getExpandedActionExecutionKeys()); } - // This ignores the ActionTemplateExpansionKey dependency of the template artifact because we - // expect to never need to rewind that. - return ImmutableSet.copyOf(actionTemplateExpansion.getExpandedActionExecutionKeys()); + + return ImmutableSet.of(artifactDependencies.getNontemplateActionExecutionKey()); } private static void assertSkyframeAwareRewindingGraph( @@ -526,6 +505,18 @@ private static void assertSkyframeAwareRewindingGraph( for (EndpointPair edge : graph.edges()) { SkyKey target = edge.target(); + + // Action rewinding could be changed to support finding other actions in a + // SkyframeAwareAction's rewinding graph. For now, fail if any are found, because it's + // currently impossible and unsupported. + Preconditions.checkArgument( + !(target instanceof ActionLookupData), + "SkyframeAwareAction's rewinding graph contains non-root action node. graph: %s, " + + "rootActionNode: %s, unexpectedActionNode: %s", + graph, + actionKey, + target); + Preconditions.checkArgument( !(target instanceof Artifact && ((Artifact) target).isSourceArtifact()), "SkyframeAwareAction's rewinding graph contains source artifact. graph: %s, " @@ -590,7 +581,8 @@ static ActionAndLookupData create(ActionLookupData lookupData, Action action) { } } - private static ImmutableList actions(List newlyVisitedActions) { + private static ImmutableList actions( + ImmutableList newlyVisitedActions) { return newlyVisitedActions.stream().map(ActionAndLookupData::action).collect(toImmutableList()); } } diff --git a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkActionFactoryApi.java b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkActionFactoryApi.java index 18618b93ee2..51a113325db 100644 --- a/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkActionFactoryApi.java +++ b/dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkActionFactoryApi.java @@ -26,7 +26,6 @@ import com.google.devtools.build.lib.syntax.SkylarkDict; import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; -import com.google.devtools.build.lib.syntax.StarlarkSemantics; /** Module providing functions to create actions. */ @SkylarkModule( @@ -410,9 +409,7 @@ public void run( + "

(Deprecated) If command is a sequence of strings, " + "the first item is the executable to run and the remaining items are its " + "arguments. If this form is used, the arguments parameter must " - + "not be supplied. Note that this form is deprecated and will soon " - + "be removed. It is disabled with `--incompatible_run_shell_command_string`. " - + "Use this flag to verify your code is compatible. " + + "not be supplied." + "" + "

Bazel uses the same shell to execute the command as it does for " + "genrules."), @@ -466,10 +463,9 @@ public void run( "(Experimental) sets the input runfiles metadata; " + "they are typically generated by resolve_command.") }, - useStarlarkSemantics = true, useLocation = true) public void runShell( - SkylarkList outputs, + SkylarkList outputs, Object inputs, Object toolsUnchecked, Object arguments, @@ -480,8 +476,7 @@ public void runShell( Object envUnchecked, Object executionRequirementsUnchecked, Object inputManifestsUnchecked, - Location location, - StarlarkSemantics semantics) + Location location) throws EvalException; @SkylarkCallable(