Add detection of WORKSPACE cycles due to load statements

Cycle when looking for a repository name will now shows a user-understable error.

Fixes #877.

--
MOS_MIGRATED_REVID=117453646

1 files changed, 29 insertions, 3 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java
index 41f71d3082..80e190b6ac 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java
@@ -18,6 +18,7 @@ import com.google.common.base.Function;
 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.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.skyframe.CycleInfo;
@@ -35,17 +36,30 @@ public class SkylarkModuleCycleReporter implements CyclesReporter.SingleCycleRep
   private static final Predicate<SkyKey> IS_PACKAGE_SKY_KEY =
       SkyFunctions.isSkyFunction(SkyFunctions.PACKAGE);
 
+  private static final Predicate<SkyKey> IS_PACKAGE_LOOKUP =
+      SkyFunctions.isSkyFunction(SkyFunctions.PACKAGE_LOOKUP);
+
+  private static final Predicate<SkyKey> IS_WORKSPACE_FILE =
+      SkyFunctions.isSkyFunction(SkyFunctions.WORKSPACE_FILE);
+
+  private static final Predicate<SkyKey> IS_REPOSITORY_DIRECTORY =
+      SkyFunctions.isSkyFunction(SkyFunctions.REPOSITORY_DIRECTORY);
+
+  private static final Predicate<SkyKey> IS_AST_FILE_LOOKUP =
+      SkyFunctions.isSkyFunction(SkyFunctions.AST_FILE_LOOKUP);
+
   @Override
   public boolean maybeReportCycle(SkyKey topLevelKey, CycleInfo cycleInfo, boolean alreadyReported,
       EventHandler eventHandler) {
     ImmutableList<SkyKey> pathToCycle = cycleInfo.getPathToCycle();
+    ImmutableList<SkyKey> cycle = cycleInfo.getCycle();
     if (pathToCycle.isEmpty()) {
       return false;
     }
-    SkyKey lastPathElement = cycleInfo.getPathToCycle().get(pathToCycle.size() - 1);
+    SkyKey lastPathElement = pathToCycle.get(pathToCycle.size() - 1);
     if (alreadyReported) {
       return true;
-    } else if (Iterables.all(cycleInfo.getCycle(), IS_SKYLARK_MODULE_SKY_KEY)
+    } else if (Iterables.all(cycle, IS_SKYLARK_MODULE_SKY_KEY)
         // The last element of the path to the cycle has to be a PackageFunction.
         && IS_PACKAGE_SKY_KEY.apply(lastPathElement)) {
       StringBuilder cycleMessage =
@@ -63,11 +77,23 @@ public class SkylarkModuleCycleReporter implements CyclesReporter.SingleCycleRep
                   .importLabel.toString();
             }
           });
-
       // TODO(bazel-team): it would be nice to pass the Location of the load Statement in the
       // BUILD file.
       eventHandler.handle(Event.error(null, cycleMessage.toString()));
       return true;
+    } else if (Iterables.any(cycle, IS_PACKAGE_LOOKUP) && Iterables.any(cycle, IS_WORKSPACE_FILE)
+        && (IS_REPOSITORY_DIRECTORY.apply(lastPathElement)
+        || IS_PACKAGE_SKY_KEY.apply(lastPathElement))) {
+      // We have a cycle in the workspace file, report as such.
+      Label fileLabel =
+          (Label) Iterables.getLast(Iterables.filter(cycle, IS_AST_FILE_LOOKUP)).argument();
+      String repositoryName = fileLabel.getPackageIdentifier().getRepository().strippedName();
+      eventHandler.handle(Event.error(null,
+          "Failed to load Skylark extension '" + fileLabel.toString() + "'.\n"
+              + "It usually happens when the repository is not defined prior to being used.\n"
+              + "Maybe repository '" + repositoryName
+              + "' was defined later in your WORKSPACE file?"));
+      return true;
     }
     return false;
   }