Save information about transitive packages in ConfiguredTargetValue and AspectValue.

--
MOS_MIGRATED_REVID=102643564

5 files changed, 77 insertions, 28 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
index d044021693..0f59428f61 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
@@ -95,6 +95,7 @@ public class GenQuery implements RuleConfiguredTargetFactory {
       new Precomputed<>(new SkyKey(SkyFunctions.PRECOMPUTED, "query_output_formatters"));
 
   @Override
+  @Nullable
   public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedException {
     Artifact outputArtifact = ruleContext.createOutputArtifact();
 
@@ -176,15 +177,18 @@ public class GenQuery implements RuleConfiguredTargetFactory {
 
   // The transitive closure of these targets is an upper estimate on the labels
   // the query will touch
+  @Nullable
   private Set<Target> getScope(RuleContext context) {
     List<Label> scopeLabels = context.attributes().get("scope", Type.LABEL_LIST);
     Set<Target> scope = Sets.newHashSetWithExpectedSize(scopeLabels.size());
     for (Label scopePart : scopeLabels) {
       try {
         SkyFunction.Environment env = context.getAnalysisEnvironment().getSkyframeEnv();
-        PackageValue packageNode = Preconditions.checkNotNull(
-            (PackageValue) env.getValue(PackageValue.key(scopePart.getPackageFragment())));
-
+        PackageValue packageNode =  (PackageValue) env.getValue(
+            PackageValue.key(scopePart.getPackageFragment()));
+        if (packageNode == null) {
+          return null;
+        }
         scope.add(packageNode.getPackage().getTarget(scopePart.getName()));
       } catch (NoSuchTargetException e) {
         throw new IllegalStateException(e);
@@ -202,6 +206,7 @@ public class GenQuery implements RuleConfiguredTargetFactory {
     return ruleContext.getAnalysisEnvironment().getEventHandler();
   }
 
+  @Nullable
   private Pair<ImmutableMap<PackageIdentifier, Package>, Set<Label>> constructPackageMap(
       SkyFunction.Environment env, Collection<Target> scope) {
     // It is not necessary for correctness to construct intermediate NestedSets; we could iterate
@@ -212,7 +217,9 @@ public class GenQuery implements RuleConfiguredTargetFactory {
     for (Target target : scope) {
       SkyKey key = TransitiveTargetValue.key(target.getLabel());
       TransitiveTargetValue transNode = (TransitiveTargetValue) env.getValue(key);
-      Preconditions.checkState(transNode != null, "%s not preloaded", key);
+      if (transNode == null) {
+        return null;
+      }
       validTargets.addTransitive(transNode.getTransitiveTargets());
       packageNames.addTransitive(transNode.getTransitiveSuccessfulPackages());
     }
@@ -230,9 +237,15 @@ public class GenQuery implements RuleConfiguredTargetFactory {
   private byte[] executeQuery(RuleContext ruleContext, QueryOptions queryOptions,
       Set<Target> scope, String query) throws InterruptedException {
 
+    if (scope == null) {
+      return null;
+    }
     SkyFunction.Environment env = ruleContext.getAnalysisEnvironment().getSkyframeEnv();
     Pair<ImmutableMap<PackageIdentifier, Package>, Set<Label>> closureInfo =
         constructPackageMap(env, scope);
+    if (closureInfo == null) {
+      return null;
+    }
     ImmutableMap<PackageIdentifier, Package> packageMap = closureInfo.first;
     Set<Label> validTargets = closureInfo.second;
     PackageProvider packageProvider = new PreloadedMapPackageProvider(packageMap, validTargets);
@@ -242,6 +255,7 @@ public class GenQuery implements RuleConfiguredTargetFactory {
     return doQuery(queryOptions, packageProvider, labelFilter, evaluator, query, ruleContext);
   }
 
+  @Nullable
   private byte[] doQuery(QueryOptions queryOptions, PackageProvider packageProvider,
                          Predicate<Label> labelFilter, TargetPatternEvaluator evaluator,
                          String query, RuleContext ruleContext)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index eb3b934018..a4d54c035a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -25,10 +25,12 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget;
 import com.google.devtools.build.lib.analysis.TargetAndConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.AspectFactory;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
+import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
 import com.google.devtools.build.lib.packages.Target;
@@ -61,6 +63,7 @@ public final class AspectFunction implements SkyFunction {
   public SkyValue compute(SkyKey skyKey, Environment env)
       throws AspectFunctionException {
     SkyframeBuildView view = buildViewProvider.getSkyframeBuildView();
+    NestedSetBuilder<Package> transitivePackages = NestedSetBuilder.stableOrder();
     AspectKey key = (AspectKey) skyKey.argument();
     ConfiguredAspectFactory aspectFactory =
         (ConfiguredAspectFactory) AspectFactory.Util.create(key.getAspect());
@@ -109,8 +112,8 @@ public final class AspectFunction implements SkyFunction {
 
     try {
       // Get the configuration targets that trigger this rule's configurable attributes.
-      Set<ConfigMatchingProvider> configConditions =
-          ConfiguredTargetFunction.getConfigConditions(target, env, resolver, ctgValue);
+      Set<ConfigMatchingProvider> configConditions = ConfiguredTargetFunction.getConfigConditions(
+          target, env, resolver, ctgValue, transitivePackages);
       if (configConditions == null) {
         // Those targets haven't yet been resolved.
         return null;
@@ -119,9 +122,11 @@ public final class AspectFunction implements SkyFunction {
       ListMultimap<Attribute, ConfiguredTarget> depValueMap =
           ConfiguredTargetFunction.computeDependencies(env, resolver, ctgValue,
               aspectFactory.getDefinition(), key.getParameters(), configConditions,
-              ruleClassProvider, view.getHostConfiguration(ctgValue.getConfiguration()));
+              ruleClassProvider, view.getHostConfiguration(ctgValue.getConfiguration()),
+              transitivePackages);
 
-      return createAspect(env, key, associatedTarget, configConditions, depValueMap);
+      return createAspect(env, key, associatedTarget, configConditions, depValueMap,
+          transitivePackages);
     } catch (DependencyEvaluationException e) {
       throw new AspectFunctionException(e.getRootCauseSkyKey(), e.getCause());
     }
@@ -130,7 +135,10 @@ public final class AspectFunction implements SkyFunction {
   @Nullable
   private AspectValue createAspect(Environment env, AspectKey key,
       RuleConfiguredTarget associatedTarget, Set<ConfigMatchingProvider> configConditions,
-      ListMultimap<Attribute, ConfiguredTarget> directDeps) throws AspectFunctionException {
+      ListMultimap<Attribute, ConfiguredTarget> directDeps,
+      NestedSetBuilder<Package> transitivePackages)
+      throws AspectFunctionException {
+
     SkyframeBuildView view = buildViewProvider.getSkyframeBuildView();
     BuildConfiguration configuration = associatedTarget.getConfiguration();
 
@@ -168,7 +176,8 @@ public final class AspectFunction implements SkyFunction {
         associatedTarget.getLabel(),
         associatedTarget.getTarget().getLocation(),
         aspect,
-        ImmutableList.copyOf(analysisEnvironment.getRegisteredActions()));
+        ImmutableList.copyOf(analysisEnvironment.getRegisteredActions()),
+        transitivePackages.build());
   }
 
   @Nullable
@@ -176,7 +185,7 @@ public final class AspectFunction implements SkyFunction {
   public String extractTag(SkyKey skyKey) {
     return null;
   }
-  
+
   /**
    * An exception indicating that there was a problem creating an aspect.
    */
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
index f1260ba3c7..7088804372 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -21,8 +21,10 @@ import com.google.devtools.build.lib.analysis.Aspect;
 import com.google.devtools.build.lib.analysis.AspectWithParameters;
 import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.AspectParameters;
+import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.syntax.Label;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
@@ -106,14 +108,17 @@ public final class AspectValue extends ActionLookupValue {
   private final Location location;
   private final AspectKey key;
   private final Aspect aspect;
+  private final NestedSet<Package> transitivePackages;
 
   public AspectValue(
-      AspectKey key, Label label, Location location, Aspect aspect, Iterable<Action> actions) {
+      AspectKey key, Label label, Location location, Aspect aspect, Iterable<Action> actions,
+      NestedSet<Package> transitivePackages) {
     super(actions);
     this.location = location;
     this.label = label;
     this.key = key;
     this.aspect = aspect;
+    this.transitivePackages = transitivePackages;
   }
 
   public Aspect getAspect() {
@@ -132,6 +137,10 @@ public final class AspectValue extends ActionLookupValue {
     return key;
   }
 
+  public NestedSet<Package> getTransitivePackages() {
+    return transitivePackages;
+  }
+
   public static SkyKey key(Label label, BuildConfiguration configuration,
       Class<? extends ConfiguredAspectFactory> aspectFactory,
       AspectParameters additionalConfiguration) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index 77153f8969..8067e60e7c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -42,6 +42,7 @@ import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
 import com.google.devtools.build.lib.analysis.config.HostTransition;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.analysis.config.PatchTransition;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.StoredEventHandler;
 import com.google.devtools.build.lib.packages.AspectDefinition;
@@ -52,6 +53,7 @@ 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.Package;
 import com.google.devtools.build.lib.packages.PackageGroup;
 import com.google.devtools.build.lib.packages.RawAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
@@ -135,7 +137,7 @@ final class ConfiguredTargetFunction implements SkyFunction {
   public SkyValue compute(SkyKey key, Environment env) throws ConfiguredTargetFunctionException,
       InterruptedException {
     SkyframeBuildView view = buildViewProvider.getSkyframeBuildView();
-
+    NestedSetBuilder<Package> transitivePackages = NestedSetBuilder.stableOrder();
     ConfiguredTargetKey configuredTargetKey = (ConfiguredTargetKey) key.argument();
     LabelAndConfiguration lc = LabelAndConfiguration.of(
         configuredTargetKey.getLabel(), configuredTargetKey.getConfiguration());
@@ -155,6 +157,7 @@ final class ConfiguredTargetFunction implements SkyFunction {
       throw new ConfiguredTargetFunctionException(new NoSuchTargetException(lc.getLabel(),
           "No such target"));
     }
+    transitivePackages.add(packageValue.getPackage());
     // TODO(bazel-team): This is problematic - we create the right key, but then end up with a value
     // that doesn't match; we can even have the same value multiple times. However, I think it's
     // only triggered in tests (i.e., in normal operation, the configuration passed in is already
@@ -177,16 +180,16 @@ final class ConfiguredTargetFunction implements SkyFunction {
     try {
       // Get the configuration targets that trigger this rule's configurable attributes.
       Set<ConfigMatchingProvider> configConditions =
-          getConfigConditions(ctgValue.getTarget(), env, resolver, ctgValue);
+          getConfigConditions(ctgValue.getTarget(), env, resolver, ctgValue, transitivePackages);
       if (env.valuesMissing()) {
         return null;
       }
 
       ListMultimap<Attribute, ConfiguredTarget> depValueMap = computeDependencies(env, resolver,
           ctgValue, null, AspectParameters.EMPTY, configConditions, ruleClassProvider,
-          view.getHostConfiguration(configuration));
+          view.getHostConfiguration(configuration), transitivePackages);
       ConfiguredTargetValue ans = createConfiguredTarget(
-          view, env, target, configuration, depValueMap, configConditions);
+          view, env, target, configuration, depValueMap, configConditions, transitivePackages);
       return ans;
     } catch (DependencyEvaluationException e) {
       throw new ConfiguredTargetFunctionException(e.getRootCauseSkyKey(), e.getCause());
@@ -220,7 +223,7 @@ final class ConfiguredTargetFunction implements SkyFunction {
       Environment env, SkyframeDependencyResolver resolver, TargetAndConfiguration ctgValue,
       AspectDefinition aspectDefinition, AspectParameters aspectParameters, 
       Set<ConfigMatchingProvider> configConditions, RuleClassProvider ruleClassProvider,
-      BuildConfiguration hostConfiguration)
+      BuildConfiguration hostConfiguration, NestedSetBuilder<Package> transitivePackages)
       throws DependencyEvaluationException {
 
     // Create the map from attributes to list of (target, configuration) pairs.
@@ -245,15 +248,15 @@ final class ConfiguredTargetFunction implements SkyFunction {
     }
 
     // Resolve configured target dependencies and handle errors.
-    Map<SkyKey, ConfiguredTarget> depValues =
-        resolveConfiguredTargetDependencies(env, depValueNames.values(), ctgValue.getTarget());
+    Map<SkyKey, ConfiguredTarget> depValues = resolveConfiguredTargetDependencies(env,
+        depValueNames.values(), ctgValue.getTarget(), transitivePackages);
     if (depValues == null) {
       return null;
     }
 
     // Resolve required aspects.
     ListMultimap<SkyKey, Aspect> depAspects = resolveAspectDependencies(
-        env, depValues, depValueNames.values());
+        env, depValues, depValueNames.values(), transitivePackages);
     if (depAspects == null) {
       return null;
     }
@@ -478,7 +481,8 @@ final class ConfiguredTargetFunction implements SkyFunction {
    */
   @Nullable
   private static ListMultimap<SkyKey, Aspect> resolveAspectDependencies(Environment env,
-      Map<SkyKey, ConfiguredTarget> configuredTargetMap, Iterable<Dependency> deps)
+      Map<SkyKey, ConfiguredTarget> configuredTargetMap, Iterable<Dependency> deps,
+      NestedSetBuilder<Package> transitivePackages)
       throws DependencyEvaluationException {
     ListMultimap<SkyKey, Aspect> result = ArrayListMultimap.create();
     Set<SkyKey> aspectKeys = new HashSet<>();
@@ -526,6 +530,7 @@ final class ConfiguredTargetFunction implements SkyFunction {
           return null;
         }
         result.put(depKey, aspectValue.getAspect());
+        transitivePackages.addTransitive(aspectValue.getTransitivePackages());
       }
     }
     return result;
@@ -547,7 +552,6 @@ final class ConfiguredTargetFunction implements SkyFunction {
         return false;
       }
     }
-
     return true;
   }
 
@@ -560,7 +564,8 @@ final class ConfiguredTargetFunction implements SkyFunction {
    */
   @Nullable
   static Set<ConfigMatchingProvider> getConfigConditions(Target target, Environment env,
-      SkyframeDependencyResolver resolver, TargetAndConfiguration ctgValue)
+      SkyframeDependencyResolver resolver, TargetAndConfiguration ctgValue,
+      NestedSetBuilder<Package> transitivePackages)
       throws DependencyEvaluationException {
     if (!(target instanceof Rule)) {
       return ImmutableSet.of();
@@ -601,7 +606,7 @@ final class ConfiguredTargetFunction implements SkyFunction {
     }
 
     Map<SkyKey, ConfiguredTarget> configValues =
-        resolveConfiguredTargetDependencies(env, configValueNames, target);
+        resolveConfiguredTargetDependencies(env, configValueNames, target, transitivePackages);
     if (configValues == null) {
       return null;
     }
@@ -634,7 +639,8 @@ final class ConfiguredTargetFunction implements SkyFunction {
    */
   @Nullable
   private static Map<SkyKey, ConfiguredTarget> resolveConfiguredTargetDependencies(
-      Environment env, Collection<Dependency> deps, Target target)
+      Environment env, Collection<Dependency> deps, Target target,
+      NestedSetBuilder<Package> transitivePackages)
       throws DependencyEvaluationException {
     boolean ok = !env.valuesMissing();
     String message = null;
@@ -682,6 +688,7 @@ final class ConfiguredTargetFunction implements SkyFunction {
         ok = false;
       } else {
         depValues.put(entry.getKey(), depValue.getConfiguredTarget());
+        transitivePackages.addTransitive(depValue.getTransitivePackages());
       }
     }
     if (message != null) {
@@ -707,7 +714,8 @@ final class ConfiguredTargetFunction implements SkyFunction {
   private ConfiguredTargetValue createConfiguredTarget(SkyframeBuildView view,
       Environment env, Target target, BuildConfiguration configuration,
       ListMultimap<Attribute, ConfiguredTarget> depValueMap,
-      Set<ConfigMatchingProvider> configConditions)
+      Set<ConfigMatchingProvider> configConditions,
+      NestedSetBuilder<Package> transitivePackages)
       throws ConfiguredTargetFunctionException, InterruptedException {
     StoredEventHandler events = new StoredEventHandler();
     BuildConfiguration ownerConfig = (configuration == null)
@@ -739,7 +747,8 @@ final class ConfiguredTargetFunction implements SkyFunction {
 
     try {
       return new ConfiguredTargetValue(configuredTarget,
-          filterSharedActionsAndThrowIfConflict(analysisEnvironment.getRegisteredActions()));
+          filterSharedActionsAndThrowIfConflict(analysisEnvironment.getRegisteredActions()),
+          transitivePackages.build());
     } catch (ActionConflictException e) {
       throw new ConfiguredTargetFunctionException(e);
     }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java
index 98aa992ae8..23491b1529 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java
@@ -21,8 +21,10 @@ import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.syntax.Label;
 import com.google.devtools.build.skyframe.SkyKey;
 
@@ -46,11 +48,14 @@ public final class ConfiguredTargetValue extends ActionLookupValue {
   // separate variable in order to save memory.
   @Nullable private volatile Iterable<Action> actions;
 
+  private final NestedSet<Package> transitivePackages;
+
   ConfiguredTargetValue(ConfiguredTarget configuredTarget,
-      Map<Artifact, Action> generatingActionMap) {
+      Map<Artifact, Action> generatingActionMap, NestedSet<Package> transitivePackages) {
     super(generatingActionMap);
     this.configuredTarget = configuredTarget;
     this.actions = generatingActionMap.values();
+    this.transitivePackages = transitivePackages;
   }
 
   @VisibleForTesting
@@ -64,6 +69,9 @@ public final class ConfiguredTargetValue extends ActionLookupValue {
     return Preconditions.checkNotNull(actions, configuredTarget);
   }
 
+  public NestedSet<Package> getTransitivePackages() {
+    return transitivePackages;
+  }
   /**
    * Clears configured target data from this value, leaving only the artifact->generating action
    * map.