Use the same method as LoadedPackageProvider.Bridge to look up targets.

This unfortunately requires injecting an EventHandler into the
getTargetForLabel call, which is not super nice. On the other hand, it's not
clear how this can be much better - looking up targets in the cycle reporter
doesn't lend itself to prettiness.

--
MOS_MIGRATED_REVID=103965373

5 files changed, 44 insertions, 27 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AbstractLabelCycleReporter.java b/src/main/java/com/google/devtools/build/lib/skyframe/AbstractLabelCycleReporter.java
index d87e787dd4..f1dff20d80 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AbstractLabelCycleReporter.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AbstractLabelCycleReporter.java
@@ -18,22 +18,27 @@ import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.concurrent.Uninterruptibles;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.packages.NoSuchPackageException;
+import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.NoSuchThingException;
 import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.pkgcache.LoadedPackageProvider;
+import com.google.devtools.build.lib.pkgcache.PackageProvider;
 import com.google.devtools.build.skyframe.CycleInfo;
 import com.google.devtools.build.skyframe.CyclesReporter;
 import com.google.devtools.build.skyframe.SkyKey;
 
+import java.util.concurrent.Callable;
+
 /** Reports cycles between skyframe values whose keys contains {@link Label}s. */
 abstract class AbstractLabelCycleReporter implements CyclesReporter.SingleCycleReporter {
 
-  private final LoadedPackageProvider loadedPackageProvider;
+  private final PackageProvider packageProvider;
 
-  AbstractLabelCycleReporter(LoadedPackageProvider loadedPackageProvider) {
-    this.loadedPackageProvider = loadedPackageProvider;
+  AbstractLabelCycleReporter(PackageProvider packageProvider) {
+    this.packageProvider = packageProvider;
   }
 
   /** Returns the String representation of the {@code SkyKey}. */
@@ -44,7 +49,8 @@ abstract class AbstractLabelCycleReporter implements CyclesReporter.SingleCycleR
 
   protected abstract boolean canReportCycle(SkyKey topLevelKey, CycleInfo cycleInfo);
 
-  protected String getAdditionalMessageAboutCycle(SkyKey topLevelKey, CycleInfo cycleInfo) {
+  protected String getAdditionalMessageAboutCycle(
+      EventHandler eventHandler, SkyKey topLevelKey, CycleInfo cycleInfo) {
     return "";
   }
 
@@ -58,7 +64,7 @@ abstract class AbstractLabelCycleReporter implements CyclesReporter.SingleCycleR
 
     if (alreadyReported) {
       Label label = getLabel(topLevelKey);
-      Target target = getTargetForLabel(label);
+      Target target = getTargetForLabel(eventHandler, label);
       eventHandler.handle(Event.error(target.getLocation(),
           "in " + target.getTargetKind() + " " + label +
               ": cycle in dependency graph: target depends on an already-reported cycle"));
@@ -78,10 +84,10 @@ abstract class AbstractLabelCycleReporter implements CyclesReporter.SingleCycleR
         }
       });
 
-      cycleMessage.append(getAdditionalMessageAboutCycle(topLevelKey, cycleInfo));
+      cycleMessage.append(getAdditionalMessageAboutCycle(eventHandler, topLevelKey, cycleInfo));
 
       Label label = getLabel(cycleValue);
-      Target target = getTargetForLabel(label);
+      Target target = getTargetForLabel(eventHandler, label);
       eventHandler.handle(Event.error(
           target.getLocation(),
           "in " + target.getTargetKind() + " " + label + ": " + cycleMessage));
@@ -117,14 +123,22 @@ abstract class AbstractLabelCycleReporter implements CyclesReporter.SingleCycleR
     return cycleValue;
   }
 
-  protected final Target getTargetForLabel(Label label) {
+  protected final Target getTargetForLabel(final EventHandler eventHandler, final Label label) {
     try {
-      return loadedPackageProvider.getLoadedTarget(label);
+      return Uninterruptibles.callUninterruptibly(new Callable<Target>() {
+        @Override
+        public Target call()
+            throws NoSuchPackageException, NoSuchTargetException, InterruptedException {
+          return packageProvider.getTarget(eventHandler, label);
+        }
+      });
     } catch (NoSuchThingException e) {
       // This method is used for getting the target from a label in a circular dependency.
       // If we have a cycle that means that we need to have accessed the target (to get its
       // dependencies). So all the labels in a dependency cycle need to exist.
       throw new IllegalStateException(e);
+    } catch (Exception e) {
+      throw new IllegalStateException(e);
     }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionArtifactCycleReporter.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionArtifactCycleReporter.java
index 07d776c3bc..91574fc16c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionArtifactCycleReporter.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionArtifactCycleReporter.java
@@ -20,7 +20,7 @@ import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.analysis.LabelAndConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.pkgcache.LoadedPackageProvider;
+import com.google.devtools.build.lib.pkgcache.PackageProvider;
 import com.google.devtools.build.lib.skyframe.ArtifactValue.OwnedArtifact;
 import com.google.devtools.build.skyframe.CycleInfo;
 import com.google.devtools.build.skyframe.SkyFunctionName;
@@ -36,8 +36,8 @@ public class ActionArtifactCycleReporter extends AbstractLabelCycleReporter {
       SkyFunctions.isSkyFunction(SkyFunctions.ACTION_EXECUTION),
       SkyFunctions.isSkyFunction(SkyFunctions.TARGET_COMPLETION));
 
-  ActionArtifactCycleReporter(LoadedPackageProvider loadedPackageProvider) {
-    super(loadedPackageProvider);
+  ActionArtifactCycleReporter(PackageProvider packageProvider) {
+    super(packageProvider);
   }
 
   @Override
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 9a77801348..c7a542f1ef 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
@@ -17,7 +17,8 @@ import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.pkgcache.LoadedPackageProvider;
+import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.pkgcache.PackageProvider;
 import com.google.devtools.build.skyframe.CycleInfo;
 import com.google.devtools.build.skyframe.SkyKey;
 
@@ -34,8 +35,8 @@ class ConfiguredTargetCycleReporter extends AbstractLabelCycleReporter {
   private static final Predicate<SkyKey> IS_CONFIGURED_TARGET_SKY_KEY =
       SkyFunctions.isSkyFunction(SkyFunctions.CONFIGURED_TARGET);
 
-  ConfiguredTargetCycleReporter(LoadedPackageProvider loadedPackageProvider) {
-    super(loadedPackageProvider);
+  ConfiguredTargetCycleReporter(PackageProvider packageProvider) {
+    super(packageProvider);
   }
 
   @Override
@@ -45,7 +46,8 @@ class ConfiguredTargetCycleReporter extends AbstractLabelCycleReporter {
   }
 
   @Override
-  protected String getAdditionalMessageAboutCycle(SkyKey topLevelKey, CycleInfo cycleInfo) {
+  protected String getAdditionalMessageAboutCycle(
+      EventHandler eventHandler, SkyKey topLevelKey, CycleInfo cycleInfo) {
     return "\nThis cycle occurred because of a configuration option";
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 7cb524a8b9..38748a9f6e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -1605,12 +1605,11 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   }
 
   private CyclesReporter createCyclesReporter() {
-    LoadedPackageProvider loadedPackageProvider = getLoadedPackageProvider();
     return new CyclesReporter(
-        new TransitiveTargetCycleReporter(loadedPackageProvider),
-        new ActionArtifactCycleReporter(loadedPackageProvider),
+        new TransitiveTargetCycleReporter(packageManager),
+        new ActionArtifactCycleReporter(packageManager),
         new SkylarkModuleCycleReporter(),
-        new ConfiguredTargetCycleReporter(loadedPackageProvider));
+        new ConfiguredTargetCycleReporter(packageManager));
   }
 
   CyclesReporter getCyclesReporter() {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetCycleReporter.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetCycleReporter.java
index e3aba5768c..d2803a28e5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetCycleReporter.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetCycleReporter.java
@@ -18,9 +18,10 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.PackageGroup;
 import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.pkgcache.LoadedPackageProvider;
+import com.google.devtools.build.lib.pkgcache.PackageProvider;
 import com.google.devtools.build.skyframe.CycleInfo;
 import com.google.devtools.build.skyframe.SkyKey;
 
@@ -35,8 +36,8 @@ class TransitiveTargetCycleReporter extends AbstractLabelCycleReporter {
   private static final Predicate<SkyKey> IS_TRANSITIVE_TARGET_SKY_KEY =
       SkyFunctions.isSkyFunction(SkyFunctions.TRANSITIVE_TARGET);
 
-  TransitiveTargetCycleReporter(LoadedPackageProvider loadedPackageProvider) {
-    super(loadedPackageProvider);
+  TransitiveTargetCycleReporter(PackageProvider packageProvider) {
+    super(packageProvider);
   }
 
   @Override
@@ -57,8 +58,9 @@ class TransitiveTargetCycleReporter extends AbstractLabelCycleReporter {
   }
 
   @Override
-  protected String getAdditionalMessageAboutCycle(SkyKey topLevelKey, CycleInfo cycleInfo) {
-    Target currentTarget = getTargetForLabel(getLabel(topLevelKey));
+  protected String getAdditionalMessageAboutCycle(
+      EventHandler eventHandler, SkyKey topLevelKey, CycleInfo cycleInfo) {
+    Target currentTarget = getTargetForLabel(eventHandler, getLabel(topLevelKey));
     List<SkyKey> keys = Lists.newArrayList();
     if (!cycleInfo.getPathToCycle().isEmpty()) {
       keys.add(topLevelKey);
@@ -70,7 +72,7 @@ class TransitiveTargetCycleReporter extends AbstractLabelCycleReporter {
     keys.add(cycleInfo.getCycle().get(0));
     for (SkyKey nextKey : keys) {
       Label nextLabel = getLabel(nextKey);
-      Target nextTarget = getTargetForLabel(nextLabel);
+      Target nextTarget = getTargetForLabel(eventHandler, nextLabel);
       // This is inefficient but it's no big deal since we only do this when there's a cycle.
       if (currentTarget.getVisibility().getDependencyLabels().contains(nextLabel)
           && !nextTarget.getTargetKind().equals(PackageGroup.targetKind())) {