Extract base class from TransitiveTargetFunction

--
MOS_MIGRATED_REVID=99386094

2 files changed, 507 insertions, 257 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java
new file mode 100644
index 0000000000..36484551e6
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java
@@ -0,0 +1,376 @@
+// Copyright 2015 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Multimap;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.packages.AspectDefinition;
+import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.InputFile;
+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.OutputFile;
+import com.google.devtools.build.lib.packages.Package;
+import com.google.devtools.build.lib.packages.PackageGroup;
+import com.google.devtools.build.lib.packages.Rule;
+import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.packages.TargetUtils;
+import com.google.devtools.build.lib.syntax.Label;
+import com.google.devtools.build.skyframe.SkyFunction;
+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.ValueOrException2;
+
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map.Entry;
+import java.util.Set;
+
+import javax.annotation.Nullable;
+
+/**
+ * This class can be extended to define {@link SkyFunction}s that traverse a target and its
+ * transitive dependencies and return values based on that traversal.
+ *
+ * <p>The {@code TProcessedTargets} type parameter represents the result of processing a target and
+ * its transitive dependencies.
+ *
+ * <p>{@code TransitiveBaseTraversalFunction} asks for one to be constructed via {@link
+ * #processTarget}, and then asks for it to be updated based on the current target's
+ * attributes' dependencies via {@link #processDeps}, and then asks for it to be updated based
+ * on the current target' aspects' dependencies via {@link #processDeps}. Finally, it calls
+ * {@link #computeSkyValue} with the {#code ProcessedTargets} to get the {@link SkyValue} to
+ * return.
+ */
+abstract class TransitiveBaseTraversalFunction<TProcessedTargets>
+    implements SkyFunction {
+
+  /**
+   * Returns a {@link SkyKey} corresponding to the traversal of a target specified by {@code label}
+   * and its transitive dependencies.
+   *
+   * <p>Extenders of this class should implement this function to return a key with their
+   * specialized {@link SkyFunction}'s name.
+   *
+   * <p>{@link TransitiveBaseTraversalFunction} calls this for each dependency of a target, and
+   * then gets their values from the environment.
+   *
+   * <p>The key's {@link SkyFunction} may throw at most {@link NoSuchPackageException} and
+   * {@link NoSuchTargetException}. Other exception types are not handled by {@link
+   * TransitiveBaseTraversalFunction}.
+   */
+  abstract SkyKey getKey(Label label);
+
+  abstract TProcessedTargets processTarget(Label label,
+      TargetAndErrorIfAny targetAndErrorIfAny);
+
+  abstract void processDeps(TProcessedTargets processedTargets, EventHandler eventHandler,
+      TargetAndErrorIfAny targetAndErrorIfAny,
+      Iterable<Entry<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>>>
+          depEntries);
+
+  /**
+   * Returns a {@link SkyValue} based on the target and any errors it has, and the values
+   * accumulated across it and a traversal of its transitive dependencies.
+   */
+  abstract SkyValue computeSkyValue(TargetAndErrorIfAny targetAndErrorIfAny,
+      TProcessedTargets processedTargets);
+
+  @Override
+  public SkyValue compute(SkyKey key, Environment env)
+      throws TransitiveBaseTraversalFunctionException {
+    Label label = (Label) key.argument();
+    LoadTargetResults loadTargetResults;
+    try {
+      loadTargetResults = loadTarget(env, label);
+    } catch (NoSuchTargetException e) {
+      throw new TransitiveBaseTraversalFunctionException(e);
+    } catch (NoSuchPackageException e) {
+      throw new TransitiveBaseTraversalFunctionException(e);
+    }
+    LoadTargetResultsType loadTargetResultsType = loadTargetResults.getType();
+    if (loadTargetResultsType.equals(LoadTargetResultsType.VALUES_MISSING)) {
+      return null;
+    }
+    Preconditions.checkState(
+        loadTargetResultsType.equals(LoadTargetResultsType.TARGET_AND_ERROR_IF_ANY),
+        loadTargetResultsType);
+    TargetAndErrorIfAny targetAndErrorIfAny = (TargetAndErrorIfAny) loadTargetResults;
+    TProcessedTargets processedTargets = processTarget(label, targetAndErrorIfAny);
+
+    // Process deps from attributes of current target.
+    Iterable<SkyKey> labelDepKeys = getLabelDepKeys(targetAndErrorIfAny.getTarget());
+    Set<Entry<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>>>
+        labelDepEntries = env.getValuesOrThrow(labelDepKeys, NoSuchPackageException.class,
+        NoSuchTargetException.class).entrySet();
+    processDeps(processedTargets, env.getListener(), targetAndErrorIfAny, labelDepEntries);
+    if (env.valuesMissing()) {
+      return null;
+    }
+
+    // Process deps from aspects.
+    Iterable<SkyKey> labelAspectKeys = getLabelAspectKeys(targetAndErrorIfAny.getTarget(), env);
+    Set<Entry<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>>>
+        labelAspectEntries = env.getValuesOrThrow(labelAspectKeys, NoSuchPackageException.class,
+        NoSuchTargetException.class).entrySet();
+    processDeps(processedTargets, env.getListener(), targetAndErrorIfAny, labelAspectEntries);
+    if (env.valuesMissing()) {
+      return null;
+    }
+
+    return computeSkyValue(targetAndErrorIfAny, processedTargets);
+  }
+
+  @Override
+  public String extractTag(SkyKey skyKey) {
+    return Label.print(((Label) skyKey.argument()));
+  }
+
+  private Iterable<SkyKey> getLabelAspectKeys(Target target, Environment env) {
+    List<SkyKey> depKeys = Lists.newArrayList();
+    if (target instanceof Rule) {
+      Multimap<Attribute, Label> transitions =
+          ((Rule) target).getTransitions(Rule.NO_NODEP_ATTRIBUTES);
+      for (Entry<Attribute, Label> entry : transitions.entries()) {
+        SkyKey packageKey = PackageValue.key(entry.getValue().getPackageIdentifier());
+        try {
+          PackageValue pkgValue = (PackageValue) env.getValueOrThrow(packageKey,
+              NoSuchThingException.class);
+          if (pkgValue == null) {
+            continue;
+          }
+          Collection<Label> labels = AspectDefinition.visitAspectsIfRequired(target, entry.getKey(),
+              pkgValue.getPackage().getTarget(entry.getValue().getName())).values();
+          for (Label label : labels) {
+            depKeys.add(getKey(label));
+          }
+        } catch (NoSuchThingException e) {
+          // Do nothing. This error was handled when we computed the corresponding
+          // TransitiveTargetValue.
+        }
+      }
+    }
+    return depKeys;
+  }
+
+  private Iterable<SkyKey> getLabelDepKeys(Target target) {
+    List<SkyKey> depKeys = Lists.newArrayList();
+    for (Label depLabel : getLabelDeps(target)) {
+      depKeys.add(getKey(depLabel));
+    }
+    return depKeys;
+  }
+
+  // TODO(bazel-team): Unify this logic with that in LabelVisitor, and possibly DependencyResolver.
+  private static Iterable<Label> getLabelDeps(Target target) {
+    final Set<Label> labels = new HashSet<>();
+    if (target instanceof OutputFile) {
+      Rule rule = ((OutputFile) target).getGeneratingRule();
+      labels.add(rule.getLabel());
+      visitTargetVisibility(target, labels);
+    } else if (target instanceof InputFile) {
+      visitTargetVisibility(target, labels);
+    } else if (target instanceof Rule) {
+      visitTargetVisibility(target, labels);
+      visitRule(target, labels);
+    } else if (target instanceof PackageGroup) {
+      visitPackageGroup((PackageGroup) target, labels);
+    }
+    return labels;
+  }
+
+  private static void visitRule(Target target, Set<Label> labels) {
+    labels.addAll(((Rule) target).getLabels(Rule.NO_NODEP_ATTRIBUTES));
+  }
+
+  private static void visitTargetVisibility(Target target, Set<Label> labels) {
+    labels.addAll(target.getVisibility().getDependencyLabels());
+  }
+
+  private static void visitPackageGroup(PackageGroup packageGroup, Set<Label> labels) {
+    labels.addAll(packageGroup.getIncludes());
+  }
+
+  protected void maybeReportErrorAboutMissingEdge(Target target, Label depLabel,
+      NoSuchThingException e, EventHandler eventHandler) {
+    if (e instanceof NoSuchTargetException) {
+      NoSuchTargetException nste = (NoSuchTargetException) e;
+      if (depLabel.equals(nste.getLabel())) {
+        eventHandler.handle(Event.error(TargetUtils.getLocationMaybe(target),
+            TargetUtils.formatMissingEdge(target, depLabel, e)));
+      }
+    } else if (e instanceof NoSuchPackageException) {
+      NoSuchPackageException nspe = (NoSuchPackageException) e;
+      if (nspe.getPackageId().equals(depLabel.getPackageIdentifier())) {
+        eventHandler.handle(Event.error(TargetUtils.getLocationMaybe(target),
+            TargetUtils.formatMissingEdge(target, depLabel, e)));
+      }
+    }
+  }
+
+  enum LoadTargetResultsType {
+    VALUES_MISSING,
+    TARGET_AND_ERROR_IF_ANY
+  }
+
+  interface LoadTargetResults {
+    LoadTargetResultsType getType();
+  }
+
+  private static class ValuesMissing implements LoadTargetResults {
+
+    private static final ValuesMissing INSTANCE = new ValuesMissing();
+
+    private ValuesMissing() {}
+
+    @Override
+    public LoadTargetResultsType getType() {
+      return LoadTargetResultsType.VALUES_MISSING;
+    }
+  }
+
+  interface TargetAndErrorIfAny {
+
+    boolean isPackageLoadedSuccessfully();
+
+    @Nullable NoSuchTargetException getErrorLoadingTarget();
+
+    Target getTarget();
+  }
+
+  private static class TargetAndErrorIfAnyImpl implements TargetAndErrorIfAny, LoadTargetResults {
+
+    private final boolean packageLoadedSuccessfully;
+    @Nullable private final NoSuchTargetException errorLoadingTarget;
+    private final Target target;
+
+    private TargetAndErrorIfAnyImpl(boolean packageLoadedSuccessfully,
+        @Nullable NoSuchTargetException errorLoadingTarget, Target target) {
+      this.packageLoadedSuccessfully = packageLoadedSuccessfully;
+      this.errorLoadingTarget = errorLoadingTarget;
+      this.target = target;
+    }
+
+    @Override
+    public LoadTargetResultsType getType() {
+      return LoadTargetResultsType.TARGET_AND_ERROR_IF_ANY;
+    }
+
+    @Override
+    public boolean isPackageLoadedSuccessfully() {
+      return packageLoadedSuccessfully;
+    }
+
+    @Override
+    @Nullable
+    public NoSuchTargetException getErrorLoadingTarget() {
+      return errorLoadingTarget;
+    }
+
+    @Override
+    public Target getTarget() {
+      return target;
+    }
+  }
+
+  private static LoadTargetResults loadTarget(Environment env, Label label)
+      throws NoSuchTargetException, NoSuchPackageException {
+    SkyKey packageKey = PackageValue.key(label.getPackageIdentifier());
+    SkyKey targetKey = TargetMarkerValue.key(label);
+
+    boolean packageLoadedSuccessfully;
+    Target target;
+    NoSuchTargetException errorLoadingTarget = null;
+    try {
+      TargetMarkerValue targetValue = (TargetMarkerValue) env.getValueOrThrow(targetKey,
+          NoSuchTargetException.class, NoSuchPackageException.class);
+      if (targetValue == null) {
+        return ValuesMissing.INSTANCE;
+      }
+      PackageValue packageValue = (PackageValue) env.getValueOrThrow(packageKey,
+          NoSuchPackageException.class);
+      if (packageValue == null) {
+        return ValuesMissing.INSTANCE;
+      }
+
+      packageLoadedSuccessfully = true;
+      try {
+        target = packageValue.getPackage().getTarget(label.getName());
+      } catch (NoSuchTargetException unexpected) {
+        // Not expected since the TargetMarkerFunction would have failed earlier if the Target
+        // was not present.
+        throw new IllegalStateException(unexpected);
+      }
+    } catch (NoSuchTargetException e) {
+      if (!e.hasTarget()) {
+        throw e;
+      }
+
+      // We know that a Target may be extracted, but we need to get it out of the Package
+      // (which is known to be in error).
+      Package pkg;
+      try {
+        PackageValue packageValue = (PackageValue) env.getValueOrThrow(packageKey,
+            NoSuchPackageException.class);
+        if (packageValue == null) {
+          return ValuesMissing.INSTANCE;
+        }
+        throw new IllegalStateException(
+            "Expected bad package: " + label.getPackageIdentifier());
+      } catch (NoSuchPackageException nsp) {
+        pkg = Preconditions.checkNotNull(nsp.getPackage(), label.getPackageIdentifier());
+      }
+      try {
+        target = pkg.getTarget(label.getName());
+      } catch (NoSuchTargetException nste) {
+        throw new IllegalStateException("Expected target to exist", nste);
+      }
+
+      errorLoadingTarget = e;
+      packageLoadedSuccessfully = false;
+    }
+    return new TargetAndErrorIfAnyImpl(packageLoadedSuccessfully, errorLoadingTarget, target);
+  }
+
+  /**
+   * Used to declare all the exception types that can be wrapped in the exception thrown by
+   * {@link TransitiveTraversalFunction#compute}.
+   */
+  static class TransitiveBaseTraversalFunctionException extends SkyFunctionException {
+    /**
+     * Used to propagate an error from a direct target dependency to the target that depended on
+     * it.
+     */
+    public TransitiveBaseTraversalFunctionException(NoSuchPackageException e) {
+      super(e, Transience.PERSISTENT);
+    }
+
+    /**
+     * In nokeep_going mode, used to propagate an error from a direct target dependency to the
+     * target that depended on it.
+     *
+     * <p>In keep_going mode, used the same way, but only for targets that could not be loaded at
+     * all (we proceed with transitive loading on targets that contain errors).</p>
+     */
+    public TransitiveBaseTraversalFunctionException(NoSuchTargetException e) {
+      super(e, Transience.PERSISTENT);
+    }
+  }
+}
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 2dd448b137..8df7e1cfbd 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
@@ -13,50 +13,39 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
-import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Multimap;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
-import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.packages.AspectDefinition;
-import com.google.devtools.build.lib.packages.Attribute;
-import com.google.devtools.build.lib.packages.InputFile;
 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.OutputFile;
-import com.google.devtools.build.lib.packages.Package;
-import com.google.devtools.build.lib.packages.PackageGroup;
 import com.google.devtools.build.lib.packages.PackageIdentifier;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
 import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.packages.TargetUtils;
+import com.google.devtools.build.lib.skyframe.TransitiveTargetFunction.TransitiveTargetValueBuilder;
 import com.google.devtools.build.lib.syntax.Label;
-import com.google.devtools.build.skyframe.SkyFunction;
-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.ValueOrException2;
 
 import java.util.Collection;
-import java.util.HashSet;
 import java.util.LinkedHashSet;
-import java.util.List;
 import java.util.Map.Entry;
 import java.util.Set;
 
+import javax.annotation.Nullable;
+
 /**
  * This class builds transitive Target values such that evaluating a Target value is similar to
  * running it through the LabelVisitor.
  */
-public class TransitiveTargetFunction implements SkyFunction {
+public class TransitiveTargetFunction
+    extends TransitiveBaseTraversalFunction<TransitiveTargetValueBuilder> {
 
   private final ConfiguredRuleClassProvider ruleClassProvider;
 
@@ -65,151 +54,28 @@ public class TransitiveTargetFunction implements SkyFunction {
   }
 
   @Override
-  public SkyValue compute(SkyKey key, Environment env) throws TransitiveTargetFunctionException {
-    Label label = (Label) key.argument();
-    SkyKey packageKey = PackageValue.key(label.getPackageIdentifier());
-    SkyKey targetKey = TargetMarkerValue.key(label);
-    Target target;
-    boolean packageLoadedSuccessfully;
-    NestedSetBuilder<Label> transitiveRootCauses = NestedSetBuilder.stableOrder();
-    NoSuchTargetException errorLoadingTarget = null;
-    try {
-      TargetMarkerValue targetValue = (TargetMarkerValue) env.getValueOrThrow(targetKey,
-          NoSuchTargetException.class, NoSuchPackageException.class);
-      if (targetValue == null) {
-        return null;
-      }
-      PackageValue packageValue = (PackageValue) env.getValueOrThrow(packageKey,
-          NoSuchPackageException.class);
-      if (packageValue == null) {
-        return null;
-      }
-
-      packageLoadedSuccessfully = true;
-      try {
-        target = packageValue.getPackage().getTarget(label.getName());
-      } catch (NoSuchTargetException unexpected) {
-        // Not expected since the TargetMarkerFunction would have failed earlier if the Target
-        // was not present.
-        throw new IllegalStateException(unexpected);
-      }
-    } catch (NoSuchTargetException e) {
-      if (!e.hasTarget()) {
-        throw new TransitiveTargetFunctionException(e);
-      }
-
-      // We know that a Target may be extracted, but we need to get it out of the Package
-      // (which is known to be in error).
-      Package pkg;
-      try {
-        PackageValue packageValue = (PackageValue) env.getValueOrThrow(packageKey,
-            NoSuchPackageException.class);
-        if (packageValue == null) {
-          return null;
-        }
-        throw new IllegalStateException("Expected bad package: " + label.getPackageIdentifier());
-      } catch (NoSuchPackageException nsp) {
-        pkg = Preconditions.checkNotNull(nsp.getPackage(), label.getPackageIdentifier());
-      }
-      try {
-        target = pkg.getTarget(label.getName());
-      } catch (NoSuchTargetException nste) {
-        throw new IllegalStateException("Expected target to exist", nste);
-      }
-
-      errorLoadingTarget = e;
-      packageLoadedSuccessfully = false;
-    } catch (NoSuchPackageException e) {
-      throw new TransitiveTargetFunctionException(e);
-    }
-
-    NestedSetBuilder<PackageIdentifier> transitiveSuccessfulPkgs = NestedSetBuilder.stableOrder();
-    NestedSetBuilder<PackageIdentifier> transitiveUnsuccessfulPkgs = NestedSetBuilder.stableOrder();
-    NestedSetBuilder<Label> transitiveTargets = NestedSetBuilder.stableOrder();
-    NestedSetBuilder<Class<? extends BuildConfiguration.Fragment>> transitiveConfigFragments =
-        NestedSetBuilder.stableOrder();
-    // No need to store directly required fragments that are also required by deps.
-    Set<Class<? extends BuildConfiguration.Fragment>> configFragmentsFromDeps =
-        new LinkedHashSet<>();
-
-    PackageIdentifier packageId = target.getPackage().getPackageIdentifier();
-    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);
-    if (env.valuesMissing()) {
-      return null;
-    }
-    // Process deps from aspects.
-    depKeys = getLabelAspectKeys(target, env);
-    successfulTransitiveLoading &= processDeps(env, target, transitiveRootCauses,
-        transitiveSuccessfulPkgs, transitiveUnsuccessfulPkgs, transitiveTargets, depKeys,
-        transitiveConfigFragments, configFragmentsFromDeps);
-    if (env.valuesMissing()) {
-      return null;
-    }
-
-    // Get configuration fragments directly required by this target.
-    if (target instanceof Rule) {
-      Set<Class<?>> configFragments =
-          target.getAssociatedRule().getRuleClassObject().getRequiredConfigurationFragments();
-      // An empty result means this rule requires all fragments (which practically means
-      // the rule isn't yet declaring its actually needed fragments). So load everything.
-      configFragments = configFragments.isEmpty() ? getAllFragments() : configFragments;
-      for (Class<?> fragment : configFragments) {
-        if (!configFragmentsFromDeps.contains(fragment)) {
-          transitiveConfigFragments.add((Class<? extends BuildConfiguration.Fragment>) fragment);
-        }
-      }
-    }
-
-    NestedSet<PackageIdentifier> successfullyLoadedPackages = transitiveSuccessfulPkgs.build();
-    NestedSet<PackageIdentifier> unsuccessfullyLoadedPackages = transitiveUnsuccessfulPkgs.build();
-    NestedSet<Label> loadedTargets = transitiveTargets.build();
-    if (successfulTransitiveLoading) {
-      return TransitiveTargetValue.successfulTransitiveLoading(successfullyLoadedPackages,
-          unsuccessfullyLoadedPackages, loadedTargets, transitiveConfigFragments.build());
-    } else {
-      NestedSet<Label> rootCauses = transitiveRootCauses.build();
-      return TransitiveTargetValue.unsuccessfulTransitiveLoading(successfullyLoadedPackages,
-          unsuccessfullyLoadedPackages, loadedTargets, rootCauses, errorLoadingTarget,
-          transitiveConfigFragments.build());
-    }
+  SkyKey getKey(Label label) {
+    return TransitiveTargetValue.key(label);
   }
 
-  /**
-   * Returns every configuration fragment known to the system.
-   */
-  private Set<Class<?>> getAllFragments() {
-    ImmutableSet.Builder<Class<?>> builder =
-        ImmutableSet.builder();
-    for (ConfigurationFragmentFactory factory : ruleClassProvider.getConfigurationFragments()) {
-      builder.add(factory.creates());
-    }
-    return builder.build();
+  @Override
+  TransitiveTargetValueBuilder processTarget(Label label, TargetAndErrorIfAny targetAndErrorIfAny) {
+    Target target = targetAndErrorIfAny.getTarget();
+    boolean packageLoadedSuccessfully = targetAndErrorIfAny.isPackageLoadedSuccessfully();
+    return new TransitiveTargetValueBuilder(label, target, packageLoadedSuccessfully);
   }
 
-  private boolean processDeps(Environment env, Target target,
-      NestedSetBuilder<Label> transitiveRootCauses,
-      NestedSetBuilder<PackageIdentifier> transitiveSuccessfulPkgs,
-      NestedSetBuilder<PackageIdentifier> transitiveUnsuccessfulPkgs,
-      NestedSetBuilder<Label> transitiveTargets, Iterable<SkyKey> depKeys,
-      NestedSetBuilder<Class<? extends BuildConfiguration.Fragment>> transitiveConfigFragments,
-      Set<Class<? extends BuildConfiguration.Fragment>> addedConfigFragments) {
-    boolean successfulTransitiveLoading = true;
+  @Override
+  void processDeps(TransitiveTargetValueBuilder builder, EventHandler eventHandler,
+      TargetAndErrorIfAny targetAndErrorIfAny,
+      Iterable<Entry<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>>>
+          depEntries) {
+    boolean successfulTransitiveLoading = builder.isSuccessfulTransitiveLoading();
+    Target target = targetAndErrorIfAny.getTarget();
+    NestedSetBuilder<Label> transitiveRootCauses = builder.getTransitiveRootCauses();
+
     for (Entry<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>> entry :
-        env.getValuesOrThrow(depKeys, NoSuchPackageException.class, NoSuchTargetException.class)
-            .entrySet()) {
+        depEntries) {
       Label depLabel = (Label) entry.getKey().argument();
       TransitiveTargetValue transitiveTargetValue;
       try {
@@ -220,27 +86,27 @@ public class TransitiveTargetFunction implements SkyFunction {
       } catch (NoSuchPackageException | NoSuchTargetException e) {
         successfulTransitiveLoading = false;
         transitiveRootCauses.add(depLabel);
-        maybeReportErrorAboutMissingEdge(target, depLabel, e, env.getListener());
+        maybeReportErrorAboutMissingEdge(target, depLabel, e, eventHandler);
         continue;
       }
-      transitiveSuccessfulPkgs.addTransitive(
+      builder.getTransitiveSuccessfulPkgs().addTransitive(
           transitiveTargetValue.getTransitiveSuccessfulPackages());
-      transitiveUnsuccessfulPkgs.addTransitive(
+      builder.getTransitiveUnsuccessfulPkgs().addTransitive(
           transitiveTargetValue.getTransitiveUnsuccessfulPackages());
-      transitiveTargets.addTransitive(transitiveTargetValue.getTransitiveTargets());
+      builder.getTransitiveTargets().addTransitive(transitiveTargetValue.getTransitiveTargets());
       NestedSet<Label> rootCauses = transitiveTargetValue.getTransitiveRootCauses();
       if (rootCauses != null) {
         successfulTransitiveLoading = false;
         transitiveRootCauses.addTransitive(rootCauses);
         if (transitiveTargetValue.getErrorLoadingTarget() != null) {
           maybeReportErrorAboutMissingEdge(target, depLabel,
-              transitiveTargetValue.getErrorLoadingTarget(), env.getListener());
+              transitiveTargetValue.getErrorLoadingTarget(), eventHandler);
         }
       }
 
-      NestedSet<Class<? extends BuildConfiguration.Fragment>> depFragments =
+      NestedSet<Class<? extends Fragment>> depFragments =
           transitiveTargetValue.getTransitiveConfigFragments();
-      Collection<Class<? extends BuildConfiguration.Fragment>> depFragmentsAsCollection =
+      Collection<Class<? extends Fragment>> depFragmentsAsCollection =
           depFragments.toCollection();
       // The simplest collection technique would be to unconditionally add all deps' nested
       // sets to the current target's nested set. But when there's large overlap between their
@@ -248,123 +114,131 @@ public class TransitiveTargetFunction implements SkyFunction {
       // that don't contribute anything unique to the required fragment set. So we optimize here
       // by completely skipping sets that don't offer anything new. More fine-tuned optimization
       // is possible, but this offers a good balance between simplicity and practical efficiency.
+      Set<Class<? extends Fragment>> addedConfigFragments = builder.getConfigFragmentsFromDeps();
       if (!addedConfigFragments.containsAll(depFragmentsAsCollection)) {
-        transitiveConfigFragments.addTransitive(depFragments);
+        builder.getTransitiveConfigFragments().addTransitive(depFragments);
         addedConfigFragments.addAll(depFragmentsAsCollection);
       }
     }
-    return successfulTransitiveLoading;
+    builder.setSuccessfulTransitiveLoading(successfulTransitiveLoading);
   }
 
   @Override
-  public String extractTag(SkyKey skyKey) {
-    return Label.print(((Label) skyKey.argument()));
-  }
+  public SkyValue computeSkyValue(TargetAndErrorIfAny targetAndErrorIfAny,
+      TransitiveTargetValueBuilder builder) {
+    Target target = targetAndErrorIfAny.getTarget();
+    NoSuchTargetException errorLoadingTarget = targetAndErrorIfAny.getErrorLoadingTarget();
 
-  private static void maybeReportErrorAboutMissingEdge(Target target, Label depLabel,
-      NoSuchThingException e, EventHandler eventHandler) {
-    if (e instanceof NoSuchTargetException) {
-      NoSuchTargetException nste = (NoSuchTargetException) e;
-      if (depLabel.equals(nste.getLabel())) {
-        eventHandler.handle(Event.error(TargetUtils.getLocationMaybe(target),
-            TargetUtils.formatMissingEdge(target, depLabel, e)));
-      }
-    } else if (e instanceof NoSuchPackageException) {
-      NoSuchPackageException nspe = (NoSuchPackageException) e;
-      if (nspe.getPackageId().equals(depLabel.getPackageIdentifier())) {
-        eventHandler.handle(Event.error(TargetUtils.getLocationMaybe(target),
-            TargetUtils.formatMissingEdge(target, depLabel, e)));
-      }
-    }
-  }
-
-  private static Iterable<SkyKey> getLabelAspectKeys(Target target, Environment env) {
-    List<SkyKey> depKeys = Lists.newArrayList();
+    // Get configuration fragments directly required by this target.
     if (target instanceof Rule) {
-      Multimap<Attribute, Label> transitions =
-          ((Rule) target).getTransitions(Rule.NO_NODEP_ATTRIBUTES);
-      for (Entry<Attribute, Label> entry : transitions.entries()) {
-        SkyKey packageKey = PackageValue.key(entry.getValue().getPackageIdentifier());
-        try {
-          PackageValue pkgValue = (PackageValue) env.getValueOrThrow(packageKey,
-              NoSuchThingException.class);
-          if (pkgValue == null) {
-            continue;
-          }
-          Collection<Label> labels = AspectDefinition.visitAspectsIfRequired(target, entry.getKey(),
-              pkgValue.getPackage().getTarget(entry.getValue().getName())).values();
-          for (Label label : labels) {
-            depKeys.add(TransitiveTargetValue.key(label));
-          }
-        } catch (NoSuchThingException e) {
-          // Do nothing. This error was handled when we computed the corresponding
-          // TransitiveTargetValue.
+      Set<Class<?>> configFragments =
+          target.getAssociatedRule().getRuleClassObject().getRequiredConfigurationFragments();
+      // An empty result means this rule requires all fragments (which practically means
+      // the rule isn't yet declaring its actually needed fragments). So load everything.
+      configFragments = configFragments.isEmpty() ? getAllFragments() : configFragments;
+      for (Class<?> fragment : configFragments) {
+        if (!builder.getConfigFragmentsFromDeps().contains(fragment)) {
+          builder.getTransitiveConfigFragments().add(
+              (Class<? extends BuildConfiguration.Fragment>) fragment);
         }
       }
     }
-    return depKeys;
+
+    return builder.build(errorLoadingTarget);
   }
 
-  private static Iterable<SkyKey> getLabelDepKeys(Target target) {
-    List<SkyKey> depKeys = Lists.newArrayList();
-    for (Label depLabel : getLabelDeps(target)) {
-      depKeys.add(TransitiveTargetValue.key(depLabel));
+  /**
+   * Returns every configuration fragment known to the system.
+   */
+  private Set<Class<?>> getAllFragments() {
+    ImmutableSet.Builder<Class<?>> builder = ImmutableSet.builder();
+    for (ConfigurationFragmentFactory factory : ruleClassProvider.getConfigurationFragments()) {
+      builder.add(factory.creates());
     }
-    return depKeys;
+    return builder.build();
   }
 
-  // TODO(bazel-team): Unify this logic with that in LabelVisitor, and possibly DependencyResolver.
-  private static Iterable<Label> getLabelDeps(Target target) {
-    final Set<Label> labels = new HashSet<>();
-    if (target instanceof OutputFile) {
-      Rule rule = ((OutputFile) target).getGeneratingRule();
-      labels.add(rule.getLabel());
-      visitTargetVisibility(target, labels);
-    } else if (target instanceof InputFile) {
-      visitTargetVisibility(target, labels);
-    } else if (target instanceof Rule) {
-      visitTargetVisibility(target, labels);
-      visitRule(target, labels);
-    } else if (target instanceof PackageGroup) {
-      visitPackageGroup((PackageGroup) target, labels);
+  /**
+   * Holds values accumulated across the given target and its transitive dependencies for the
+   * purpose of constructing a {@link TransitiveTargetValue}.
+   *
+   * <p>Note that this class is mutable! The {@code successfulTransitiveLoading} property is
+   * initialized with the {@code packageLoadedSuccessfully} constructor parameter, and may be
+   * modified if a transitive dependency is found to be in error.
+   */
+  static class TransitiveTargetValueBuilder {
+    private boolean successfulTransitiveLoading;
+    private final NestedSetBuilder<PackageIdentifier> transitiveSuccessfulPkgs;
+    private final NestedSetBuilder<PackageIdentifier> transitiveUnsuccessfulPkgs;
+    private final NestedSetBuilder<Label> transitiveTargets;
+    private final NestedSetBuilder<Class<? extends Fragment>> transitiveConfigFragments;
+    private final Set<Class<? extends Fragment>> configFragmentsFromDeps;
+    private final NestedSetBuilder<Label> transitiveRootCauses;
+
+    public TransitiveTargetValueBuilder(Label label, Target target,
+        boolean packageLoadedSuccessfully) {
+      this.transitiveSuccessfulPkgs = NestedSetBuilder.stableOrder();
+      this.transitiveUnsuccessfulPkgs = NestedSetBuilder.stableOrder();
+      this.transitiveTargets = NestedSetBuilder.stableOrder();
+      this.transitiveConfigFragments = NestedSetBuilder.stableOrder();
+      // No need to store directly required fragments that are also required by deps.
+      this.configFragmentsFromDeps = new LinkedHashSet<>();
+      this.transitiveRootCauses = NestedSetBuilder.stableOrder();
+
+      this.successfulTransitiveLoading = packageLoadedSuccessfully;
+      PackageIdentifier packageId = target.getPackage().getPackageIdentifier();
+      if (packageLoadedSuccessfully) {
+        transitiveSuccessfulPkgs.add(packageId);
+      } else {
+        transitiveRootCauses.add(label);
+        transitiveUnsuccessfulPkgs.add(packageId);
+      }
+      transitiveTargets.add(target.getLabel());
     }
-    return labels;
-  }
 
-  private static void visitRule(Target target, Set<Label> labels) {
-    labels.addAll(((Rule) target).getLabels(Rule.NO_NODEP_ATTRIBUTES));
-  }
+    public NestedSetBuilder<PackageIdentifier> getTransitiveSuccessfulPkgs() {
+      return transitiveSuccessfulPkgs;
+    }
 
-  private static void visitTargetVisibility(Target target, Set<Label> labels) {
-    labels.addAll(target.getVisibility().getDependencyLabels());
-  }
+    public NestedSetBuilder<PackageIdentifier> getTransitiveUnsuccessfulPkgs() {
+      return transitiveUnsuccessfulPkgs;
+    }
 
-  private static void visitPackageGroup(PackageGroup packageGroup, Set<Label> labels) {
-    labels.addAll(packageGroup.getIncludes());
-  }
+    public NestedSetBuilder<Label> getTransitiveTargets() {
+      return transitiveTargets;
+    }
 
-  /**
-   * Used to declare all the exception types that can be wrapped in the exception thrown by
-   * {@link TransitiveTargetFunction#compute}.
-   */
-  private static class TransitiveTargetFunctionException extends SkyFunctionException {
-    /**
-     * Used to propagate an error from a direct target dependency to the
-     * target that depended on it.
-     */
-    public TransitiveTargetFunctionException(NoSuchPackageException e) {
-      super(e, Transience.PERSISTENT);
+    public NestedSetBuilder<Class<? extends Fragment>> getTransitiveConfigFragments() {
+      return transitiveConfigFragments;
+    }
+
+    public Set<Class<? extends Fragment>> getConfigFragmentsFromDeps() {
+      return configFragmentsFromDeps;
+    }
+
+    public NestedSetBuilder<Label> getTransitiveRootCauses() {
+      return transitiveRootCauses;
+    }
+
+    public boolean isSuccessfulTransitiveLoading() {
+      return successfulTransitiveLoading;
+    }
+
+    public void setSuccessfulTransitiveLoading(boolean successfulTransitiveLoading) {
+      this.successfulTransitiveLoading = successfulTransitiveLoading;
     }
 
-    /**
-     * In nokeep_going mode, used to propagate an error from a direct target dependency to the
-     * target that depended on it.
-     *
-     * <p>In keep_going mode, used the same way, but only for targets that could not be loaded at
-     * all (we proceed with transitive loading on targets that contain errors).</p>
-     */
-    public TransitiveTargetFunctionException(NoSuchTargetException e) {
-      super(e, Transience.PERSISTENT);
+    public SkyValue build(@Nullable NoSuchTargetException errorLoadingTarget) {
+      NestedSet<PackageIdentifier> successfullyLoadedPkgs = transitiveSuccessfulPkgs.build();
+      NestedSet<PackageIdentifier> unsuccessfullyLoadedPkgs = transitiveUnsuccessfulPkgs.build();
+      NestedSet<Label> loadedTargets = transitiveTargets.build();
+      NestedSet<Class<? extends Fragment>> configFragments = transitiveConfigFragments.build();
+      return successfulTransitiveLoading
+          ? TransitiveTargetValue.successfulTransitiveLoading(successfullyLoadedPkgs,
+          unsuccessfullyLoadedPkgs, loadedTargets, configFragments)
+          : TransitiveTargetValue.unsuccessfulTransitiveLoading(successfullyLoadedPkgs,
+              unsuccessfullyLoadedPkgs, loadedTargets, transitiveRootCauses.build(),
+              errorLoadingTarget, configFragments);
     }
   }
 }