Slightly improve bug reports for unexpected conditions in ActionExecutionFunction.
PiperOrigin-RevId: 375096335
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
index 93497c7..09c790b 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
@@ -452,6 +452,14 @@
}
@Override
+ public final String toDebugString() {
+ if (hasGeneratingActionKey()) {
+ return super.toDetailString() + " (" + getGeneratingActionKey() + ")";
+ }
+ return super.toDebugString();
+ }
+
+ @Override
public final PathFragment getRootRelativePath() {
return getExecPath().relativeTo(getRoot().getExecPath());
}
@@ -886,6 +894,13 @@
}
}
+ public String toDebugString() {
+ if (getOwner() == null || getOwner().toPathFragment().equals(getExecPath())) {
+ return toDetailString();
+ }
+ return toDetailString() + " (" + getArtifactOwner() + ")";
+ }
+
@Override
public final SkyFunctionName functionName() {
return ARTIFACT;
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 076fdd0..1b56b23 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
@@ -283,7 +283,9 @@
try {
if (previousExecution == null && !state.hasArtifactData()) {
// Do we actually need to find our metadata?
- checkedInputs = checkInputs(env, action, inputDeps, allInputs, mandatoryInputs, depKeys);
+ checkedInputs =
+ checkInputs(
+ env, action, inputDeps, allInputs, mandatoryInputs, depKeys, actionLookupData);
}
} catch (ActionExecutionException e) {
// Remove action from state map in case it's there (won't be unless it discovers inputs).
@@ -476,7 +478,8 @@
RewindPlan rewindPlan = null;
try {
ActionInputDepOwners inputDepOwners =
- createAugmentedInputDepOwners(e, action, env, inputDeps, allInputs, depKeys);
+ createAugmentedInputDepOwners(
+ e, action, env, inputDeps, allInputs, depKeys, actionLookupData);
// Collect the set of direct deps of this action which may be responsible for the lost inputs,
// some of which may be discovered.
@@ -562,7 +565,8 @@
ArtifactNestedSetEvalException>>
inputDeps,
NestedSet<Artifact> allInputs,
- Iterable<SkyKey> requestedSkyKeys)
+ Iterable<SkyKey> requestedSkyKeys,
+ ActionLookupData actionLookupDataForError)
throws InterruptedException {
Set<ActionInput> lostInputsAndOwnersSoFar = new HashSet<>();
@@ -582,7 +586,8 @@
allInputs,
action.discoversInputs() ? action.getMandatoryInputs().toSet() : null,
requestedSkyKeys,
- lostInputsAndOwnersSoFar);
+ lostInputsAndOwnersSoFar,
+ actionLookupDataForError);
} 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
@@ -963,7 +968,10 @@
if (!input.isSourceArtifact()) {
bugReporter.sendBugReport(
new IllegalStateException(
- "Non-source artifact had SourceArtifactException" + input, e));
+ String.format(
+ "Non-source artifact had SourceArtifactException %s %s",
+ input.toDebugString(), actionForError.prettyPrint()),
+ e));
}
skyframeActionExecutor.printError(e.getMessage(), actionForError);
@@ -1099,7 +1107,8 @@
inputDeps,
NestedSet<Artifact> allInputs,
ImmutableSet<Artifact> mandatoryInputs,
- Iterable<SkyKey> requestedSkyKeys)
+ Iterable<SkyKey> requestedSkyKeys,
+ ActionLookupData actionLookupDataForError)
throws ActionExecutionException, InterruptedException {
return accumulateInputs(
env,
@@ -1110,7 +1119,8 @@
requestedSkyKeys,
ActionInputMap::new,
CheckInputResults::new,
- /*allowValuesMissingEarlyReturn=*/ true);
+ /*allowValuesMissingEarlyReturn=*/ true,
+ actionLookupDataForError);
}
/**
@@ -1128,7 +1138,8 @@
NestedSet<Artifact> allInputs,
ImmutableSet<Artifact> mandatoryInputs,
Iterable<SkyKey> requestedSkyKeys,
- Collection<ActionInput> lostInputs)
+ Collection<ActionInput> lostInputs,
+ ActionLookupData actionLookupDataForError)
throws ActionExecutionException, InterruptedException {
// The rewinding strategy should be calculated with whatever information is available, instead
// of returning null if there are missing dependencies, so this uses false for
@@ -1147,7 +1158,8 @@
archivedArtifacts,
filesetsInsideRunfiles,
topLevelFilesets) -> actionInputMapSink,
- /*allowValuesMissingEarlyReturn=*/ false);
+ /*allowValuesMissingEarlyReturn=*/ false,
+ actionLookupDataForError);
}
/**
@@ -1169,7 +1181,8 @@
Iterable<SkyKey> requestedSkyKeys,
IntFunction<S> actionInputMapSinkFactory,
AccumulateInputResultsFactory<S, R> accumulateInputResultsFactory,
- boolean allowValuesMissingEarlyReturn)
+ boolean allowValuesMissingEarlyReturn,
+ ActionLookupData actionLookupDataForError)
throws ActionExecutionException, InterruptedException {
if (evalInputsAsNestedSet(allInputs)) {
@@ -1182,7 +1195,8 @@
requestedSkyKeys,
actionInputMapSinkFactory,
accumulateInputResultsFactory,
- allowValuesMissingEarlyReturn);
+ allowValuesMissingEarlyReturn,
+ actionLookupDataForError);
}
// Only populate input data if we have the input values, otherwise they'll just go unused.
// We still want to loop through the inputs to collect missing deps errors. During the
@@ -1241,7 +1255,10 @@
if (!input.isSourceArtifact()) {
bugReporter.sendBugReport(
new IllegalStateException(
- "Non-source artifact had SourceArtifactException" + input, e));
+ String.format(
+ "Non-source artifact had SourceArtifactException %s %s %s",
+ input.toDebugString(), actionLookupDataForError, action.prettyPrint()),
+ e));
}
if (mandatory) {
sourceArtifactErrorCauses.add(
@@ -1349,7 +1366,8 @@
Iterable<SkyKey> requestedSkyKeys,
IntFunction<S> actionInputMapSinkFactory,
AccumulateInputResultsFactory<S, R> accumulateInputResultsFactory,
- boolean allowValuesMissingEarlyReturn)
+ boolean allowValuesMissingEarlyReturn,
+ ActionLookupData actionLookupDataForError)
throws ActionExecutionException, InterruptedException {
ActionExecutionFunctionExceptionHandler actionExecutionFunctionExceptionHandler =
@@ -1401,8 +1419,8 @@
BugReport.sendBugReport(
new IllegalStateException(
String.format(
- "Null value for mandatory %s with no errors or values missing: %s",
- input, action)));
+ "Null value for mandatory %s with no errors or values missing: %s %s",
+ input.toDebugString(), actionLookupDataForError, action.prettyPrint())));
}
continue;
}
