Fix crash when a command-line aspect depends on a cycle.
PiperOrigin-RevId: 231246040
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
index 320a8a2..eb525be 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -45,6 +45,9 @@
*/
public abstract static class AspectValueKey extends ActionLookupKey {
public abstract String getDescription();
+
+ @Override
+ public abstract Label getLabel();
}
/** A base class for a key representing an aspect applied to a particular target. */
@@ -313,11 +316,11 @@
return SkyFunctions.LOAD_SKYLARK_ASPECT;
}
- public String getSkylarkValueName() {
+ String getSkylarkValueName() {
return skylarkValueName;
}
- public SkylarkImport getSkylarkImport() {
+ SkylarkImport getSkylarkImport() {
return skylarkImport;
}
@@ -326,6 +329,11 @@
}
@Override
+ public Label getLabel() {
+ return targetLabel;
+ }
+
+ @Override
public String getDescription() {
// Skylark aspects are referred to on command line with <file>%<value ame>
return String.format("%s%%%s of %s", skylarkImport.getImportString(),
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporter.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporter.java
index 2baeaca..fe73b63 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporter.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetCycleReporter.java
@@ -16,9 +16,11 @@
import static com.google.devtools.build.lib.skyframe.SkyFunctions.TRANSITIVE_TARGET;
import com.google.common.base.Function;
+import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.pkgcache.PackageProvider;
@@ -38,8 +40,9 @@
private static final Predicate<SkyKey> IS_CONFIGURED_TARGET_SKY_KEY =
Predicates.or(
- SkyFunctions.isSkyFunction(SkyFunctions.CONFIGURED_TARGET),
- SkyFunctions.isSkyFunction(SkyFunctions.ASPECT));
+ SkyFunctions.isSkyFunction(SkyFunctions.CONFIGURED_TARGET),
+ SkyFunctions.isSkyFunction(SkyFunctions.ASPECT),
+ SkyFunctions.isSkyFunction(SkyFunctions.LOAD_SKYLARK_ASPECT));
private static final Predicate<SkyKey> IS_TRANSITIVE_TARGET_SKY_KEY =
SkyFunctions.isSkyFunction(TRANSITIVE_TARGET);
@@ -97,6 +100,8 @@
return ((ConfiguredTargetKey) key.argument()).prettyPrint();
} else if (SkyFunctions.isSkyFunction(SkyFunctions.ASPECT).apply(key)) {
return ((AspectKey) key.argument()).prettyPrint();
+ } else if (key instanceof AspectValue.AspectValueKey) {
+ return ((AspectValue.AspectValueKey) key).getDescription();
} else {
return getLabel(key).toString();
}
@@ -104,14 +109,13 @@
@Override
public Label getLabel(SkyKey key) {
- if (SkyFunctions.isSkyFunction(SkyFunctions.CONFIGURED_TARGET).apply(key)) {
- return ((ConfiguredTargetKey) key.argument()).getLabel();
- } else if (SkyFunctions.isSkyFunction(SkyFunctions.ASPECT).apply(key)) {
- return ((AspectKey) key.argument()).getLabel();
+ if (key instanceof ActionLookupValue.ActionLookupKey) {
+ return Preconditions.checkNotNull(
+ ((ActionLookupValue.ActionLookupKey) key.argument()).getLabel(), key);
} else if (SkyFunctions.isSkyFunction(TRANSITIVE_TARGET).apply(key)) {
return ((TransitiveTargetKey) key).getLabel();
} else {
- throw new UnsupportedOperationException();
+ throw new UnsupportedOperationException(key.toString());
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java
index d89593c..38c361b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
@@ -76,7 +77,8 @@
@Override
public Target getTarget(ExtendedEventHandler eventHandler, Label label)
throws NoSuchPackageException, NoSuchTargetException, InterruptedException {
- return getPackage(eventHandler, label.getPackageIdentifier()).getTarget(label.getName());
+ return Preconditions.checkNotNull(getPackage(eventHandler, label.getPackageIdentifier()), label)
+ .getTarget(label.getName());
}
@Override
diff --git a/src/main/java/com/google/devtools/build/skyframe/CyclesReporter.java b/src/main/java/com/google/devtools/build/skyframe/CyclesReporter.java
index 39c5fec..301774d 100644
--- a/src/main/java/com/google/devtools/build/skyframe/CyclesReporter.java
+++ b/src/main/java/com/google/devtools/build/skyframe/CyclesReporter.java
@@ -69,36 +69,41 @@
*/
public void reportCycles(
Iterable<CycleInfo> cycles, SkyKey topLevelKey, ExtendedEventHandler eventHandler) {
- Preconditions.checkNotNull(eventHandler);
+ Preconditions.checkNotNull(eventHandler, "topLevelKey: %s, Cycles %s", topLevelKey, cycles);
for (CycleInfo cycleInfo : cycles) {
- boolean alreadyReported = false;
- if (!cycleDeduper.seen(cycleInfo.getCycle())) {
- alreadyReported = true;
- }
- boolean successfullyReported = false;
- for (SingleCycleReporter cycleReporter : cycleReporters) {
- if (cycleReporter.maybeReportCycle(topLevelKey, cycleInfo, alreadyReported, eventHandler)) {
- successfullyReported = true;
- break;
- }
- }
- Preconditions.checkState(successfullyReported,
- printArbitraryCycle(topLevelKey, cycleInfo, alreadyReported));
+ maybeReportCycle(cycleInfo, topLevelKey, eventHandler);
}
}
- private String printArbitraryCycle(SkyKey topLevelKey, CycleInfo cycleInfo,
- boolean alreadyReported) {
- StringBuilder cycleMessage = new StringBuilder()
- .append("topLevelKey: " + topLevelKey + "\n")
- .append("alreadyReported: " + alreadyReported + "\n")
- .append("path to cycle:\n");
+ private void maybeReportCycle(
+ CycleInfo cycleInfo, SkyKey topLevelKey, ExtendedEventHandler eventHandler) {
+ boolean alreadyReported = !cycleDeduper.seen(cycleInfo.getCycle());
+ for (SingleCycleReporter cycleReporter : cycleReporters) {
+ if (cycleReporter.maybeReportCycle(topLevelKey, cycleInfo, alreadyReported, eventHandler)) {
+ return;
+ }
+ }
+ throw new IllegalStateException(
+ printArbitraryCycle(topLevelKey, cycleInfo, alreadyReported) + "\n" + cycleReporters);
+ }
+
+ private static String printArbitraryCycle(
+ SkyKey topLevelKey, CycleInfo cycleInfo, boolean alreadyReported) {
+ StringBuilder cycleMessage =
+ new StringBuilder()
+ .append("topLevelKey: ")
+ .append(topLevelKey)
+ .append("\n")
+ .append("alreadyReported: ")
+ .append(alreadyReported)
+ .append("\n")
+ .append("path to cycle:\n");
for (SkyKey skyKey : cycleInfo.getPathToCycle()) {
- cycleMessage.append(skyKey + "\n");
+ cycleMessage.append(skyKey).append("\n");
}
cycleMessage.append("cycle:\n");
for (SkyKey skyKey : cycleInfo.getCycle()) {
- cycleMessage.append(skyKey + "\n");
+ cycleMessage.append(skyKey).append("\n");
}
return cycleMessage.toString();
}
