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 9bf3d134438712..30cb3a421962d4 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 @@ -32,6 +32,7 @@ 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; import com.google.devtools.build.lib.actions.ActionCompletionEvent; @@ -138,6 +139,8 @@ */ public final class ActionExecutionFunction implements SkyFunction { + private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); + private final ActionRewindStrategy actionRewindStrategy = new ActionRewindStrategy(); private final SkyframeActionExecutor skyframeActionExecutor; private final BlazeDirectories directories; @@ -1498,8 +1501,25 @@ private void handleActionExecutionExceptionFromSkykey(SkyKey key, ActionExecutio handleActionExecutionExceptionPerArtifact((Artifact) key, e); return; } - for (Artifact input : skyKeyToDerivedArtifactSetForExceptions.get().get(key)) { - handleActionExecutionExceptionPerArtifact(input, e); + Set associatedInputs = skyKeyToDerivedArtifactSetForExceptions.get().get(key); + if (associatedInputs.isEmpty()) { + // This can happen if an action prunes its inputs, e.g. the way StarlarkAction implements + // unused_inputs_list. An input may no longer be present in getInputs(), but its generating + // action could still be a Skyframe dependency because Skyframe eagerly adds a dep group to + // a dirty node if all prior dep groups are clean. If the pruned input is in error, it + // propagates during error bubbling, and we reach this point. + // TODO(lberki): Can inputs be immutable instead? + logger.atWarning().log( + "While handling errors for %s, encountered error from %s which is not associated with" + + " any inputs", + action.prettyPrint(), key); + if (firstActionExecutionException == null) { + firstActionExecutionException = e; + } + } else { + for (Artifact input : associatedInputs) { + handleActionExecutionExceptionPerArtifact(input, e); + } } } diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/UnusedInputsFailureIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/UnusedInputsFailureIntegrationTest.java index 12928697ab2da1..db2793fad1b934 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/UnusedInputsFailureIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/UnusedInputsFailureIntegrationTest.java @@ -105,6 +105,98 @@ public void incrementalFailureOnUnusedInput() throws Exception { } } + /** + * Regression test for b/185998331. + * + *

The action graph is: + * + *

+   *            top [consume.out] -> [top.out]
+   *                           |
+   *           consume [consume.sh, prune.out] -> [consume.out]
+   *                           |
+   *      prune [prune.sh, [bad.out, good.out]] -> [prune.out, unused_list]
+   *                   /                \
+   *     bad [bad.sh] -> [bad.out]     good [] -> [good.out]
+   * 
+ * + * where 'prune' reports 'bad' as an unused input. On the first build, 'consume' fails. On the + * second build, 'bad' fails. If the error is not handled correctly by 'prune', 'top' won't know + * that 'consume' is unavailable. + */ + @Test + public void incrementalFailureOnUnusedInput_downstreamInputNotReady() throws Exception { + write( + "foo/defs.bzl", + "def _example_rule_impl(ctx):", + " bad = ctx.actions.declare_file('bad.out')", + " ctx.actions.run(", + " outputs = [bad],", + " executable = ctx.executable.bad_sh,", + " arguments = [bad.path],", + " )", + "", + " good = ctx.actions.declare_file('good.out')", + " ctx.actions.run_shell(outputs = [good], command = 'touch %s' % good.path)", + "", + " unused_list = ctx.actions.declare_file('unused_list')", + " prune = ctx.actions.declare_file('prune.out')", + " ctx.actions.run(", + " outputs = [prune, unused_list],", + " inputs = [bad, good],", + " unused_inputs_list = unused_list,", + " executable = ctx.executable.prune_sh,", + " arguments = [prune.path, unused_list.path, bad.path],", + " )", + "", + " consume = ctx.actions.declare_file('consume.out')", + " ctx.actions.run(", + " outputs = [consume],", + " inputs = [prune],", + " executable = ctx.executable.consume_sh,", + " arguments = [consume.path],", + " )", + "", + " top = ctx.actions.declare_file('top.out')", + " ctx.actions.run_shell(", + " outputs = [top],", + " inputs = [consume],", + " command = 'touch %s' % top.path,", + " )", + " return DefaultInfo(files = depset([top]))", + "", + "example_rule = rule(", + " implementation = _example_rule_impl,", + " attrs = {", + " 'bad_sh': attr.label(", + " executable = True, allow_single_file = True, cfg = 'exec', default = 'bad.sh'", + " ),", + " 'prune_sh': attr.label(", + " executable = True, allow_single_file = True, cfg = 'exec', default = 'prune.sh'", + " ),", + " 'consume_sh': attr.label(", + " executable = True, allow_single_file = True, cfg = 'exec', default = 'consume.sh'", + " ),", + " },", + ")"); + write("foo/BUILD", "load(':defs.bzl', 'example_rule')", "example_rule(name = 'example')"); + write("foo/bad.sh", "#!/bin/bash", "touch $1").setExecutable(true); + write("foo/prune.sh", "#!/bin/bash", "touch $1 && echo $3 > $2").setExecutable(true); + write("foo/consume.sh", "#!/bin/bash", "exit 1").setExecutable(true); + assertThrows(BuildFailedException.class, () -> buildTarget("//foo:example")); + assertContainsError("Action foo/consume.out failed"); + + write("foo/bad.sh", "#!/bin/bash", "exit 1").setExecutable(true); + write("foo/consume.sh", "#!/bin/bash", "touch $@").setExecutable(true); + + if (keepGoing) { + buildTarget("//foo:example"); + } else { + assertThrows(BuildFailedException.class, () -> buildTarget("//foo:example")); + assertContainsError("Action foo/bad.out failed"); + } + } + @Test public void incrementalUnusedSymlinkCycle() throws Exception { RecordingBugReporter bugReporter = recordBugReportsAndReinitialize();