commit c18381e17f3fcadb7771c2c51087de3421b62316
author Mark Schaller <mschaller@google.com> Wed Jul 29 16:51:14 2015 +0000
committer Damien Martin-Guillerez <dmarting@google.com> Thu Jul 30 11:30:18 2015 +0000
tree 41e7049d9205710b74ab2f63cfb61ec4af2d70af
parent 49543429f56cfe9c90562a1e9f9c66a9eabb5385

Cleanup of TransitiveTarget and TargetMarker functions

Added more information to TargetMarkerFunction documentation.

Cleaned up and rearranged some code in TransitiveTargetFunction to
help with future refactoring.

--
MOS_MIGRATED_REVID=99384291

src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
index 9e21ee9..2dd448b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
@@ -43,7 +43,7 @@
 import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
-import com.google.devtools.build.skyframe.ValueOrException;
+import com.google.devtools.build.skyframe.ValueOrException2;
 
 import java.util.Collection;
 import java.util.HashSet;
@@ -71,15 +71,11 @@
     SkyKey targetKey = TargetMarkerValue.key(label);
     Target target;
     boolean packageLoadedSuccessfully;
-    boolean successfulTransitiveLoading = true;
     NestedSetBuilder<Label> transitiveRootCauses = NestedSetBuilder.stableOrder();
     NoSuchTargetException errorLoadingTarget = null;
     try {
-      // TODO(bazel-team): Why not NoSuchTargetException and NoSuchPackageException explicitly?
-      // Please note that the exception values declared thrown by TargetMarkerFunction are exactly
-      // those two.
       TargetMarkerValue targetValue = (TargetMarkerValue) env.getValueOrThrow(targetKey,
-          NoSuchThingException.class);
+          NoSuchTargetException.class, NoSuchPackageException.class);
       if (targetValue == null) {
         return null;
       }
@@ -121,14 +117,10 @@
         throw new IllegalStateException("Expected target to exist", nste);
       }
 
-      successfulTransitiveLoading = false;
-      transitiveRootCauses.add(label);
       errorLoadingTarget = e;
       packageLoadedSuccessfully = false;
     } catch (NoSuchPackageException e) {
       throw new TransitiveTargetFunctionException(e);
-    } catch (NoSuchThingException e) {
-      throw new IllegalStateException(e + " not NoSuchTargetException or NoSuchPackageException");
     }
 
     NestedSetBuilder<PackageIdentifier> transitiveSuccessfulPkgs = NestedSetBuilder.stableOrder();
@@ -144,12 +136,14 @@
     if (packageLoadedSuccessfully) {
       transitiveSuccessfulPkgs.add(packageId);
     } else {
+      transitiveRootCauses.add(label);
       transitiveUnsuccessfulPkgs.add(packageId);
     }
     transitiveTargets.add(target.getLabel());
 
     // Process deps from attributes of current target.
     Iterable<SkyKey> depKeys = getLabelDepKeys(target);
+    boolean successfulTransitiveLoading = packageLoadedSuccessfully;
     successfulTransitiveLoading &= processDeps(env, target, transitiveRootCauses,
         transitiveSuccessfulPkgs, transitiveUnsuccessfulPkgs, transitiveTargets, depKeys,
         transitiveConfigFragments, configFragmentsFromDeps);
@@ -213,8 +207,9 @@
       NestedSetBuilder<Class<? extends BuildConfiguration.Fragment>> transitiveConfigFragments,
       Set<Class<? extends BuildConfiguration.Fragment>> addedConfigFragments) {
     boolean successfulTransitiveLoading = true;
-    for (Entry<SkyKey, ValueOrException<NoSuchThingException>> entry :
-        env.getValuesOrThrow(depKeys, NoSuchThingException.class).entrySet()) {
+    for (Entry<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>> entry :
+        env.getValuesOrThrow(depKeys, NoSuchPackageException.class, NoSuchTargetException.class)
+            .entrySet()) {
       Label depLabel = (Label) entry.getKey().argument();
       TransitiveTargetValue transitiveTargetValue;
       try {
@@ -227,8 +222,6 @@
         transitiveRootCauses.add(depLabel);
         maybeReportErrorAboutMissingEdge(target, depLabel, e, env.getListener());
         continue;
-      } catch (NoSuchThingException e) {
-        throw new IllegalStateException("Unexpected Exception type from TransitiveTargetValue.", e);
       }
       transitiveSuccessfulPkgs.addTransitive(
           transitiveTargetValue.getTransitiveSuccessfulPackages());