Make RedirectChaser behave properly if a referenced target is not found.

--
MOS_MIGRATED_REVID=128682147

2 files changed, 14 insertions, 29 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RedirectChaser.java b/src/main/java/com/google/devtools/build/lib/analysis/RedirectChaser.java
index c0bcfbad5e..a908ada500 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RedirectChaser.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RedirectChaser.java
@@ -17,19 +17,15 @@ package com.google.devtools.build.lib.analysis;
 import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.packages.AbstractAttributeMapper;
 import com.google.devtools.build.lib.packages.BuildType;
-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.Rule;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.syntax.Type;
-
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
-
 import javax.annotation.Nullable;
 
 /**
@@ -76,6 +72,7 @@ public final class RedirectChaser {
   @Nullable
   public static Label followRedirects(ConfigurationEnvironment env, Label label, String name)
       throws InvalidConfigurationException {
+    Label oldLabel = null;
     Set<Label> visitedLabels = new HashSet<>();
     visitedLabels.add(label);
     try {
@@ -93,18 +90,18 @@ public final class RedirectChaser {
         }
 
         newLabel = label.resolveRepositoryRelative(newLabel);
+        oldLabel = label;
         label = newLabel;
         if (!visitedLabels.add(label)) {
           throw new InvalidConfigurationException("The " + name + " points to a filegroup which "
               + "recursively includes itself. The label " + label + " is part of the loop");
         }
       }
-    } catch (NoSuchPackageException e) {
-      env.getEventHandler().handle(Event.error(e.getMessage()));
-      throw new InvalidConfigurationException(e.getMessage(), e);
-    } catch (NoSuchTargetException e) {
-      // TODO(ulfjack): Consider throwing an exception here instead of returning silently.
-      return label;
+    } catch (NoSuchThingException e) {
+      String prefix = oldLabel == null
+          ? ""
+          : "in target '" + oldLabel + "': ";
+      throw new InvalidConfigurationException(prefix + e.getMessage(), e);
     }
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
index f48c02cee5..3efe26b16d 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -58,16 +58,14 @@ import com.google.devtools.build.skyframe.NotifyingHelper.Listener;
 import com.google.devtools.build.skyframe.NotifyingHelper.Order;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.TrackingAwaiter;
-
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
 import java.util.Collection;
 import java.util.LinkedHashSet;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.regex.Pattern;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
 
 /**
  * Tests for the {@link BuildView}.
@@ -1056,8 +1054,7 @@ public class BuildViewTest extends BuildViewTestBase {
       update(defaultFlags().with(Flag.KEEP_GOING));
       fail();
     } catch (LoadingFailedException | InvalidConfigurationException e) {
-      assertContainsEvent(
-          "no such package 'third_party/crosstool/v2': BUILD file not found on package path");
+      assertThat(e.getMessage()).contains("third_party/crosstool/v2");
     }
   }
 
@@ -1090,15 +1087,7 @@ public class BuildViewTest extends BuildViewTestBase {
       update(defaultFlags().with(Flag.KEEP_GOING));
       fail();
     } catch (LoadingFailedException | InvalidConfigurationException e) {
-      if (getAnalysisMock().isThisBazel()) {
-        // TODO(ulfjack): Bazel ignores the --cpu setting and just uses "default" instead. This
-        // means all cross-platform Java builds are broken for checked-in JDKs.
-        assertContainsEvent(
-            "no such target '//does/not/exist:c-default': target 'c-default' not declared in");
-      } else {
-        assertContainsEvent(
-            "no such target '//does/not/exist:b-k8': target 'b-k8' not declared in package");
-      }
+      // Expected
     }
   }
 
@@ -1114,8 +1103,7 @@ public class BuildViewTest extends BuildViewTestBase {
       update(defaultFlags().with(Flag.KEEP_GOING));
       fail();
     } catch (LoadingFailedException | InvalidConfigurationException e) {
-      assertContainsEvent(
-          "no such target '//xcode:does_not_exist': target 'does_not_exist' not declared");
+      assertThat(e.getMessage()).contains("//xcode:does_not_exist");
     }
   }