Activate interleaved package and transitive target loading
Hooks up the recently introduced interleaved loading functions to
normal graph loading.
--
MOS_MIGRATED_REVID=97679451
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
index c10b252..9960b4a 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
@@ -148,11 +148,18 @@
/**
* Returns {@code true} iff this pattern has type {@code Type.TARGETS_BELOW_DIRECTORY} and
- * {@code containedPattern} is contained by or equals this pattern. For example,
- * returns {@code true} for {@code this = TargetPattern ("//...")} and {@code containedPattern
- * = TargetPattern ("//foo/...")}.
+ * {@param directory} is contained by or equals this pattern's directory. For example,
+ * returns {@code true} for {@code this = TargetPattern ("//...")} and {@code directory
+ * = "foo")}.
*/
- public abstract boolean containsBelowDirectory(TargetPattern containedPattern);
+ public abstract boolean containsBelowDirectory(String directory);
+
+ /**
+ * Shorthand for {@code containsBelowDirectory(containedPattern.getDirectory())}.
+ */
+ public boolean containsBelowDirectory(TargetPattern containedPattern) {
+ return containsBelowDirectory(containedPattern.getDirectory());
+ }
/**
* Returns the most specific containing directory of the patterns that could be matched by this
@@ -165,6 +172,13 @@
*/
public abstract String getDirectory();
+ /**
+ * Returns {@code true} iff this pattern has type {@code Type.TARGETS_BELOW_DIRECTORY} or
+ * {@code Type.TARGETS_IN_PACKAGE} and the target pattern suffix specified it should match
+ * rules only.
+ */
+ public abstract boolean getRulesOnly();
+
private static final class SingleTarget extends TargetPattern {
private final String targetName;
@@ -187,7 +201,7 @@
}
@Override
- public boolean containsBelowDirectory(TargetPattern containedPattern) {
+ public boolean containsBelowDirectory(String directory) {
return false;
}
@@ -197,6 +211,11 @@
}
@Override
+ public boolean getRulesOnly() {
+ return false;
+ }
+
+ @Override
public boolean equals(Object o) {
if (this == o) {
return true;
@@ -251,7 +270,7 @@
}
@Override
- public boolean containsBelowDirectory(TargetPattern containedPattern) {
+ public boolean containsBelowDirectory(String directory) {
return false;
}
@@ -262,6 +281,11 @@
}
@Override
+ public boolean getRulesOnly() {
+ return false;
+ }
+
+ @Override
public boolean equals(Object o) {
if (this == o) {
return true;
@@ -315,7 +339,7 @@
}
@Override
- public boolean containsBelowDirectory(TargetPattern containedPattern) {
+ public boolean containsBelowDirectory(String directory) {
return false;
}
@@ -325,6 +349,11 @@
}
@Override
+ public boolean getRulesOnly() {
+ return rulesOnly;
+ }
+
+ @Override
public boolean equals(Object o) {
if (this == o) {
return true;
@@ -399,12 +428,11 @@
}
@Override
- public boolean containsBelowDirectory(TargetPattern containedPattern) {
- // Note that merely checking to see if the containedPattern's string expression beginsWith
- // the TargetsBelowDirectory's directory is insufficient. "food" begins with "foo", but
- // "//foo/..." does not contain "//food/...".
- String containedDirectory = containedPattern.getDirectory() + "/";
- return containedDirectory.startsWith(directory + "/");
+ public boolean containsBelowDirectory(String containedDirectory) {
+ // Note that merely checking to see if the directory startsWith the TargetsBelowDirectory's
+ // directory is insufficient. "food" begins with "foo", but "//foo/..." does not contain
+ // "//food/...".
+ return directory.isEmpty() || (containedDirectory + "/").startsWith(directory + "/");
}
@Override
@@ -413,6 +441,11 @@
}
@Override
+ public boolean getRulesOnly() {
+ return rulesOnly;
+ }
+
+ @Override
public boolean equals(Object o) {
if (this == o) {
return true;
diff --git a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
index 8a6e5dc..d98b933 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
@@ -45,11 +45,14 @@
import com.google.devtools.build.lib.query2.engine.QueryExpression;
import com.google.devtools.build.lib.skyframe.GraphBackedRecursivePackageProvider;
import com.google.devtools.build.lib.skyframe.PackageValue;
+import com.google.devtools.build.lib.skyframe.PrepareDepsOfPatternsValue;
import com.google.devtools.build.lib.skyframe.RecursivePackageProviderBackedTargetPatternResolver;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.TargetPatternValue;
+import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey;
import com.google.devtools.build.lib.skyframe.TransitiveTargetValue;
import com.google.devtools.build.lib.syntax.Label;
+import com.google.devtools.build.skyframe.EvaluationResult;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
@@ -78,6 +81,8 @@
public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> {
private WalkableGraph graph;
+ private ImmutableList<TargetPatternKey> universeTargetPatternKeys;
+
private final BlazeTargetAccessor accessor = new BlazeTargetAccessor(this);
private final int loadingPhaseThreads;
private final WalkableGraphFactory graphFactory;
@@ -106,7 +111,29 @@
}
private void init() throws InterruptedException {
- graph = graphFactory.prepareAndGet(universeScope, loadingPhaseThreads, eventHandler);
+ EvaluationResult<SkyValue> result =
+ graphFactory.prepareAndGet(universeScope, loadingPhaseThreads, eventHandler);
+ graph = result.getWalkableGraph();
+ Collection<SkyValue> values = result.values();
+
+ // The universe query may fail if there are errors during its evaluation, e.g. because of
+ // cycles in the target graph.
+ boolean singleValueEvaluated = values.size() == 1;
+ boolean foundError = !result.errorMap().isEmpty();
+ boolean evaluationFoundCycle =
+ foundError && !Iterables.isEmpty(result.getError().getCycleInfo());
+ Preconditions.checkState(singleValueEvaluated || evaluationFoundCycle,
+ "Universe query \"%s\" unexpectedly did not result in a single value as expected (%s"
+ + " values in result) and it did not fail because of a cycle.%s",
+ universeScope, values.size(), foundError ? " Error: " + result.getError().toString() : "");
+ if (singleValueEvaluated) {
+ PrepareDepsOfPatternsValue prepareDepsOfPatternsValue =
+ (PrepareDepsOfPatternsValue) Iterables.getOnlyElement(values);
+ universeTargetPatternKeys = prepareDepsOfPatternsValue.getTargetPatternKeys();
+ } else {
+ // The error is because of a cycle, so keep going with the graph we managed to load.
+ universeTargetPatternKeys = ImmutableList.of();
+ }
}
@Override
@@ -338,7 +365,7 @@
protected Map<String, ResolvedTargets<Target>> preloadOrThrow(QueryExpression caller,
Collection<String> patterns) throws QueryException, TargetParsingException {
GraphBackedRecursivePackageProvider provider =
- new GraphBackedRecursivePackageProvider(graph);
+ new GraphBackedRecursivePackageProvider(graph, universeTargetPatternKeys);
Map<String, ResolvedTargets<Target>> result = Maps.newHashMapWithExpectedSize(patterns.size());
for (String pattern : patterns) {
SkyKey patternKey = TargetPatternValue.key(pattern,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java
index e2c83a0..c4e8c49 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java
@@ -14,8 +14,11 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.cmdline.TargetPattern;
+import com.google.devtools.build.lib.cmdline.TargetPattern.Type;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.BuildFileNotFoundException;
@@ -24,22 +27,30 @@
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.PackageIdentifier;
import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.pkgcache.FilteringPolicies;
+import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
import com.google.devtools.build.lib.pkgcache.RecursivePackageProvider;
+import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey;
import com.google.devtools.build.lib.syntax.Label;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.WalkableGraph;
-import java.util.Collections;
-
-/** A {@link RecursivePackageProvider} backed by a {@link WalkableGraph}. */
+/**
+ * A {@link RecursivePackageProvider} backed by a {@link WalkableGraph}, used by
+ * {@code SkyQueryEnvironment} to look up the packages and targets matching the universe that's
+ * been preloaded in {@code graph}.
+ * */
public final class GraphBackedRecursivePackageProvider implements RecursivePackageProvider {
private final WalkableGraph graph;
+ private final ImmutableList<TargetPatternKey> universeTargetPatternKeys;
- public GraphBackedRecursivePackageProvider(WalkableGraph graph) {
- this.graph = graph;
+ public GraphBackedRecursivePackageProvider(WalkableGraph graph,
+ ImmutableList<TargetPatternKey> universeTargetPatternKeys) {
+ this.graph = Preconditions.checkNotNull(graph);
+ this.universeTargetPatternKeys = Preconditions.checkNotNull(universeTargetPatternKeys);
}
@Override
@@ -94,20 +105,61 @@
@Override
public Iterable<PathFragment> getPackagesUnderDirectory(RootedPath directory,
ImmutableSet<PathFragment> excludedSubdirectories) {
- SkyKey recursivePackageKey = RecursivePkgValue.key(directory, excludedSubdirectories);
- if (!graph.exists(recursivePackageKey)) {
- // If the recursive package key does not exist in the graph, then it must not correspond to
- // any directory transitively containing packages, because the SkyQuery environment has
- // already loaded the universe.
- return Collections.emptyList();
+ PathFragment.checkAllPathsAreUnder(excludedSubdirectories, directory.getRelativePath());
+
+ // Find the filtering policy of a TargetsBelowDirectory pattern, if any, in the universe that
+ // contains this directory.
+ FilteringPolicy filteringPolicy = null;
+ for (TargetPatternKey patternKey : universeTargetPatternKeys) {
+ TargetPattern pattern = patternKey.getParsedPattern();
+ boolean isTBD = pattern.getType().equals(Type.TARGETS_BELOW_DIRECTORY);
+ if (isTBD && pattern.containsBelowDirectory(directory.getRelativePath().getPathString())) {
+ filteringPolicy =
+ pattern.getRulesOnly() ? FilteringPolicies.RULES_ONLY : FilteringPolicies.NO_FILTER;
+ break;
+ }
}
- // If the recursive package key exists in the graph, then it must have a value and must not
- // have an exception, because RecursivePkgFunction#compute never throws.
- RecursivePkgValue lookup =
- (RecursivePkgValue) Preconditions.checkNotNull(graph.getValue(recursivePackageKey));
- // TODO(bazel-team): Make RecursivePkgValue return NestedSet<PathFragment> so this transform is
- // unnecessary.
- return Iterables.transform(lookup.getPackages(), PathFragment.TO_PATH_FRAGMENT);
+
+ // If we found a TargetsBelowDirectory pattern in the universe that contains this directory,
+ // then we can look for packages in and under it in the graph. If we didn't find one, then the
+ // directory wasn't in the universe, so return an empty list.
+ ImmutableList.Builder<PathFragment> builder = ImmutableList.builder();
+ if (filteringPolicy != null) {
+ collectPackagesUnder(directory, excludedSubdirectories, builder, filteringPolicy);
+ }
+ return builder.build();
+ }
+
+ private void collectPackagesUnder(RootedPath directory,
+ ImmutableSet<PathFragment> excludedSubdirectories,
+ ImmutableList.Builder<PathFragment> builder, FilteringPolicy policy) {
+ SkyKey key =
+ PrepareDepsOfTargetsUnderDirectoryValue.key(directory, excludedSubdirectories, policy);
+ // If the key does not exist in the graph, because the SkyQuery environment has
+ // already loaded the universe, and we found a TargetsBelowDirectory pattern in the universe
+ // that contained it, then we know the directory does not exist in the universe.
+ if (!graph.exists(key)) {
+ return;
+ }
+
+ // If the key exists in the graph, then it must have a value and must not have an exception,
+ // because PrepareDepsOfTargetsUnderDirectoryFunction#compute never throws.
+ PrepareDepsOfTargetsUnderDirectoryValue prepDepsValue =
+ (PrepareDepsOfTargetsUnderDirectoryValue) Preconditions.checkNotNull(graph.getValue(key));
+ if (prepDepsValue.isDirectoryPackage()) {
+ builder.add(directory.getRelativePath());
+ }
+ ImmutableMap<RootedPath, Boolean> subdirectoryTransitivelyContainsPackages =
+ prepDepsValue.getSubdirectoryTransitivelyContainsPackages();
+ for (RootedPath subdirectory : subdirectoryTransitivelyContainsPackages.keySet()) {
+ if (subdirectoryTransitivelyContainsPackages.get(subdirectory)) {
+ PathFragment subdirectoryRelativePath = subdirectory.getRelativePath();
+ ImmutableSet<PathFragment> excludedSubdirectoriesBeneathThisSubdirectory =
+ PathFragment.filterPathsStartingWith(excludedSubdirectories, subdirectoryRelativePath);
+ collectPackagesUnder(subdirectory, excludedSubdirectoriesBeneathThisSubdirectory, builder,
+ policy);
+ }
+ }
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
index a4996a4..db3e7c1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
@@ -202,7 +202,7 @@
* {@link PackageLookupFunction#compute}.
*/
private static final class PackageLookupFunctionException extends SkyFunctionException {
- public PackageLookupFunctionException(NoSuchPackageException e, Transience transience) {
+ public PackageLookupFunctionException(BuildFileNotFoundException e, Transience transience) {
super(e, transience);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
index bf32843..5230d09 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.cmdline.ResolvedTargets;
@@ -62,10 +63,19 @@
throws SkyFunctionException, InterruptedException {
TargetPatternValue.TargetPatternKey patternKey =
((TargetPatternValue.TargetPatternKey) key.argument());
+
+ // DepsOfPatternPreparer below expects to be able to ignore the filtering policy from the
+ // TargetPatternKey, which should be valid because PrepareDepsOfPatternValue.keys
+ // unconditionally creates TargetPatternKeys with the NO_FILTER filtering policy. (Compare
+ // with SkyframeTargetPatternEvaluator, which can create TargetPatternKeys with other
+ // filtering policies like FILTER_TESTS or FILTER_MANUAL.) This check makes sure that the
+ // key's filtering policy is NO_FILTER as expected.
+ Preconditions.checkState(patternKey.getPolicy().equals(FilteringPolicies.NO_FILTER),
+ patternKey.getPolicy());
+
try {
TargetPattern parsedPattern = patternKey.getParsedPattern();
- DepsOfPatternPreparer preparer =
- new DepsOfPatternPreparer(env, patternKey.getPolicy(), pkgPath.get());
+ DepsOfPatternPreparer preparer = new DepsOfPatternPreparer(env, pkgPath.get());
ImmutableSet<String> excludedSubdirectories = patternKey.getExcludedSubdirectories();
parsedPattern.eval(preparer, excludedSubdirectories);
} catch (TargetParsingException e) {
@@ -105,14 +115,11 @@
private final EnvironmentBackedRecursivePackageProvider packageProvider;
private final Environment env;
- private final FilteringPolicy policy;
private final PathPackageLocator pkgPath;
- public DepsOfPatternPreparer(Environment env, FilteringPolicy policy,
- PathPackageLocator pkgPath) {
+ public DepsOfPatternPreparer(Environment env, PathPackageLocator pkgPath) {
this.env = env;
this.packageProvider = new EnvironmentBackedRecursivePackageProvider(env);
- this.policy = policy;
this.pkgPath = pkgPath;
}
@@ -153,10 +160,9 @@
@Override
public ResolvedTargets<Void> getTargetsInPackage(String originalPattern, String packageName,
boolean rulesOnly) throws TargetParsingException, InterruptedException {
- FilteringPolicy actualPolicy = rulesOnly
- ? FilteringPolicies.and(FilteringPolicies.RULES_ONLY, policy)
- : policy;
- return getTargetsInPackage(originalPattern, new PathFragment(packageName), actualPolicy);
+ FilteringPolicy policy =
+ rulesOnly ? FilteringPolicies.RULES_ONLY : FilteringPolicies.NO_FILTER;
+ return getTargetsInPackage(originalPattern, new PathFragment(packageName), policy);
}
private ResolvedTargets<Void> getTargetsInPackage(String originalPattern,
@@ -203,16 +209,15 @@
public ResolvedTargets<Void> findTargetsBeneathDirectory(String originalPattern,
String directory, boolean rulesOnly, ImmutableSet<String> excludedSubdirectories)
throws TargetParsingException, InterruptedException {
- FilteringPolicy actualPolicy = rulesOnly
- ? FilteringPolicies.and(FilteringPolicies.RULES_ONLY, policy)
- : policy;
+ FilteringPolicy policy =
+ rulesOnly ? FilteringPolicies.RULES_ONLY : FilteringPolicies.NO_FILTER;
ImmutableSet<PathFragment> excludedPathFragments =
TargetPatternResolverUtil.getPathFragments(excludedSubdirectories);
PathFragment pathFragment = TargetPatternResolverUtil.getPathFragment(directory);
for (Path root : pkgPath.getPathEntries()) {
RootedPath rootedPath = RootedPath.toRootedPath(root, pathFragment);
SkyValue token = env.getValue(PrepareDepsOfTargetsUnderDirectoryValue.key(rootedPath,
- excludedPathFragments, actualPolicy));
+ excludedPathFragments, policy));
if (token == null) {
// A null token value means there is a missing dependency, because RecursivePkgFunction
// never throws.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java
index 638b078..e180db7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java
@@ -16,7 +16,7 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
-import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
+import com.google.devtools.build.lib.pkgcache.FilteringPolicies;
import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey;
import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternSkyKeyOrException;
import com.google.devtools.build.skyframe.SkyKey;
@@ -56,14 +56,13 @@
*
* @param patterns The list of patterns, e.g. "-foo/biz...". If a pattern's first character is
* "-", it is treated as a negative pattern.
- * @param policy The filtering policy, e.g. "only return test targets"
* @param offset The offset to apply to relative target patterns.
*/
@ThreadSafe
public static Iterable<PrepareDepsOfPatternSkyKeyOrException> keys(List<String> patterns,
- FilteringPolicy policy, String offset) {
+ String offset) {
Iterable<TargetPatternSkyKeyOrException> keysMaybe =
- TargetPatternValue.keys(patterns, policy, offset);
+ TargetPatternValue.keys(patterns, FilteringPolicies.NO_FILTER, offset);
ImmutableList.Builder<PrepareDepsOfPatternSkyKeyOrException> builder = ImmutableList.builder();
for (TargetPatternSkyKeyOrException keyMaybe : keysMaybe) {
try {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunction.java
index 6653c48..26d1e30 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunction.java
@@ -13,27 +13,22 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.common.base.Function;
import com.google.common.base.Preconditions;
-import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.cmdline.ResolvedTargets;
+import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.packages.NoSuchPackageException;
-import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.pkgcache.ParseFailureListener;
+import com.google.devtools.build.lib.skyframe.PrepareDepsOfPatternValue.PrepareDepsOfPatternSkyKeyOrException;
import com.google.devtools.build.lib.skyframe.PrepareDepsOfPatternsValue.TargetPatternSequence;
import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey;
-import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternSkyKeyOrException;
-import com.google.devtools.build.lib.syntax.Label;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.ValueOrException;
-import java.util.ArrayList;
-import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
@@ -55,12 +50,13 @@
EventHandler eventHandler = env.getListener();
boolean handlerIsParseFailureListener = eventHandler instanceof ParseFailureListener;
TargetPatternSequence targetPatternSequence = (TargetPatternSequence) skyKey.argument();
- Iterable<TargetPatternSkyKeyOrException> keysMaybe =
- TargetPatternValue.keys(targetPatternSequence.getPatterns(),
- targetPatternSequence.getPolicy(), targetPatternSequence.getOffset());
+
+ Iterable<PrepareDepsOfPatternSkyKeyOrException> keysMaybe =
+ PrepareDepsOfPatternValue.keys(targetPatternSequence.getPatterns(),
+ targetPatternSequence.getOffset());
ImmutableList.Builder<SkyKey> skyKeyBuilder = ImmutableList.builder();
- for (TargetPatternSkyKeyOrException skyKeyOrException : keysMaybe) {
+ for (PrepareDepsOfPatternSkyKeyOrException skyKeyOrException : keysMaybe) {
try {
skyKeyBuilder.add(skyKeyOrException.getSkyKey());
} catch (TargetParsingException e) {
@@ -70,48 +66,33 @@
}
ImmutableList<SkyKey> skyKeys = skyKeyBuilder.build();
- Map<SkyKey, ValueOrException<TargetParsingException>> targetPatternValuesByKey =
+ Map<SkyKey, ValueOrException<TargetParsingException>> tokensByKey =
env.getValuesOrThrow(skyKeys, TargetParsingException.class);
if (env.valuesMissing()) {
return null;
}
- ResolvedTargets.Builder<Label> builder = ResolvedTargets.builder();
for (SkyKey key : skyKeys) {
try {
- // The only exception type throwable by TargetPatternFunction is TargetParsingException.
- // Therefore all ValueOrException values in the map will either be non-null or throw
- // TargetParsingException when get is called.
- TargetPatternValue resultValue = Preconditions.checkNotNull(
- (TargetPatternValue) targetPatternValuesByKey.get(key).get());
- ResolvedTargets<Label> results = resultValue.getTargets();
- if (((TargetPatternKey) key.argument()).isNegative()) {
- builder.filter(Predicates.not(Predicates.in(results.getTargets())));
- } else {
- builder.merge(results);
- }
+ // The only exception type throwable by PrepareDepsOfPatternFunction is
+ // TargetParsingException. Therefore all ValueOrException values in the map will either
+ // be non-null or throw TargetParsingException when get is called.
+ Preconditions.checkNotNull(tokensByKey.get(key).get());
} catch (TargetParsingException e) {
// If a target pattern can't be evaluated, notify the user of the problem and keep going.
handleTargetParsingException(eventHandler, handlerIsParseFailureListener, key, e);
}
}
- ResolvedTargets<Label> resolvedTargets = builder.build();
- List<SkyKey> targetKeys = new ArrayList<>();
- for (Label target : resolvedTargets.getTargets()) {
- targetKeys.add(TransitiveTargetValue.key(target));
- }
-
- // TransitiveTargetFunction can produce exceptions of types NoSuchPackageException and
- // NoSuchTargetException. However, PrepareDepsOfPatternsFunction doesn't care about any errors
- // found during transitive target loading--it just wants the graph to have loaded whatever
- // transitive target values are available.
- env.getValuesOrThrow(targetKeys, NoSuchPackageException.class, NoSuchTargetException.class);
- if (env.valuesMissing()) {
- return null;
- }
-
- return PrepareDepsOfPatternsValue.INSTANCE;
+ ImmutableList<TargetPatternKey> targetPatternKeys =
+ ImmutableList.copyOf(Iterables.transform(skyKeys,
+ new Function<SkyKey, TargetPatternKey>() {
+ @Override
+ public TargetPatternKey apply(SkyKey skyKey) {
+ return (TargetPatternKey) skyKey.argument();
+ }
+ }));
+ return new PrepareDepsOfPatternsValue(targetPatternKeys);
}
private static void handleTargetParsingException(EventHandler eventHandler,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsValue.java
index eac1b89..ec9435b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsValue.java
@@ -13,10 +13,11 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
-import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
+import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
@@ -24,53 +25,55 @@
import java.util.Objects;
/**
- * The value returned by {@link PrepareDepsOfPatternsFunction}. Because that function is
- * invoked only for its side effect (i.e. ensuring the graph contains targets matching the
- * pattern sequence and their transitive dependencies), this value carries no information.
+ * The value returned by {@link PrepareDepsOfPatternsFunction}. Although that function is
+ * invoked primarily for its side effect (i.e. ensuring the graph contains targets matching the
+ * pattern sequence and their transitive dependencies), this value contains the
+ * {@link TargetPatternKey} arguments of the {@link PrepareDepsOfPatternFunction}s evaluated in
+ * service of it.
*
- * <p>Because the returned value is always the same object, this value and the
- * {@link PrepareDepsOfPatternsFunction} which computes it are incompatible with change pruning. It
- * should only be requested by consumers who do not require reevaluation when
- * {@link PrepareDepsOfPatternsFunction} is reevaluated. Safe consumers include, e.g., top-level
- * consumers, and other functions which invoke {@link PrepareDepsOfPatternsFunction} solely for its
- * side-effects.
+ * <p>Because the returned value may remain the same when the side-effects of this function
+ * evaluation change, this value and the {@link PrepareDepsOfPatternsFunction} which computes it
+ * are incompatible with change pruning. It should only be requested by consumers who do not
+ * require reevaluation when {@link PrepareDepsOfPatternsFunction} is reevaluated. Safe consumers
+ * include, e.g., top-level consumers, and other functions which invoke {@link
+ * PrepareDepsOfPatternsFunction} solely for its side-effects and which do not behave differently
+ * depending on those side-effects.
*/
@Immutable
@ThreadSafe
public final class PrepareDepsOfPatternsValue implements SkyValue {
- public static final PrepareDepsOfPatternsValue INSTANCE = new PrepareDepsOfPatternsValue();
- private PrepareDepsOfPatternsValue() {
+ private final ImmutableList<TargetPatternKey> targetPatternKeys;
+
+ PrepareDepsOfPatternsValue(ImmutableList<TargetPatternKey> targetPatternKeys) {
+ this.targetPatternKeys = targetPatternKeys;
+ }
+
+ public ImmutableList<TargetPatternKey> getTargetPatternKeys() {
+ return targetPatternKeys;
}
@ThreadSafe
- public static SkyKey key(ImmutableList<String> patterns, FilteringPolicy policy, String offset) {
+ public static SkyKey key(ImmutableList<String> patterns, String offset) {
return new SkyKey(SkyFunctions.PREPARE_DEPS_OF_PATTERNS,
- new TargetPatternSequence(patterns, policy, offset));
+ new TargetPatternSequence(patterns, offset));
}
/** The argument value for {@link SkyKey}s of {@link PrepareDepsOfPatternsFunction}. */
@ThreadSafe
public static class TargetPatternSequence implements Serializable {
private final ImmutableList<String> patterns;
- private final FilteringPolicy policy;
private final String offset;
- private TargetPatternSequence(ImmutableList<String> patterns, FilteringPolicy policy,
- String offset) {
- this.patterns = patterns;
- this.policy = policy;
- this.offset = offset;
+ private TargetPatternSequence(ImmutableList<String> patterns, String offset) {
+ this.patterns = Preconditions.checkNotNull(patterns);
+ this.offset = Preconditions.checkNotNull(offset);
}
public ImmutableList<String> getPatterns() {
return patterns;
}
- public FilteringPolicy getPolicy() {
- return policy;
- }
-
public String getOffset() {
return offset;
}
@@ -84,13 +87,12 @@
return false;
}
TargetPatternSequence that = (TargetPatternSequence) o;
- return offset.equals(that.offset) && patterns.equals(that.patterns)
- && policy.equals(that.policy);
+ return Objects.equals(offset, that.offset) && Objects.equals(patterns, that.patterns);
}
@Override
public int hashCode() {
- return Objects.hash(patterns, policy, offset);
+ return Objects.hash(patterns, offset);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryFunction.java
index 1edf7a5..93a0ccf 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryFunction.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -86,18 +87,27 @@
@Override
protected PrepareDepsOfTargetsUnderDirectoryValue aggregateWithSubdirectorySkyValues(
MyVisitor visitor, Map<SkyKey, SkyValue> subdirectorySkyValues) {
- // Aggregate the child subdirectory package counts.
- ImmutableMap.Builder<RootedPath, Integer> builder = ImmutableMap.builder();
+ // Aggregate the child subdirectory package state.
+ ImmutableMap.Builder<RootedPath, Boolean> builder = ImmutableMap.builder();
for (SkyKey key : subdirectorySkyValues.keySet()) {
PrepareDepsOfTargetsUnderDirectoryKey prepDepsKey =
(PrepareDepsOfTargetsUnderDirectoryKey) key.argument();
PrepareDepsOfTargetsUnderDirectoryValue prepDepsValue =
(PrepareDepsOfTargetsUnderDirectoryValue) subdirectorySkyValues.get(key);
- int numPackagesInSubdirectory = (prepDepsValue.isDirectoryPackage() ? 1 : 0);
- for (Integer numPkgsInSubSub : prepDepsValue.getSubdirectoryPackageCount().values()) {
- numPackagesInSubdirectory += numPkgsInSubSub;
+ boolean packagesInSubdirectory = prepDepsValue.isDirectoryPackage();
+ // If the subdirectory isn't a package, check to see if any of its subdirectories
+ // transitively contain packages.
+ if (!packagesInSubdirectory) {
+ ImmutableCollection<Boolean> subdirectoryValues =
+ prepDepsValue.getSubdirectoryTransitivelyContainsPackages().values();
+ for (Boolean pkgsInSubSub : subdirectoryValues) {
+ if (pkgsInSubSub) {
+ packagesInSubdirectory = true;
+ break;
+ }
+ }
}
- builder.put(prepDepsKey.getRecursivePkgKey().getRootedPath(), numPackagesInSubdirectory);
+ builder.put(prepDepsKey.getRecursivePkgKey().getRootedPath(), packagesInSubdirectory);
}
return new PrepareDepsOfTargetsUnderDirectoryValue(visitor.isDirectoryPackage(),
builder.build());
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryValue.java
index 840ce7b..353bd5e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryValue.java
@@ -30,7 +30,7 @@
/**
* The value computed by {@link PrepareDepsOfTargetsUnderDirectoryFunction}. Contains a mapping for
- * all its non-excluded directories to the count of packages beneath them.
+ * all its non-excluded directories to whether there are packages beneath them.
*
* <p>This value is used by {@link GraphBackedRecursivePackageProvider#getPackagesUnderDirectory}
* to help it traverse the graph and find the set of packages under a directory, and recursively by
@@ -41,7 +41,7 @@
* part because of its side-effects (i.e. loading transitive dependencies of targets), this value
* interacts safely with change pruning, despite the fact that this value is a lossy representation
* of the packages beneath a directory (i.e. it doesn't care <b>which</b> packages are under a
- * directory, just the count of them). When the targets in a package change, the
+ * directory, just whether there are any). When the targets in a package change, the
* {@link PackageValue} that {@link PrepareDepsOfTargetsUnderDirectoryFunction} depends on will be
* invalidated, and the PrepareDeps function for that package's directory will be reevaluated,
* loading any new transitive dependencies. Change pruning may prevent the reevaluation of
@@ -49,15 +49,16 @@
*/
public final class PrepareDepsOfTargetsUnderDirectoryValue implements SkyValue {
public static final PrepareDepsOfTargetsUnderDirectoryValue EMPTY =
- new PrepareDepsOfTargetsUnderDirectoryValue(false, ImmutableMap.<RootedPath, Integer>of());
+ new PrepareDepsOfTargetsUnderDirectoryValue(false, ImmutableMap.<RootedPath, Boolean>of());
private final boolean isDirectoryPackage;
- private final ImmutableMap<RootedPath, Integer> subdirectoryPackageCount;
+ private final ImmutableMap<RootedPath, Boolean> subdirectoryTransitivelyContainsPackages;
public PrepareDepsOfTargetsUnderDirectoryValue(boolean isDirectoryPackage,
- ImmutableMap<RootedPath, Integer> subdirectoryPackageCount) {
+ ImmutableMap<RootedPath, Boolean> subdirectoryTransitivelyContainsPackages) {
this.isDirectoryPackage = isDirectoryPackage;
- this.subdirectoryPackageCount = Preconditions.checkNotNull(subdirectoryPackageCount);
+ this.subdirectoryTransitivelyContainsPackages = Preconditions.checkNotNull(
+ subdirectoryTransitivelyContainsPackages);
}
/** Whether the directory is a package (i.e. contains a BUILD file). */
@@ -66,11 +67,11 @@
}
/**
- * Returns a map from non-excluded immediate subdirectories of this directory to the number of
- * non-excluded packages under them.
+ * Returns a map from non-excluded immediate subdirectories of this directory to whether there
+ * are non-excluded packages under them.
*/
- public ImmutableMap<RootedPath, Integer> getSubdirectoryPackageCount() {
- return subdirectoryPackageCount;
+ public ImmutableMap<RootedPath, Boolean> getSubdirectoryTransitivelyContainsPackages() {
+ return subdirectoryTransitivelyContainsPackages;
}
@Override
@@ -83,12 +84,13 @@
}
PrepareDepsOfTargetsUnderDirectoryValue that = (PrepareDepsOfTargetsUnderDirectoryValue) o;
return isDirectoryPackage == that.isDirectoryPackage
- && Objects.equals(subdirectoryPackageCount, that.subdirectoryPackageCount);
+ && Objects.equals(subdirectoryTransitivelyContainsPackages,
+ that.subdirectoryTransitivelyContainsPackages);
}
@Override
public int hashCode() {
- return Objects.hash(isDirectoryPackage, subdirectoryPackageCount);
+ return Objects.hash(isDirectoryPackage, subdirectoryTransitivelyContainsPackages);
}
/** Create a prepare deps of targets under directory request. */
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index b2ed3c6..96c39ef 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -76,11 +76,9 @@
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.RuleVisibility;
import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
import com.google.devtools.build.lib.pkgcache.PackageManager;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
-import com.google.devtools.build.lib.pkgcache.TargetPatternEvaluator;
import com.google.devtools.build.lib.pkgcache.TransitivePackageLoader;
import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ActionCompletedReceiver;
import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ProgressSupplier;
@@ -109,7 +107,6 @@
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
-import com.google.devtools.build.skyframe.WalkableGraph;
import com.google.devtools.build.skyframe.WalkableGraph.WalkableGraphFactory;
import java.io.IOException;
@@ -1174,23 +1171,23 @@
/**
* For internal use in queries: performs a graph update to make sure the transitive closure of
- * the specified target {@code patterns} is present in the graph, and returns a traversable view
- * of the graph.
+ * the specified target {@code patterns} is present in the graph, and returns the {@link
+ * EvaluationResult}.
*
* <p>The graph update is unconditionally done in keep-going mode, so that the query is guaranteed
* a complete graph to work on.
*/
@Override
- public WalkableGraph prepareAndGet(Collection<String> patterns, int numThreads,
- EventHandler eventHandler) throws InterruptedException {
+ public EvaluationResult<SkyValue> prepareAndGet(Collection<String> patterns,
+ int numThreads, EventHandler eventHandler) throws InterruptedException {
SkyframeTargetPatternEvaluator patternEvaluator =
(SkyframeTargetPatternEvaluator) packageManager.getTargetPatternEvaluator();
String offset = patternEvaluator.getOffset();
- FilteringPolicy policy = TargetPatternEvaluator.DEFAULT_FILTERING_POLICY;
- SkyKey skyKey = PrepareDepsOfPatternsValue.key(ImmutableList.copyOf(patterns), policy, offset);
+ SkyKey skyKey = PrepareDepsOfPatternsValue.key(ImmutableList.copyOf(patterns), offset);
EvaluationResult<SkyValue> evaluationResult =
buildDriver.evaluate(ImmutableList.of(skyKey), true, numThreads, eventHandler);
- return Preconditions.checkNotNull(evaluationResult.getWalkableGraph(), patterns);
+ Preconditions.checkNotNull(evaluationResult.getWalkableGraph(), patterns);
+ return evaluationResult;
}
/**
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 8d2ffe8..9e21ee9 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
@@ -75,6 +75,9 @@
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);
if (targetValue == null) {
diff --git a/src/main/java/com/google/devtools/build/skyframe/WalkableGraph.java b/src/main/java/com/google/devtools/build/skyframe/WalkableGraph.java
index 181b7dc..cd87633 100644
--- a/src/main/java/com/google/devtools/build/skyframe/WalkableGraph.java
+++ b/src/main/java/com/google/devtools/build/skyframe/WalkableGraph.java
@@ -76,7 +76,7 @@
/** Provides a WalkableGraph on demand after preparing it. */
interface WalkableGraphFactory {
- WalkableGraph prepareAndGet(Collection<String> roots, int numThreads,
- EventHandler eventHandler) throws InterruptedException;
+ EvaluationResult<SkyValue> prepareAndGet(Collection<String> roots,
+ int numThreads, EventHandler eventHandler) throws InterruptedException;
}
}
