Skip to content

Commit f93ceb1

Browse files
justinhorvitzcopybara-github
authored andcommitted
Remove the "ugly debugging" added in unknown commit.
The debugging was added for b/185998331, which has since been fixed. PiperOrigin-RevId: 597593216 Change-Id: I6905769f660ce5bb0fc42a17ec94207e4eb353b0
1 parent 6da1495 commit f93ceb1

File tree

1 file changed

+30
-99
lines changed

1 file changed

+30
-99
lines changed

src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java

Lines changed: 30 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import com.google.common.collect.Maps;
3333
import com.google.common.collect.MultimapBuilder;
3434
import com.google.common.collect.SetMultimap;
35-
import com.google.common.collect.Sets;
3635
import com.google.common.flogger.GoogleLogger;
3736
import com.google.devtools.build.lib.actions.Action;
3837
import com.google.devtools.build.lib.actions.ActionCacheChecker.Token;
@@ -118,7 +117,6 @@
118117
import java.util.Map;
119118
import java.util.Set;
120119
import java.util.concurrent.atomic.AtomicBoolean;
121-
import java.util.function.Consumer;
122120
import java.util.function.IntFunction;
123121
import java.util.function.Predicate;
124122
import java.util.function.Supplier;
@@ -302,8 +300,7 @@ private SkyValue computeInternal(ActionLookupData actionLookupData, Environment
302300
if (previousExecution == null) {
303301
// Do we actually need to find our metadata?
304302
try {
305-
checkedInputs =
306-
checkInputs(env, action, inputDepsResult, allInputs, inputDepKeys, actionLookupData);
303+
checkedInputs = checkInputs(env, action, inputDepsResult, allInputs, inputDepKeys);
307304
} catch (ActionExecutionException e) {
308305
throw new ActionExecutionFunctionException(e);
309306
}
@@ -535,7 +532,7 @@ private SkyFunction.Reset handleLostInputs(
535532
RewindPlan rewindPlan = null;
536533
try {
537534
ActionInputDepOwners inputDepOwners =
538-
createAugmentedInputDepOwners(e, action, inputDepKeys, env, allInputs, actionLookupData);
535+
createAugmentedInputDepOwners(e, action, inputDepKeys, env, allInputs);
539536
rewindPlan =
540537
actionRewindStrategy.getRewindPlan(
541538
actionLookupData, action, failedActionDeps, e, inputDepOwners, env);
@@ -590,8 +587,7 @@ private ActionInputDepOwners createAugmentedInputDepOwners(
590587
Action action,
591588
ImmutableSet<SkyKey> inputDepKeys,
592589
Environment env,
593-
NestedSet<Artifact> allInputs,
594-
ActionLookupData actionLookupDataForError)
590+
NestedSet<Artifact> allInputs)
595591
throws InterruptedException, UndoneInputsException {
596592
Set<ActionInput> lostInputsAndOwnersSoFar = new HashSet<>();
597593
ActionInputDepOwners owners = e.getOwners();
@@ -603,13 +599,7 @@ private ActionInputDepOwners createAugmentedInputDepOwners(
603599
ActionInputDepOwnerMap inputDepOwners;
604600
try {
605601
inputDepOwners =
606-
getInputDepOwners(
607-
env,
608-
action,
609-
inputDepKeys,
610-
allInputs,
611-
lostInputsAndOwnersSoFar,
612-
actionLookupDataForError);
602+
getInputDepOwners(env, action, inputDepKeys, allInputs, lostInputsAndOwnersSoFar);
613603
} catch (ActionExecutionException unexpected) {
614604
// getInputDepOwners should not be able to throw, because it does the same work as
615605
// checkInputs, so if getInputDepOwners throws then checkInputs should have thrown, and if
@@ -1131,8 +1121,7 @@ private CheckInputResults checkInputs(
11311121
Action action,
11321122
SkyframeLookupResult inputDepsResult,
11331123
NestedSet<Artifact> allInputs,
1134-
ImmutableSet<SkyKey> inputDepKeys,
1135-
ActionLookupData actionLookupDataForError)
1124+
ImmutableSet<SkyKey> inputDepKeys)
11361125
throws ActionExecutionException, InterruptedException, UndoneInputsException {
11371126
return accumulateInputs(
11381127
env,
@@ -1142,8 +1131,7 @@ private CheckInputResults checkInputs(
11421131
inputDepKeys,
11431132
sizeHint -> new ActionInputMap(bugReporter, sizeHint),
11441133
CheckInputResults::new,
1145-
/* returnEarlyIfValuesMissing= */ true,
1146-
actionLookupDataForError);
1134+
/* returnEarlyIfValuesMissing= */ true);
11471135
}
11481136

11491137
/**
@@ -1154,8 +1142,7 @@ private ActionInputDepOwnerMap getInputDepOwners(
11541142
Action action,
11551143
ImmutableSet<SkyKey> inputDepKeys,
11561144
NestedSet<Artifact> allInputs,
1157-
Collection<ActionInput> lostInputs,
1158-
ActionLookupData actionLookupDataForError)
1145+
Collection<ActionInput> lostInputs)
11591146
throws ActionExecutionException, InterruptedException, UndoneInputsException {
11601147
return accumulateInputs(
11611148
env,
@@ -1174,8 +1161,7 @@ private ActionInputDepOwnerMap getInputDepOwners(
11741161
// returnEarlyIfValuesMissing. Lost inputs coinciding with missing dependencies is
11751162
// possible during include scanning, see the test case
11761163
// generatedHeaderRequestedWhileDirty_coincidesWithLostInput.
1177-
/* returnEarlyIfValuesMissing= */ false,
1178-
actionLookupDataForError);
1164+
/* returnEarlyIfValuesMissing= */ false);
11791165
}
11801166

11811167
private static Predicate<Artifact> makeMandatoryInputPredicate(Action action) {
@@ -1231,8 +1217,7 @@ private <S extends ActionInputMapSink, R> R accumulateInputs(
12311217
ImmutableSet<SkyKey> inputDepKeys,
12321218
IntFunction<S> actionInputMapSinkFactory,
12331219
AccumulateInputResultsFactory<S, R> accumulateInputResultsFactory,
1234-
boolean returnEarlyIfValuesMissing,
1235-
ActionLookupData actionLookupDataForError)
1220+
boolean returnEarlyIfValuesMissing)
12361221
throws ActionExecutionException, InterruptedException, UndoneInputsException {
12371222
Predicate<Artifact> isMandatoryInput = makeMandatoryInputPredicate(action);
12381223
ActionExecutionFunctionExceptionHandler actionExecutionFunctionExceptionHandler = null;
@@ -1294,8 +1279,7 @@ private <S extends ActionInputMapSink, R> R accumulateInputs(
12941279
input,
12951280
inputDepKeys,
12961281
isMandatoryInput,
1297-
actionExecutionFunctionExceptionHandler,
1298-
actionLookupDataForError);
1282+
actionExecutionFunctionExceptionHandler);
12991283

13001284
if (value != null) {
13011285
ActionInputMapHelper.addToMap(
@@ -1336,8 +1320,7 @@ private <S extends ActionInputMapSink, R> R accumulateInputs(
13361320
input,
13371321
inputDepKeys,
13381322
isMandatoryInput,
1339-
actionExecutionFunctionExceptionHandler,
1340-
actionLookupDataForError);
1323+
actionExecutionFunctionExceptionHandler);
13411324
}
13421325

13431326
for (NestedSet<Artifact> nonLeaf : action.getSchedulingDependencies().getNonLeaves()) {
@@ -1349,8 +1332,7 @@ private <S extends ActionInputMapSink, R> R accumulateInputs(
13491332
input,
13501333
inputDepKeys,
13511334
isMandatoryInput,
1352-
actionExecutionFunctionExceptionHandler,
1353-
actionLookupDataForError);
1335+
actionExecutionFunctionExceptionHandler);
13541336
}
13551337
}
13561338
}
@@ -1378,67 +1360,31 @@ private SkyValue getAndCheckInputSkyValue(
13781360
Artifact input,
13791361
ImmutableSet<SkyKey> inputDepKeys,
13801362
Predicate<Artifact> isMandatoryInput,
1381-
ActionExecutionFunctionExceptionHandler actionExecutionFunctionExceptionHandler,
1382-
ActionLookupData actionLookupDataForError)
1363+
@Nullable ActionExecutionFunctionExceptionHandler actionExecutionFunctionExceptionHandler)
13831364
throws InterruptedException {
13841365
SkyValue value = lookupInput(input, inputDepKeys, env);
13851366
if (value == null) {
1386-
if (isMandatoryInput.test(input) && !skyframeActionExecutor.rewindingEnabled()) {
1387-
StringBuilder errorMessage = new StringBuilder();
1388-
ImmutableSet<Artifact> outputs = ImmutableSet.copyOf(action.getOutputs());
1389-
NestedSet<Artifact> nestedInputs = action.getInputs();
1390-
ImmutableSet<Artifact> inputs = nestedInputs.toSet();
1391-
if (action.discoversInputs()) {
1392-
errorMessage.append("\nAction discovers inputs");
1393-
} else {
1394-
errorMessage.append("\nAction does not discover inputs");
1395-
}
1396-
if (outputs.contains(input)) {
1397-
errorMessage.append("\nInput is an *output* of action");
1398-
}
1399-
if (inputs.contains(input)) {
1400-
errorMessage.append("\nInput is an input of action, bottom-up path:\n");
1401-
if (!findPathToKey(
1402-
nestedInputs,
1403-
input,
1404-
n -> {
1405-
ImmutableList<Artifact> artifacts = n.toList();
1406-
errorMessage
1407-
.append(" ")
1408-
.append(artifacts.size())
1409-
.append(", ")
1410-
.append(Iterables.limit(artifacts, 10))
1411-
.append('\n');
1412-
},
1413-
Sets.newHashSet(nestedInputs.toNode()))) {
1414-
errorMessage.append("Could not find input in action's NestedSet inputs");
1415-
}
1416-
} else {
1417-
errorMessage.append("\nInput not present in action's inputs");
1418-
}
1419-
throw new IllegalStateException(
1420-
String.format(
1421-
"Null value for mandatory %s with no errors or values missing: %s %s %s",
1422-
input.toDebugString(),
1423-
actionLookupDataForError,
1424-
action.prettyPrint(),
1425-
errorMessage));
1426-
}
1367+
// Undone mandatory inputs are only expected for generated artifacts when rewinding is
1368+
// enabled. Returning null allows the caller to use UndoneInputsException to recover.
1369+
checkState(
1370+
!isMandatoryInput.test(input)
1371+
|| (input.hasKnownGeneratingAction() && skyframeActionExecutor.rewindingEnabled()),
1372+
"Unexpected undone mandatory input: %s",
1373+
input);
14271374
return null;
14281375
}
14291376
if (value instanceof MissingArtifactValue) {
1430-
if (isMandatoryInput.test(input)) {
1431-
checkNotNull(
1432-
actionExecutionFunctionExceptionHandler,
1433-
"Missing artifact should have been caught already %s %s %s",
1434-
input,
1435-
value,
1436-
action)
1437-
.accumulateMissingFileArtifactValue(input, (MissingArtifactValue) value);
1438-
return null;
1439-
} else {
1440-
value = FileArtifactValue.MISSING_FILE_MARKER;
1377+
if (!isMandatoryInput.test(input)) {
1378+
return FileArtifactValue.MISSING_FILE_MARKER;
14411379
}
1380+
checkNotNull(
1381+
actionExecutionFunctionExceptionHandler,
1382+
"Missing artifact should have been caught already %s %s %s",
1383+
input,
1384+
value,
1385+
action)
1386+
.accumulateMissingFileArtifactValue(input, (MissingArtifactValue) value);
1387+
return null;
14421388
}
14431389
return value;
14441390
}
@@ -1469,21 +1415,6 @@ private SkyValue lookupInput(Artifact input, ImmutableSet<SkyKey> inputDepKeys,
14691415
return entry.toValue();
14701416
}
14711417

1472-
private static <T> boolean findPathToKey(
1473-
NestedSet<T> start, T target, Consumer<NestedSet<T>> receiver, Set<NestedSet.Node> seen) {
1474-
if (start.getLeaves().contains(target)) {
1475-
receiver.accept(start);
1476-
return true;
1477-
}
1478-
for (NestedSet<T> next : start.getNonLeaves()) {
1479-
if (seen.add(next.toNode()) && findPathToKey(next, target, receiver, seen)) {
1480-
receiver.accept(start);
1481-
return true;
1482-
}
1483-
}
1484-
return false;
1485-
}
1486-
14871418
static LabelCause createLabelCause(
14881419
Artifact input,
14891420
DetailedExitCode detailedExitCode,

0 commit comments

Comments
 (0)