Skip to content

Commit

Permalink
Make attempting to rewind a source artifact a hard crash.
Browse files Browse the repository at this point in the history
This simplifies adding support for rewinding of lost top-level outputs, for which there is no failed `Action`.

Given that we never expect this to happen, a less detailed error message is acceptable.

PiperOrigin-RevId: 604494510
Change-Id: I7088218b12ccee71ee1485a9cf491679a15840c1
  • Loading branch information
justinhorvitz authored and copybara-github committed Feb 6, 2024
1 parent d7b9f8e commit 8ff9957
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,7 @@ public RewindPlan getRewindPlan(
.addAll(failedActionDeps)
.addAll(Artifact.keys(nestedSetKeys.keySet()))
.build(),
failedAction,
lostInputsException);
failedAction);

for (DerivedArtifact lostArtifact : lostArtifacts) {
SkyKey artifactKey = Artifact.key(lostArtifact);
Expand Down Expand Up @@ -289,17 +288,14 @@ private Set<DerivedArtifact> getLostInputOwningDirectDeps(
ImmutableList<ActionInput> lostInputs,
ActionInputDepOwners inputDepOwners,
Set<SkyKey> failedActionDeps,
Action failedAction,
LostInputsActionExecutionException lostInputsException)
throws ActionExecutionException {

Action failedAction) {
Set<DerivedArtifact> lostInputOwningDirectDeps = new HashSet<>();
for (ActionInput lostInput : lostInputs) {
boolean foundLostInputDepOwner = false;

Collection<Artifact> owners = inputDepOwners.getDepOwners(lostInput);
for (Artifact owner : owners) {
checkDerived(/*lostInputQualifier=*/ " owner", owner, failedAction, lostInputsException);
checkDerived(owner);

// 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
Expand All @@ -309,11 +305,7 @@ private Set<DerivedArtifact> getLostInputOwningDirectDeps(

Collection<Artifact> transitiveOwners = inputDepOwners.getDepOwners(owner);
for (Artifact transitiveOwner : transitiveOwners) {
checkDerived(
/*lostInputQualifier=*/ " transitive owner",
transitiveOwner,
failedAction,
lostInputsException);
checkDerived(transitiveOwner);

if (failedActionDeps.contains(Artifact.key(transitiveOwner))) {
// The lost input is included in an aggregation artifact (e.g. a tree artifact or
Expand All @@ -334,8 +326,7 @@ private Set<DerivedArtifact> getLostInputOwningDirectDeps(

if (lostInput instanceof Artifact
&& failedActionDeps.contains(Artifact.key((Artifact) lostInput))) {
checkDerived(
/* lostInputQualifier= */ "", (Artifact) lostInput, failedAction, lostInputsException);
checkDerived((Artifact) lostInput);

lostInputOwningDirectDeps.add((DerivedArtifact) lostInput);
foundLostInputDepOwner = true;
Expand All @@ -357,23 +348,8 @@ private Set<DerivedArtifact> getLostInputOwningDirectDeps(
return lostInputOwningDirectDeps;
}

private 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.
String message =
String.format(
"Unexpected source artifact as lost input%s: %s %s",
lostInputQualifier, expectedDerived, failedAction);
bugReporter.sendBugReport(new IllegalStateException(message));
throw createActionExecutionException(
lostInputsException, failedAction, message, Code.LOST_INPUT_IS_SOURCE);
private static void checkDerived(Artifact artifact) {
checkState(!artifact.isSourceArtifact(), "Unexpected source artifact: %s", artifact);
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/main/protobuf/failure_details.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1035,8 +1035,10 @@ message ActionRewinding {
enum Code {
ACTION_REWINDING_UNKNOWN = 0 [(metadata) = { exit_code: 37 }];
LOST_INPUT_TOO_MANY_TIMES = 1 [(metadata) = { exit_code: 1 }];
LOST_INPUT_IS_SOURCE = 2 [(metadata) = { exit_code: 1 }];
REWIND_LOST_INPUTS_PREREQ_UNMET = 3 [(metadata) = { exit_code: 2 }];
// Deprecated: attempting to rewind a source artifact is now a hard crash.
DEPRECATED_LOST_INPUT_IS_SOURCE = 2
[(metadata) = { exit_code: 1 }, deprecated = true];
}

Code code = 1;
Expand Down

0 comments on commit 8ff9957

Please sign in to comment.