From b042810bcd0ab084fe6a0eef67191228ca76832c Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 10 Oct 2024 12:10:10 -0700 Subject: [PATCH] Describe failed action sources in more detail. Specifically, some actions are registered by aspects, so mention the aspects when `--verbose_failures` is mentioned. PiperOrigin-RevId: 684530757 Change-Id: I7b74fcfccc92ce686fd9e078592d1ce31d799c41 --- .../lib/actions/ActionExecutionContext.java | 3 +-- .../build/lib/actions/ActionOwner.java | 23 +++++++++++++++++++ .../devtools/build/lib/actions/Spawn.java | 5 ++++ .../build/lib/util/CommandFailureUtils.java | 8 +++---- .../lib/util/DescribableExecutionUnit.java | 2 +- .../lib/util/CommandFailureUtilsTest.java | 10 ++++---- 6 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java index 02ecff77a1e9c0..030a725a8c15d1 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java @@ -24,7 +24,6 @@ import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue.RunfileSymlinksMode; import com.google.devtools.build.lib.bugreport.BugReporter; import com.google.devtools.build.lib.clock.Clock; -import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventKind; @@ -463,7 +462,7 @@ public void maybeReportSubcommand(Spawn spawn) { if (owner == null) { reason.append(spawn.getResourceOwner().prettyPrint()); } else { - reason.append(Label.print(owner.getLabel())); + reason.append(owner.getDescription()); reason.append(" ["); reason.append(spawn.getResourceOwner().prettyPrint()); reason.append(", configuration: "); diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java b/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java index 566be00bc0b6d5..21b2cd9f74ac7c 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java @@ -13,6 +13,8 @@ // limitations under the License. package com.google.devtools.build.lib.actions; +import static java.util.stream.Collectors.joining; + import com.google.auto.value.AutoValue; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; @@ -99,6 +101,27 @@ public static ActionOwner createDummy( execProperties); } + @Nullable + public String getDescription() { + Label label = getLabel(); + if (label == null) { + return null; + } + String targetDescription = "target " + label; + + ImmutableList aspectDescriptors = getAspectDescriptors(); + if (aspectDescriptors.isEmpty()) { + return targetDescription; + } + + String aspectNames = + aspectDescriptors.stream().map(AspectDescriptor::getDescription).collect(joining(", ")); + + return String.format( + "aspect%s [%s] on %s", + aspectDescriptors.size() >= 1 ? "s" : "", aspectNames, targetDescription); + } + /** * Returns the label for this {@link ActionOwner}, or null if the {@link #SYSTEM_ACTION_OWNER}. */ diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java index 5bec0638438f5e..08f85bd609fb1d 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java @@ -179,6 +179,11 @@ default String getConfigurationChecksum() { } @Override + @Nullable + default String getTargetDescription() { + return getResourceOwner().getOwner().getDescription(); + } + @Nullable default Label getTargetLabel() { return getResourceOwner().getOwner().getLabel(); diff --git a/src/main/java/com/google/devtools/build/lib/util/CommandFailureUtils.java b/src/main/java/com/google/devtools/build/lib/util/CommandFailureUtils.java index c5160070dad7af..d765380ddc6f23 100644 --- a/src/main/java/com/google/devtools/build/lib/util/CommandFailureUtils.java +++ b/src/main/java/com/google/devtools/build/lib/util/CommandFailureUtils.java @@ -151,7 +151,7 @@ static String describeCommandFailure( Map env, @Nullable String cwd, @Nullable String configurationChecksum, - @Nullable Label targetLabel, + @Nullable String targetDescription, @Nullable Label executionPlatformLabel) { String commandName = commandLineElements.iterator().next(); @@ -166,8 +166,8 @@ static String describeCommandFailure( output.append("error executing "); output.append(mnemonic); output.append(" command "); - if (targetLabel != null) { - output.append("(from target ").append(targetLabel).append(") "); + if (targetDescription != null) { + output.append("(from ").append(targetDescription).append(") "); } if (verbose) { output.append("\n "); @@ -194,7 +194,7 @@ public static String describeCommandFailure( command.getEnvironment(), cwd, command.getConfigurationChecksum(), - command.getTargetLabel(), + command.getTargetDescription(), command.getExecutionPlatformLabel()); } } diff --git a/src/main/java/com/google/devtools/build/lib/util/DescribableExecutionUnit.java b/src/main/java/com/google/devtools/build/lib/util/DescribableExecutionUnit.java index 6ccc9a3b468f60..c0aece5acaadc5 100644 --- a/src/main/java/com/google/devtools/build/lib/util/DescribableExecutionUnit.java +++ b/src/main/java/com/google/devtools/build/lib/util/DescribableExecutionUnit.java @@ -24,7 +24,7 @@ public interface DescribableExecutionUnit { @Nullable - default Label getTargetLabel() { + default String getTargetDescription() { return null; } diff --git a/src/test/java/com/google/devtools/build/lib/util/CommandFailureUtilsTest.java b/src/test/java/com/google/devtools/build/lib/util/CommandFailureUtilsTest.java index d4949df013118c..5c4f7eb5baba79 100644 --- a/src/test/java/com/google/devtools/build/lib/util/CommandFailureUtilsTest.java +++ b/src/test/java/com/google/devtools/build/lib/util/CommandFailureUtilsTest.java @@ -50,7 +50,7 @@ public void describeCommandFailure() throws Exception { env, cwd, "cfg12345", - target, + "target " + target, executionPlatform.label()); assertThat(message) .isEqualTo( @@ -79,7 +79,7 @@ public void describeCommandFailure_verbose() throws Exception { env, cwd, "cfg12345", - target, + "target " + target, executionPlatform.label()); assertThat(message) .isEqualTo( @@ -116,7 +116,7 @@ public void describeCommandFailure_longMessage() throws Exception { env, cwd, "cfg12345", - target, + "target " + target, executionPlatform.label()); assertThat(message) .isEqualTo( @@ -152,7 +152,7 @@ public void describeCommandFailure_longMessage_verbose() throws Exception { env, cwd, "cfg12345", - target, + "target " + target, executionPlatform.label()); assertThat(message) .isEqualTo( @@ -190,7 +190,7 @@ public void describeCommandFailure_singleSkippedArgument() throws Exception { env, cwd, "cfg12345", - target, + "target " + target, executionPlatform.label()); assertThat(message) .isEqualTo(