Refactor lost input ownership calculation, do it unconditionally
Previously, lost input ownership information from an action's inputDeps
was only calculated if any lost inputs came from runfiles. By
calculating ownership information unconditionally, lost input owners can
be determined for each kind of aggregating artifact, except for filesets,
at the ActionExecutionFunction level.
By not treating runfiles ownership specially, ActionRewindStrategy's
lost-input-owning direct deps calculation can be simpler. Also, the
runfiles-specific tracking in LostInputsExecException.InputOwners is no
longer necessary, so it can be eliminated in favor of the now-equivalent
ActionInputDepOwners.
This CL also makes LostInputs{ExecException,ActionExecutionException}
independent. That allows other parts of action execution, besides those
which throw ExecExceptions, to be able to communicate lost input
failures. Now that the ActionExecutionFunction level calculates lost
input ownership unconditionally, the requirements for those parts is
looser. Before, they would have been responsible for determining all
ownership relations except for those from runfiles. Now, they're
responsible only for determining ownership relations from filesets.
RELNOTES: None.
PiperOrigin-RevId: 280692324
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputDepOwnerMap.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputDepOwnerMap.java
index 85620ad..3d9ae5c 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionInputDepOwnerMap.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputDepOwnerMap.java
@@ -33,6 +33,10 @@
@Override
public boolean put(ActionInput input, FileArtifactValue metadata, @Nullable Artifact depOwner) {
+ return addOwner(input, depOwner);
+ }
+
+ public boolean addOwner(ActionInput input, @Nullable Artifact depOwner) {
if (depOwner == null || !inputsOfInterest.contains(input)) {
return false;
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputDepOwners.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputDepOwners.java
index a34f3ae..0cdcd00 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionInputDepOwners.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputDepOwners.java
@@ -13,18 +13,17 @@
// limitations under the License.
package com.google.devtools.build.lib.actions;
-import com.google.common.collect.ImmutableList;
import java.util.Collection;
/**
- * Association between {@link ActionInput}s and the {@link Artifact}s, directly depended on by an
- * action, that are responsible for that action's inclusion of those inputs.
+ * Association between {@link ActionInput}s and the {@link Artifact}s, directly or indirectly
+ * depended on by an action, that are responsible for that action's inclusion of those inputs.
+ *
+ * <p>The association is not necessarily comprehensive. Inputs may have other owners not specified
+ * here, or the owners specified here may not be direct dependencies of the action.
*/
public interface ActionInputDepOwners {
- /** An {@link ActionInputDepOwners} without any ownership associations. */
- ActionInputDepOwners EMPTY_INSTANCE = input -> ImmutableList.of();
-
/**
* Returns the collection of {@link Artifact}s associated with {@code input}. The collection is
* empty if no such association exists.
diff --git a/src/main/java/com/google/devtools/build/lib/actions/LostInputsActionExecutionException.java b/src/main/java/com/google/devtools/build/lib/actions/LostInputsActionExecutionException.java
new file mode 100644
index 0000000..bf22401
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/actions/LostInputsActionExecutionException.java
@@ -0,0 +1,104 @@
+// Copyright 2019 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.actions;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.util.io.FileOutErr;
+import com.google.devtools.build.lib.vfs.Path;
+
+/**
+ * An {@link ActionExecutionException} thrown when an action fails to execute because one or more of
+ * its inputs was lost. In some cases, Bazel may know how to fix this on its own.
+ */
+public class LostInputsActionExecutionException extends ActionExecutionException {
+
+ /** Maps lost input digests to their ActionInputs. */
+ private final ImmutableMap<String, ActionInput> lostInputs;
+
+ private final ActionInputDepOwners owners;
+
+ /**
+ * If an ActionStartedEvent was emitted, then:
+ *
+ * <ul>
+ * <li>if rewinding is attempted, then an ActionRewindEvent should be emitted.
+ * <li>if rewinding fails, then an ActionCompletionEvent should be emitted.
+ * </ul>
+ */
+ private boolean actionStartedEventAlreadyEmitted;
+
+ /** Used to report the action execution failure if rewinding also fails. */
+ private Path primaryOutputPath;
+
+ /**
+ * Used to report the action execution failure if rewinding also fails. Note that this will be
+ * closed, so it may only be used for reporting.
+ */
+ private FileOutErr fileOutErr;
+
+ /** Used to inform rewinding that lost inputs were found during input discovery. */
+ private boolean fromInputDiscovery;
+
+ public LostInputsActionExecutionException(
+ String message,
+ ImmutableMap<String, ActionInput> lostInputs,
+ ActionInputDepOwners owners,
+ Action action,
+ Exception cause) {
+ super(message, cause, action, /*catastrophe=*/ false);
+ this.lostInputs = lostInputs;
+ this.owners = owners;
+ }
+
+ public ImmutableMap<String, ActionInput> getLostInputs() {
+ return lostInputs;
+ }
+
+ public ActionInputDepOwners getOwners() {
+ return owners;
+ }
+
+ public Path getPrimaryOutputPath() {
+ return primaryOutputPath;
+ }
+
+ public void setPrimaryOutputPath(Path primaryOutputPath) {
+ this.primaryOutputPath = primaryOutputPath;
+ }
+
+ public FileOutErr getFileOutErr() {
+ return fileOutErr;
+ }
+
+ public void setFileOutErr(FileOutErr fileOutErr) {
+ this.fileOutErr = fileOutErr;
+ }
+
+ public boolean isActionStartedEventAlreadyEmitted() {
+ return actionStartedEventAlreadyEmitted;
+ }
+
+ public void setActionStartedEventAlreadyEmitted() {
+ this.actionStartedEventAlreadyEmitted = true;
+ }
+
+ public boolean isFromInputDiscovery() {
+ return fromInputDiscovery;
+ }
+
+ public void setFromInputDiscovery() {
+ this.fromInputDiscovery = true;
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java b/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java
index e04d2af..71f861e 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java
@@ -17,10 +17,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.util.io.FileOutErr;
-import com.google.devtools.build.lib.vfs.Path;
-import java.util.Set;
-import javax.annotation.Nullable;
/**
* An {@link ExecException} thrown when an action fails to execute because one or more of its inputs
@@ -28,14 +24,16 @@
*/
public class LostInputsExecException extends ExecException {
+ /** Maps lost input digests to their ActionInputs. */
private final ImmutableMap<String, ActionInput> lostInputs;
- private final InputOwners inputOwners;
+
+ private final ActionInputDepOwners owners;
public LostInputsExecException(
- ImmutableMap<String, ActionInput> lostInputs, InputOwners inputOwners) {
+ ImmutableMap<String, ActionInput> lostInputs, ActionInputDepOwners owners) {
super("lost inputs with digests: " + Joiner.on(",").join(lostInputs.keySet()));
this.lostInputs = lostInputs;
- this.inputOwners = inputOwners;
+ this.owners = owners;
}
@VisibleForTesting
@@ -44,102 +42,15 @@
}
@VisibleForTesting
- public InputOwners getInputOwners() {
- return inputOwners;
+ public ActionInputDepOwners getOwners() {
+ return owners;
}
@Override
public ActionExecutionException toActionExecutionException(
String messagePrefix, boolean verboseFailures, Action action) {
String message = messagePrefix + " failed";
- return new LostInputsActionExecutionException(message + ": " + getMessage(), this, action);
- }
-
- /** An {@link ActionExecutionException} wrapping a {@link LostInputsExecException}. */
- public static class LostInputsActionExecutionException extends ActionExecutionException {
-
- /**
- * If an ActionStartedEvent was emitted, then:
- *
- * <ul>
- * <li>if rewinding is attempted, then an ActionRewindEvent should be emitted.
- * <li>if rewinding fails, then an ActionCompletionEvent should be emitted.
- * </ul>
- */
- private boolean actionStartedEventAlreadyEmitted;
-
- /** Used to report the action execution failure if rewinding also fails. */
- private Path primaryOutputPath;
-
- /**
- * Used to report the action execution failure if rewinding also fails. Note that this will be
- * closed, so it may only be used for reporting.
- */
- private FileOutErr fileOutErr;
-
- /** Used to inform rewinding that lost inputs were found during input discovery. */
- private boolean fromInputDiscovery;
-
- private LostInputsActionExecutionException(
- String message, LostInputsExecException cause, Action action) {
- super(message, cause, action, /*catastrophe=*/ false);
- }
-
- public ImmutableMap<String, ActionInput> getLostInputs() {
- return ((LostInputsExecException) getCause()).getLostInputs();
- }
-
- public InputOwners getInputOwners() {
- return ((LostInputsExecException) getCause()).getInputOwners();
- }
-
- public Path getPrimaryOutputPath() {
- return primaryOutputPath;
- }
-
- public void setPrimaryOutputPath(Path primaryOutputPath) {
- this.primaryOutputPath = primaryOutputPath;
- }
-
- public FileOutErr getFileOutErr() {
- return fileOutErr;
- }
-
- public void setFileOutErr(FileOutErr fileOutErr) {
- this.fileOutErr = fileOutErr;
- }
-
- public boolean isActionStartedEventAlreadyEmitted() {
- return actionStartedEventAlreadyEmitted;
- }
-
- public void setActionStartedEventAlreadyEmitted() {
- this.actionStartedEventAlreadyEmitted = true;
- }
-
- public boolean isFromInputDiscovery() {
- return fromInputDiscovery;
- }
-
- public void setFromInputDiscovery() {
- this.fromInputDiscovery = true;
- }
- }
-
- /**
- * Specifies the owning {@link Artifact}s that were responsible for the lost inputs and whether
- * the inputs came from runfiles.
- */
- public interface InputOwners {
-
- /**
- * Returns the owning {@link Artifact} that was responsible for the lost {@link ActionInput} or
- * {@code null} if there is no such owner. Throws if {@code input} was not lost.
- */
- @Nullable
- Artifact getOwner(ActionInput input);
-
- /** Returns the lost {@link ActionInput}s that came from runfiles along with their owners. */
- Set<ActionInput> getRunfilesInputsAndOwners();
+ return new LostInputsActionExecutionException(
+ message + ": " + getMessage(), lostInputs, owners, action, /*cause=*/ this);
}
}
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 7ecefe0..852a4d3 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
@@ -40,7 +40,7 @@
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
-import com.google.devtools.build.lib.actions.LostInputsExecException.LostInputsActionExecutionException;
+import com.google.devtools.build.lib.actions.LostInputsActionExecutionException;
import com.google.devtools.build.lib.actions.MissingDepException;
import com.google.devtools.build.lib.actions.MissingInputFileException;
import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit;
@@ -82,6 +82,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -447,31 +448,8 @@
RewindPlan rewindPlan = null;
try {
- // Reconstruct the relationship between lost inputs and this action's direct deps if any of
- // the lost inputs came from runfiles:
- ActionInputDepOwners runfilesDepOwners;
- Set<ActionInput> lostRunfiles = e.getInputOwners().getRunfilesInputsAndOwners();
- if (!lostRunfiles.isEmpty()) {
- try {
- runfilesDepOwners =
- getInputDepOwners(
- env,
- action,
- inputDeps,
- allInputs,
- action.discoversInputs()
- ? ImmutableSet.copyOf(action.getMandatoryInputs())
- : null,
- lostRunfiles);
- } catch (ActionExecutionException unexpected) {
- // getInputDepOwners should not be able to throw, because it does the same work as
- // checkInputs, so if getInputDepOwners throws then checkInputs should have thrown, and if
- // checkInputs threw then we shouldn't have reached this point in action execution.
- throw new IllegalStateException(unexpected);
- }
- } else {
- runfilesDepOwners = ActionInputDepOwners.EMPTY_INSTANCE;
- }
+ ActionInputDepOwners inputDepOwners =
+ createAugmentedInputDepOwners(e, action, env, inputDeps, allInputs);
// Collect the set of direct deps of this action which may be responsible for the lost inputs,
// some of which may be discovered.
@@ -498,7 +476,7 @@
try {
rewindPlan =
actionRewindStrategy.getRewindPlan(
- action, actionLookupData, failedActionDeps, e, runfilesDepOwners, env);
+ action, actionLookupData, failedActionDeps, e, inputDepOwners, env);
} catch (ActionExecutionException rewindingFailedException) {
// This call to processAndGetExceptionToThrow will emit an ActionExecutedEvent and report
// the error. The previous call to processAndGetExceptionToThrow didn't.
@@ -532,6 +510,57 @@
}
}
+ /**
+ * Returns an augmented version of {@code e.getOwners()}'s {@link ActionInputDepOwners}, adding
+ * ownership information from {@code inputDeps}.
+ *
+ * <p>This compensates for how the ownership information in {@code e.getOwners()} is potentially
+ * incomplete. E.g., it may lack knowledge of a runfiles middleman owning a fileset, even if it
+ * knows that fileset owns a lost input.
+ */
+ private static ActionInputDepOwners createAugmentedInputDepOwners(
+ LostInputsActionExecutionException e,
+ Action action,
+ Environment env,
+ Map<SkyKey, ValueOrException2<IOException, ActionExecutionException>> inputDeps,
+ Iterable<Artifact> allInputs)
+ throws InterruptedException {
+
+ Set<ActionInput> lostInputsAndOwnersSoFar = new HashSet<>();
+ ActionInputDepOwners owners = e.getOwners();
+ for (ActionInput lostInput : e.getLostInputs().values()) {
+ lostInputsAndOwnersSoFar.add(lostInput);
+ lostInputsAndOwnersSoFar.addAll(owners.getDepOwners(lostInput));
+ }
+
+ ActionInputDepOwnerMap inputDepOwners;
+ try {
+ inputDepOwners =
+ getInputDepOwners(
+ env,
+ action,
+ inputDeps,
+ allInputs,
+ action.discoversInputs() ? ImmutableSet.copyOf(action.getMandatoryInputs()) : null,
+ lostInputsAndOwnersSoFar);
+ } catch (ActionExecutionException unexpected) {
+ // getInputDepOwners should not be able to throw, because it does the same work as
+ // checkInputs, so if getInputDepOwners throws then checkInputs should have thrown, and if
+ // checkInputs threw then we shouldn't have reached this point in action execution.
+ throw new IllegalStateException(unexpected);
+ }
+
+ // Ownership information from inputDeps may be incomplete. Notably, it does not expand
+ // filesets. Fileset and other ownership relationships should have been captured in the
+ // exception's ActionInputDepOwners, and this copies that knowledge into the augmented version.
+ for (ActionInput lostInput : e.getLostInputs().values()) {
+ for (Artifact depOwner : owners.getDepOwners(lostInput)) {
+ inputDepOwners.addOwner(lostInput, depOwner);
+ }
+ }
+ return inputDepOwners;
+ }
+
@Nullable
static Action getActionForLookupData(Environment env, ActionLookupData actionLookupData)
throws InterruptedException {
@@ -1080,7 +1109,7 @@
/**
* Reconstructs the relationships between lost inputs and the direct deps responsible for them.
*/
- private static ActionInputDepOwners getInputDepOwners(
+ private static ActionInputDepOwnerMap getInputDepOwners(
Environment env,
Action action,
Map<SkyKey, ValueOrException2<IOException, ActionExecutionException>> inputDeps,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java
index 4187de5..f2fe907 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java
@@ -36,8 +36,7 @@
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.LostInputsExecException;
-import com.google.devtools.build.lib.actions.LostInputsExecException.LostInputsActionExecutionException;
+import com.google.devtools.build.lib.actions.LostInputsActionExecutionException;
import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
@@ -91,7 +90,7 @@
ActionLookupData actionLookupData,
Iterable<? extends SkyKey> failedActionDeps,
LostInputsActionExecutionException lostInputsException,
- ActionInputDepOwners runfilesDepOwners,
+ ActionInputDepOwners inputDepOwners,
Environment env)
throws ActionExecutionException, InterruptedException {
checkIfActionLostInputTooManyTimes(actionLookupData, failedAction, lostInputsException);
@@ -109,10 +108,9 @@
ImmutableList.Builder<Action> additionalActionsToRestart = ImmutableList.builder();
Set<Artifact.DerivedArtifact> lostArtifacts =
- getLostInputsByDepOwners(
+ getLostInputOwningDirectDeps(
lostInputs,
- lostInputsException.getInputOwners(),
- runfilesDepOwners,
+ inputDepOwners,
ImmutableSet.copyOf(failedActionDeps),
failedAction,
lostInputsException);
@@ -188,82 +186,59 @@
}
}
- private static Set<Artifact.DerivedArtifact> getLostInputsByDepOwners(
+ private static Set<Artifact.DerivedArtifact> getLostInputOwningDirectDeps(
ImmutableList<ActionInput> lostInputs,
- LostInputsExecException.InputOwners inputOwners,
- ActionInputDepOwners runfilesDepOwners,
+ ActionInputDepOwners inputDepOwners,
ImmutableSet<SkyKey> failedActionDeps,
Action failedAction,
LostInputsActionExecutionException lostInputsException)
throws ActionExecutionException {
- Set<Artifact.DerivedArtifact> lostArtifacts = new HashSet<>();
+ Set<Artifact.DerivedArtifact> lostInputOwningDirectDeps = new HashSet<>();
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
- // invalidated, then their values may become stale.
+ Collection<Artifact> owners = inputDepOwners.getDepOwners(lostInput);
+ for (Artifact owner : owners) {
+ checkDerived(/*lostInputQualifier=*/ " owner", owner, failedAction, lostInputsException);
- Collection<Artifact> runfilesTransitiveOwners = null;
- if (owner != null) {
- runfilesTransitiveOwners = runfilesDepOwners.getDepOwners(owner);
- for (Artifact runfilesTransitiveOwner : runfilesTransitiveOwners) {
- if (failedActionDeps.contains(Artifact.key(runfilesTransitiveOwner))) {
- lostArtifacts.add((Artifact.DerivedArtifact) runfilesTransitiveOwner);
+ // 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
+ // invalidated, then their values may become stale. Therefore, this method collects not only
+ // the first action dep associated with the lost input, but all of them.
+
+ Collection<Artifact> transitiveOwners = inputDepOwners.getDepOwners(owner);
+ for (Artifact transitiveOwner : transitiveOwners) {
+ checkDerived(
+ /*lostInputQualifier=*/ " transitive owner",
+ transitiveOwner,
+ failedAction,
+ lostInputsException);
+
+ if (failedActionDeps.contains(Artifact.key(transitiveOwner))) {
+ // The lost input is included in an aggregation artifact (e.g. a tree artifact or
+ // fileset) that is included by an aggregation artifact (e.g. a middleman) that the
+ // action directly depends on.
+ lostInputOwningDirectDeps.add((Artifact.DerivedArtifact) transitiveOwner);
foundLostInputDepOwner = true;
}
}
- }
- Collection<Artifact> 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(Artifact.key(runfilesOwner))) {
- lostArtifacts.add((Artifact.DerivedArtifact) runfilesOwner);
+ if (failedActionDeps.contains(Artifact.key(owner))) {
+ // The lost input is included in an aggregation artifact (e.g. a tree artifact, fileset,
+ // or middleman) that the action directly depends on.
+ lostInputOwningDirectDeps.add((Artifact.DerivedArtifact) owner);
foundLostInputDepOwner = true;
}
}
- if (owner != null && failedActionDeps.contains(Artifact.key(owner))) {
- // The lost input is included in a tree artifact or fileset that the action directly depends
- // on.
- lostArtifacts.add((Artifact.DerivedArtifact) owner);
- foundLostInputDepOwner = true;
- }
-
if (lostInput instanceof Artifact
&& failedActionDeps.contains(Artifact.key((Artifact) lostInput))) {
+ checkDerived(
+ /*lostInputQualifier=*/ "", (Artifact) lostInput, failedAction, lostInputsException);
- if (((Artifact) lostInput).isSourceArtifact()) {
- BugReport.sendBugReport(
- new IllegalStateException(
- "Unexpected source artifact as input: " + lostInput + " " + failedAction));
- throw new ActionExecutionException(
- lostInputsException, failedAction, /*catastrophe=*/ false);
- }
-
- lostArtifacts.add((Artifact.DerivedArtifact) lostInput);
+ lostInputOwningDirectDeps.add((Artifact.DerivedArtifact) lostInput);
foundLostInputDepOwner = true;
}
@@ -282,11 +257,28 @@
logger.warning(
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: %s, owners: %s, failedAction: %.10000s",
+ lostInput, owners, failedAction));
}
- return lostArtifacts;
+ return lostInputOwningDirectDeps;
+ }
+
+ private static void checkDerived(
+ String lostInputQualifier,
+ Artifact expectedDerived,
+ Action failedAction,
+ LostInputsActionExecutionException lostInputsException)
+ throws ActionExecutionException {
+ if (!expectedDerived.isSourceArtifact()) {
+ return;
+ }
+ // TODO(b/19539699): tighten signatures for ActionInputDepOwnerMap to make this impossible.
+ BugReport.sendBugReport(
+ new IllegalStateException(
+ String.format(
+ "Unexpected source artifact as lost input%s: %s %s",
+ lostInputQualifier, expectedDerived, failedAction)));
+ throw new ActionExecutionException(lostInputsException, failedAction, /*catastrophe=*/ false);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index 21df3bd..9a370f6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -61,7 +61,7 @@
import com.google.devtools.build.lib.actions.Executor;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
-import com.google.devtools.build.lib.actions.LostInputsExecException.LostInputsActionExecutionException;
+import com.google.devtools.build.lib.actions.LostInputsActionExecutionException;
import com.google.devtools.build.lib.actions.MapBasedActionGraph;
import com.google.devtools.build.lib.actions.MetadataConsumer;
import com.google.devtools.build.lib.actions.MetadataProvider;
