Skip to content

Commit

Permalink
Remove the "ugly debugging" added in unknown commit.
Browse files Browse the repository at this point in the history 
The debugging was added for b/185998331, which has since been fixed.

PiperOrigin-RevId: 597593216
Change-Id: I6905769f660ce5bb0fc42a17ec94207e4eb353b0
  • Loading branch information
@justinhorvitz @copybara-github
justinhorvitz authored and copybara-github committed Jan 11, 2024
1 parent 6da1495 commit f93ceb1
Showing 1 changed file with 30 additions and 99 deletions.
129 changes: 30 additions & 99 deletions src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import com.google.common.collect.Maps;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.SetMultimap;
import com.google.common.collect.Sets;
import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionCacheChecker.Token;
Expand Down Expand Up @@ -118,7 +117,6 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
import java.util.function.IntFunction;
import java.util.function.Predicate;
import java.util.function.Supplier;
Expand Down Expand Up @@ -302,8 +300,7 @@ private SkyValue computeInternal(ActionLookupData actionLookupData, Environment
if (previousExecution == null) {
// Do we actually need to find our metadata?
try {
checkedInputs =
checkInputs(env, action, inputDepsResult, allInputs, inputDepKeys, actionLookupData);
checkedInputs = checkInputs(env, action, inputDepsResult, allInputs, inputDepKeys);
} catch (ActionExecutionException e) {
throw new ActionExecutionFunctionException(e);
}
Expand Down Expand Up @@ -535,7 +532,7 @@ private SkyFunction.Reset handleLostInputs(
RewindPlan rewindPlan = null;
try {
ActionInputDepOwners inputDepOwners =
createAugmentedInputDepOwners(e, action, inputDepKeys, env, allInputs, actionLookupData);
createAugmentedInputDepOwners(e, action, inputDepKeys, env, allInputs);
rewindPlan =
actionRewindStrategy.getRewindPlan(
actionLookupData, action, failedActionDeps, e, inputDepOwners, env);
Expand Down Expand Up @@ -590,8 +587,7 @@ private ActionInputDepOwners createAugmentedInputDepOwners(
Action action,
ImmutableSet<SkyKey> inputDepKeys,
Environment env,
NestedSet<Artifact> allInputs,
ActionLookupData actionLookupDataForError)
NestedSet<Artifact> allInputs)
throws InterruptedException, UndoneInputsException {
Set<ActionInput> lostInputsAndOwnersSoFar = new HashSet<>();
ActionInputDepOwners owners = e.getOwners();
Expand All @@ -603,13 +599,7 @@ private ActionInputDepOwners createAugmentedInputDepOwners(
ActionInputDepOwnerMap inputDepOwners;
try {
inputDepOwners =
getInputDepOwners(
env,
action,
inputDepKeys,
allInputs,
lostInputsAndOwnersSoFar,
actionLookupDataForError);
getInputDepOwners(env, action, inputDepKeys, allInputs, 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
Expand Down Expand Up @@ -1131,8 +1121,7 @@ private CheckInputResults checkInputs(
Action action,
SkyframeLookupResult inputDepsResult,
NestedSet<Artifact> allInputs,
ImmutableSet<SkyKey> inputDepKeys,
ActionLookupData actionLookupDataForError)
ImmutableSet<SkyKey> inputDepKeys)
throws ActionExecutionException, InterruptedException, UndoneInputsException {
return accumulateInputs(
env,
Expand All @@ -1142,8 +1131,7 @@ private CheckInputResults checkInputs(
inputDepKeys,
sizeHint -> new ActionInputMap(bugReporter, sizeHint),
CheckInputResults::new,
/* returnEarlyIfValuesMissing= */ true,
actionLookupDataForError);
/* returnEarlyIfValuesMissing= */ true);
}

/**
Expand All @@ -1154,8 +1142,7 @@ private ActionInputDepOwnerMap getInputDepOwners(
Action action,
ImmutableSet<SkyKey> inputDepKeys,
NestedSet<Artifact> allInputs,
Collection<ActionInput> lostInputs,
ActionLookupData actionLookupDataForError)
Collection<ActionInput> lostInputs)
throws ActionExecutionException, InterruptedException, UndoneInputsException {
return accumulateInputs(
env,
Expand All @@ -1174,8 +1161,7 @@ private ActionInputDepOwnerMap getInputDepOwners(
// returnEarlyIfValuesMissing. Lost inputs coinciding with missing dependencies is
// possible during include scanning, see the test case
// generatedHeaderRequestedWhileDirty_coincidesWithLostInput.
/* returnEarlyIfValuesMissing= */ false,
actionLookupDataForError);
/* returnEarlyIfValuesMissing= */ false);
}

private static Predicate<Artifact> makeMandatoryInputPredicate(Action action) {
Expand Down Expand Up @@ -1231,8 +1217,7 @@ private <S extends ActionInputMapSink, R> R accumulateInputs(
ImmutableSet<SkyKey> inputDepKeys,
IntFunction<S> actionInputMapSinkFactory,
AccumulateInputResultsFactory<S, R> accumulateInputResultsFactory,
boolean returnEarlyIfValuesMissing,
ActionLookupData actionLookupDataForError)
boolean returnEarlyIfValuesMissing)
throws ActionExecutionException, InterruptedException, UndoneInputsException {
Predicate<Artifact> isMandatoryInput = makeMandatoryInputPredicate(action);
ActionExecutionFunctionExceptionHandler actionExecutionFunctionExceptionHandler = null;
Expand Down Expand Up @@ -1294,8 +1279,7 @@ private <S extends ActionInputMapSink, R> R accumulateInputs(
input,
inputDepKeys,
isMandatoryInput,
actionExecutionFunctionExceptionHandler,
actionLookupDataForError);
actionExecutionFunctionExceptionHandler);

if (value != null) {
ActionInputMapHelper.addToMap(
Expand Down Expand Up @@ -1336,8 +1320,7 @@ private <S extends ActionInputMapSink, R> R accumulateInputs(
input,
inputDepKeys,
isMandatoryInput,
actionExecutionFunctionExceptionHandler,
actionLookupDataForError);
actionExecutionFunctionExceptionHandler);
}

for (NestedSet<Artifact> nonLeaf : action.getSchedulingDependencies().getNonLeaves()) {
Expand All @@ -1349,8 +1332,7 @@ private <S extends ActionInputMapSink, R> R accumulateInputs(
input,
inputDepKeys,
isMandatoryInput,
actionExecutionFunctionExceptionHandler,
actionLookupDataForError);
actionExecutionFunctionExceptionHandler);
}
}
}
Expand Down Expand Up @@ -1378,67 +1360,31 @@ private SkyValue getAndCheckInputSkyValue(
Artifact input,
ImmutableSet<SkyKey> inputDepKeys,
Predicate<Artifact> isMandatoryInput,
ActionExecutionFunctionExceptionHandler actionExecutionFunctionExceptionHandler,
ActionLookupData actionLookupDataForError)
@Nullable ActionExecutionFunctionExceptionHandler actionExecutionFunctionExceptionHandler)
throws InterruptedException {
SkyValue value = lookupInput(input, inputDepKeys, env);
if (value == null) {
if (isMandatoryInput.test(input) && !skyframeActionExecutor.rewindingEnabled()) {
StringBuilder errorMessage = new StringBuilder();
ImmutableSet<Artifact> outputs = ImmutableSet.copyOf(action.getOutputs());
NestedSet<Artifact> nestedInputs = action.getInputs();
ImmutableSet<Artifact> inputs = nestedInputs.toSet();
if (action.discoversInputs()) {
errorMessage.append("\nAction discovers inputs");
} else {
errorMessage.append("\nAction does not discover inputs");
}
if (outputs.contains(input)) {
errorMessage.append("\nInput is an *output* of action");
}
if (inputs.contains(input)) {
errorMessage.append("\nInput is an input of action, bottom-up path:\n");
if (!findPathToKey(
nestedInputs,
input,
n -> {
ImmutableList<Artifact> artifacts = n.toList();
errorMessage
.append(" ")
.append(artifacts.size())
.append(", ")
.append(Iterables.limit(artifacts, 10))
.append('\n');
},
Sets.newHashSet(nestedInputs.toNode()))) {
errorMessage.append("Could not find input in action's NestedSet inputs");
}
} else {
errorMessage.append("\nInput not present in action's inputs");
}
throw new IllegalStateException(
String.format(
"Null value for mandatory %s with no errors or values missing: %s %s %s",
input.toDebugString(),
actionLookupDataForError,
action.prettyPrint(),
errorMessage));
}
// Undone mandatory inputs are only expected for generated artifacts when rewinding is
// enabled. Returning null allows the caller to use UndoneInputsException to recover.
checkState(
!isMandatoryInput.test(input)
|| (input.hasKnownGeneratingAction() && skyframeActionExecutor.rewindingEnabled()),
"Unexpected undone mandatory input: %s",
input);
return null;
}
if (value instanceof MissingArtifactValue) {
if (isMandatoryInput.test(input)) {
checkNotNull(
actionExecutionFunctionExceptionHandler,
"Missing artifact should have been caught already %s %s %s",
input,
value,
action)
.accumulateMissingFileArtifactValue(input, (MissingArtifactValue) value);
return null;
} else {
value = FileArtifactValue.MISSING_FILE_MARKER;
if (!isMandatoryInput.test(input)) {
return FileArtifactValue.MISSING_FILE_MARKER;
}
checkNotNull(
actionExecutionFunctionExceptionHandler,
"Missing artifact should have been caught already %s %s %s",
input,
value,
action)
.accumulateMissingFileArtifactValue(input, (MissingArtifactValue) value);
return null;
}
return value;
}
Expand Down Expand Up @@ -1469,21 +1415,6 @@ private SkyValue lookupInput(Artifact input, ImmutableSet<SkyKey> inputDepKeys,
return entry.toValue();
}

private static <T> boolean findPathToKey(
NestedSet<T> start, T target, Consumer<NestedSet<T>> receiver, Set<NestedSet.Node> seen) {
if (start.getLeaves().contains(target)) {
receiver.accept(start);
return true;
}
for (NestedSet<T> next : start.getNonLeaves()) {
if (seen.add(next.toNode()) && findPathToKey(next, target, receiver, seen)) {
receiver.accept(start);
return true;
}
}
return false;
}

static LabelCause createLabelCause(
Artifact input,
DetailedExitCode detailedExitCode,
Expand Down

0 comments on commit f93ceb1

Please sign in to comment.