In SkyQueryEnvironment, don't silently give up when there's a cycle in the graph. We can compute the universe target patterns outside of skyframe, which is the only reason we need the value we were requesting. Giving up was preventing us from evaluating "..." patterns even if the "..." pattern didn't contain any cycles itself.
--
MOS_MIGRATED_REVID=111605976
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 78aab4a..614f502 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
@@ -55,7 +55,7 @@
import com.google.devtools.build.lib.skyframe.GraphBackedRecursivePackageProvider;
import com.google.devtools.build.lib.skyframe.PackageLookupValue;
import com.google.devtools.build.lib.skyframe.PackageValue;
-import com.google.devtools.build.lib.skyframe.PrepareDepsOfPatternsValue;
+import com.google.devtools.build.lib.skyframe.PrepareDepsOfPatternsFunction;
import com.google.devtools.build.lib.skyframe.RecursivePackageProviderBackedTargetPatternResolver;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.TargetPatternValue;
@@ -147,21 +147,23 @@
LOG.info("Spent " + (duration / 1000 / 1000) + " ms on evaluation and walkable graph");
}
+ SkyKey universeKey = graphFactory.getUniverseKey(universeScope, parserPrefix);
+ universeTargetPatternKeys =
+ PrepareDepsOfPatternsFunction.getTargetPatternKeys(
+ PrepareDepsOfPatternsFunction.getSkyKeys(universeKey, eventHandler));
+
// The prepareAndGet call above evaluates a single PrepareDepsOfPatterns SkyKey.
// We expect to see either a single successfully evaluated value or a cycle in the result.
Collection<SkyValue> values = result.values();
if (!values.isEmpty()) {
Preconditions.checkState(values.size() == 1, "Universe query \"%s\" returned multiple"
+ " values unexpectedly (%s values in result)", universeScope, values.size());
- PrepareDepsOfPatternsValue prepareDepsOfPatternsValue =
- (PrepareDepsOfPatternsValue) Iterables.getOnlyElement(values);
- universeTargetPatternKeys = prepareDepsOfPatternsValue.getTargetPatternKeys();
+ Preconditions.checkNotNull(result.get(universeKey), result);
} else {
// No values in the result, so there must be an error. We expect the error to be a cycle.
boolean foundCycle = !Iterables.isEmpty(result.getError().getCycleInfo());
Preconditions.checkState(foundCycle, "Universe query \"%s\" failed with non-cycle error: %s",
universeScope, result.getError());
- universeTargetPatternKeys = ImmutableList.of();
}
}
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 db306d2..60ec9ca 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
@@ -39,6 +39,40 @@
*/
public class PrepareDepsOfPatternsFunction implements SkyFunction {
+ public static ImmutableList<SkyKey> getSkyKeys(SkyKey skyKey, EventHandler eventHandler) {
+ TargetPatternSequence targetPatternSequence = (TargetPatternSequence) skyKey.argument();
+ Iterable<PrepareDepsOfPatternSkyKeyOrException> keysMaybe =
+ PrepareDepsOfPatternValue.keys(targetPatternSequence.getPatterns(),
+ targetPatternSequence.getOffset());
+
+ ImmutableList.Builder<SkyKey> skyKeyBuilder = ImmutableList.builder();
+ boolean handlerIsParseFailureListener = eventHandler instanceof ParseFailureListener;
+ for (PrepareDepsOfPatternSkyKeyOrException skyKeyOrException : keysMaybe) {
+ try {
+ skyKeyBuilder.add(skyKeyOrException.getSkyKey());
+ } catch (TargetParsingException e) {
+ handleTargetParsingException(eventHandler, handlerIsParseFailureListener,
+ skyKeyOrException.getOriginalPattern(), e);
+ }
+ }
+
+ return skyKeyBuilder.build();
+ }
+
+ private static final Function<SkyKey, TargetPatternKey> SKY_TO_TARGET_PATTERN =
+ new Function<SkyKey, TargetPatternKey>() {
+ @Nullable
+ @Override
+ public TargetPatternKey apply(SkyKey skyKey) {
+ return (TargetPatternKey) skyKey.argument();
+ }
+ };
+
+ public static ImmutableList<TargetPatternKey> getTargetPatternKeys(
+ ImmutableList<SkyKey> skyKeys) {
+ return ImmutableList.copyOf(Iterables.transform(skyKeys, SKY_TO_TARGET_PATTERN));
+ }
+
/**
* Given a {@link SkyKey} that contains a sequence of target patterns, when this function returns
* {@link PrepareDepsOfPatternsValue}, then all targets matching that sequence, and those targets'
@@ -48,23 +82,7 @@
@Override
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
EventHandler eventHandler = env.getListener();
- boolean handlerIsParseFailureListener = eventHandler instanceof ParseFailureListener;
- TargetPatternSequence targetPatternSequence = (TargetPatternSequence) skyKey.argument();
-
- Iterable<PrepareDepsOfPatternSkyKeyOrException> keysMaybe =
- PrepareDepsOfPatternValue.keys(targetPatternSequence.getPatterns(),
- targetPatternSequence.getOffset());
-
- ImmutableList.Builder<SkyKey> skyKeyBuilder = ImmutableList.builder();
- for (PrepareDepsOfPatternSkyKeyOrException skyKeyOrException : keysMaybe) {
- try {
- skyKeyBuilder.add(skyKeyOrException.getSkyKey());
- } catch (TargetParsingException e) {
- handleTargetParsingException(eventHandler, handlerIsParseFailureListener,
- skyKeyOrException.getOriginalPattern(), e);
- }
- }
- ImmutableList<SkyKey> skyKeys = skyKeyBuilder.build();
+ ImmutableList<SkyKey> skyKeys = getSkyKeys(skyKey, eventHandler);
Map<SkyKey, ValueOrException<TargetParsingException>> tokensByKey =
env.getValuesOrThrow(skyKeys, TargetParsingException.class);
@@ -72,6 +90,7 @@
return null;
}
+ boolean handlerIsParseFailureListener = eventHandler instanceof ParseFailureListener;
for (SkyKey key : skyKeys) {
try {
// The only exception type throwable by PrepareDepsOfPatternFunction is
@@ -84,15 +103,7 @@
}
}
- 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);
+ return new PrepareDepsOfPatternsValue(getTargetPatternKeys(skyKeys));
}
private static void handleTargetParsingException(EventHandler eventHandler,
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 883a605..d720942 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
@@ -1444,7 +1444,7 @@
@Override
public EvaluationResult<SkyValue> prepareAndGet(Collection<String> patterns, String offset,
int numThreads, EventHandler eventHandler) throws InterruptedException {
- SkyKey skyKey = getPrepareDepsKey(patterns, offset);
+ SkyKey skyKey = getUniverseKey(patterns, offset);
EvaluationResult<SkyValue> evaluationResult =
buildDriver.evaluate(ImmutableList.of(skyKey), true, numThreads, eventHandler);
Preconditions.checkNotNull(evaluationResult.getWalkableGraph(), patterns);
@@ -1456,10 +1456,11 @@
* underlying evaluation implementation.
*/
public String prepareAndGetMetadata(Collection<String> patterns, String offset) {
- return buildDriver.meta(ImmutableList.of(getPrepareDepsKey(patterns, offset)));
+ return buildDriver.meta(ImmutableList.of(getUniverseKey(patterns, offset)));
}
- private SkyKey getPrepareDepsKey(Collection<String> patterns, String offset) {
+ @Override
+ public SkyKey getUniverseKey(Collection<String> patterns, String offset) {
return PrepareDepsOfPatternsValue.key(ImmutableList.copyOf(patterns), offset);
}
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 89fe2fd..0c073a7 100644
--- a/src/main/java/com/google/devtools/build/skyframe/WalkableGraph.java
+++ b/src/main/java/com/google/devtools/build/skyframe/WalkableGraph.java
@@ -79,5 +79,8 @@
interface WalkableGraphFactory {
EvaluationResult<SkyValue> prepareAndGet(Collection<String> roots, String offset,
int numThreads, EventHandler eventHandler) throws InterruptedException;
+
+ /** Returns the {@link SkyKey} that defines this universe. */
+ SkyKey getUniverseKey(Collection<String> roots, String offset);
}
}
