fix crash for recursive target pattern outside of universe scope
PiperOrigin-RevId: 350560818
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/BUILD b/src/main/java/com/google/devtools/build/lib/pkgcache/BUILD
index b174fd9..cf2dee9 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/BUILD
@@ -31,6 +31,7 @@
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
+ "//src/main/java/com/google/devtools/build/lib/query2/engine",
"//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java
index 316bd23..378905e 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/RecursivePackageProvider.java
@@ -24,6 +24,7 @@
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.query2.engine.QueryException;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.RootedPath;
import java.util.Map;
@@ -56,7 +57,7 @@
PathFragment directory,
ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories)
- throws InterruptedException;
+ throws InterruptedException, QueryException;
/**
* Returns the {@link Package} corresponding to each Package in "pkgIds". If any of the packages
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index 4962e08..d5eceb7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -1575,6 +1575,7 @@
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
+ "//src/main/java/com/google/devtools/build/lib/query2/engine",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
@@ -2140,6 +2141,7 @@
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
+ "//src/main/java/com/google/devtools/build/lib/query2/engine",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/protobuf:failure_details_java_proto",
"//third_party:guava",
@@ -2217,9 +2219,11 @@
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/events",
+ "//src/main/java/com/google/devtools/build/lib/query2/engine",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/skyframe",
+ "//src/main/protobuf:failure_details_java_proto",
"//third_party:guava",
],
)
@@ -2274,6 +2278,7 @@
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/events",
+ "//src/main/java/com/google/devtools/build/lib/query2/engine",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/skyframe",
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 1776a29..22bd020 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
@@ -37,6 +37,7 @@
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.pkgcache.AbstractRecursivePackageProvider;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
+import com.google.devtools.build.lib.query2.engine.QueryException;
import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
@@ -207,7 +208,7 @@
PathFragment directory,
ImmutableSet<PathFragment> ignoredSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories)
- throws InterruptedException {
+ throws InterruptedException, QueryException {
ImmutableList<Root> roots =
checkValidDirectoryAndGetRoots(
repository, directory, ignoredSubdirectories, excludedSubdirectories);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java
index 72b343a..f89e4db 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java
@@ -44,6 +44,7 @@
import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
import com.google.devtools.build.lib.pkgcache.RecursivePackageProvider;
import com.google.devtools.build.lib.pkgcache.TargetPatternResolverUtil;
+import com.google.devtools.build.lib.query2.engine.QueryException;
import com.google.devtools.build.lib.server.FailureDetails.TargetPatterns;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
@@ -253,7 +254,7 @@
pathFragment,
blacklistedSubdirectories,
excludedSubdirectories);
- } catch (TargetParsingException e) {
+ } catch (TargetParsingException | QueryException e) {
return Futures.immediateFailedFuture(e);
} catch (InterruptedException e) {
return Futures.immediateCancelledFuture();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgValueRootPackageExtractor.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgValueRootPackageExtractor.java
index ac1ea8a..580105e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgValueRootPackageExtractor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePkgValueRootPackageExtractor.java
@@ -13,7 +13,6 @@
// 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.common.collect.Iterables;
@@ -22,6 +21,8 @@
import com.google.devtools.build.lib.concurrent.BatchCallback;
import com.google.devtools.build.lib.concurrent.ParallelVisitor.UnusedException;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.query2.engine.QueryException;
+import com.google.devtools.build.lib.server.FailureDetails.Query.Code;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.RootedPath;
@@ -41,7 +42,7 @@
PathFragment directory,
ImmutableSet<PathFragment> ignoredSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories)
- throws InterruptedException {
+ throws InterruptedException, QueryException {
ImmutableSet<PathFragment> filteredIgnoredSubdirectories =
ImmutableSet.copyOf(
Iterables.filter(
@@ -49,21 +50,24 @@
path -> !path.equals(directory) && path.startsWith(directory)));
for (Root root : roots) {
- // Note: no need to check if lookup == null because it will never be null.
- // {@link RecursivePkgFunction} handles all errors in a keep_going build.
- // In a nokeep_going build, we would never reach this part of the code.
+ RootedPath rootedPath = RootedPath.toRootedPath(root, directory);
RecursivePkgValue lookup =
(RecursivePkgValue)
graph.getValue(
- RecursivePkgValue.key(
- repository,
- RootedPath.toRootedPath(root, directory),
- filteredIgnoredSubdirectories));
- Preconditions.checkState(
- lookup != null,
- "Root %s in repository %s could not be found in the graph.",
- root.asPath(),
- repository.getName());
+ RecursivePkgValue.key(repository, rootedPath, filteredIgnoredSubdirectories));
+ if (lookup == null) {
+ // A null lookup should only happen during post-analysis queries which have access to
+ // --universe_scope logic. For builds lookup should never be null because {@link
+ // RecursivePkgFunction} handles all errors in a --keep_going build. In a --nokeep_going
+ // build, we should never reach this part of the code.
+ throw new QueryException(
+ String.format(
+ "Unable to load package '%s' because package is not in scope. Check that all"
+ + " target patterns in query expression are within the --universe_scope of this"
+ + " query.",
+ rootedPath),
+ Code.TARGET_NOT_IN_UNIVERSE_SCOPE);
+ }
ImmutableList.Builder<PackageIdentifier> packageIds = ImmutableList.builder();
for (String packageName : lookup.getPackages().toList()) {
// TODO(bazel-team): Make RecursivePkgValue return NestedSet<PathFragment> so this transform
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RootPackageExtractor.java b/src/main/java/com/google/devtools/build/lib/skyframe/RootPackageExtractor.java
index b92e8e1..65a4fa2 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RootPackageExtractor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RootPackageExtractor.java
@@ -19,6 +19,7 @@
import com.google.devtools.build.lib.concurrent.BatchCallback;
import com.google.devtools.build.lib.concurrent.ParallelVisitor.UnusedException;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.query2.engine.QueryException;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.skyframe.WalkableGraph;
@@ -53,5 +54,5 @@
PathFragment directory,
ImmutableSet<PathFragment> blacklistedSubdirectories,
ImmutableSet<PathFragment> excludedSubdirectories)
- throws InterruptedException;
+ throws InterruptedException, QueryException;
}
diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java
index 807c092..4f2988d 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java
@@ -38,7 +38,8 @@
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.Setting;
import com.google.devtools.build.lib.query2.engine.QueryException;
-import com.google.devtools.build.lib.server.FailureDetails.ConfigurableQuery.Code;
+import com.google.devtools.build.lib.server.FailureDetails.ConfigurableQuery;
+import com.google.devtools.build.lib.server.FailureDetails.Query;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.vfs.Path;
import java.util.Collection;
@@ -161,7 +162,8 @@
// Test that the proper error is thrown when requesting an attribute that doesn't exist.
EvalThrowsResult evalThrowsResult = evalThrows("labels('fake_attr', //test:my_rule)", true);
- assertConfigurableQueryCode(evalThrowsResult.getFailureDetail(), Code.ATTRIBUTE_MISSING);
+ assertConfigurableQueryCode(
+ evalThrowsResult.getFailureDetail(), ConfigurableQuery.Code.ATTRIBUTE_MISSING);
assertThat(evalThrowsResult.getMessage())
.isEqualTo(
"in 'fake_attr' of rule //test:my_rule: configured target of type"
@@ -337,11 +339,13 @@
EvalThrowsResult hostResult = evalThrows("config(//test:host_dep, target)", true);
assertThat(hostResult.getMessage())
.isEqualTo("No target (in) //test:host_dep could be found in the 'target' configuration");
- assertConfigurableQueryCode(hostResult.getFailureDetail(), Code.TARGET_MISSING);
+ assertConfigurableQueryCode(
+ hostResult.getFailureDetail(), ConfigurableQuery.Code.TARGET_MISSING);
EvalThrowsResult execResult = evalThrows("config(//test:exec_dep, target)", true);
assertThat(execResult.getMessage())
.isEqualTo("No target (in) //test:exec_dep could be found in the 'target' configuration");
- assertConfigurableQueryCode(execResult.getFailureDetail(), Code.TARGET_MISSING);
+ assertConfigurableQueryCode(
+ execResult.getFailureDetail(), ConfigurableQuery.Code.TARGET_MISSING);
BuildConfiguration configuration =
getConfiguration(Iterables.getOnlyElement(eval("config(//test:dep, target)")));
@@ -361,12 +365,14 @@
EvalThrowsResult targetResult = evalThrows("config(//test:target_dep, host)", true);
assertThat(targetResult.getMessage())
.isEqualTo("No target (in) //test:target_dep could be found in the 'host' configuration");
- assertConfigurableQueryCode(targetResult.getFailureDetail(), Code.TARGET_MISSING);
+ assertConfigurableQueryCode(
+ targetResult.getFailureDetail(), ConfigurableQuery.Code.TARGET_MISSING);
assertThat(eval("config(//test:host_dep, host)")).isEqualTo(eval("//test:host_dep"));
EvalThrowsResult hostResult = evalThrows("config(//test:exec_dep, host)", true);
assertThat(hostResult.getMessage())
.isEqualTo("No target (in) //test:exec_dep could be found in the 'host' configuration");
- assertConfigurableQueryCode(hostResult.getFailureDetail(), Code.TARGET_MISSING);
+ assertConfigurableQueryCode(
+ hostResult.getFailureDetail(), ConfigurableQuery.Code.TARGET_MISSING);
BuildConfiguration configuration =
getConfiguration(Iterables.getOnlyElement(eval("config(//test:dep, host)")));
@@ -455,7 +461,8 @@
QueryException e =
assertThrows(
QueryException.class, () -> eval("config(//mytest:mytarget," + wrongPrefix + ")"));
- assertConfigurableQueryCode(e.getFailureDetail(), Code.INCORRECT_CONFIG_ARGUMENT_ERROR);
+ assertConfigurableQueryCode(
+ e.getFailureDetail(), ConfigurableQuery.Code.INCORRECT_CONFIG_ARGUMENT_ERROR);
assertThat(e)
.hasMessageThat()
.contains("config()'s second argument must identify a unique configuration");
@@ -492,6 +499,19 @@
+ "find BUILD file /workspace/parent/child/BUILD");
}
+ // Regression test for b/175739699
+ @Test
+ public void testRecursiveTargetPatternOutsideOfScopeFailsGracefully() throws Exception {
+ writeFile("testA/BUILD", "sh_library(name = 'testA')");
+ writeFile("testB/BUILD", "sh_library(name = 'testB')");
+ writeFile("testB/testC/BUILD", "sh_library(name = 'testC')");
+ helper.setUniverseScope("//testA");
+ QueryException e = assertThrows(QueryException.class, () -> eval("//testB/..."));
+ assertThat(e.getFailureDetail().getQuery().getCode())
+ .isEqualTo(Query.Code.TARGET_NOT_IN_UNIVERSE_SCOPE);
+ assertThat(e).hasMessageThat().contains("package is not in scope");
+ }
+
@Override
@Test
public void testMultipleTopLevelConfigurations_nullConfigs() throws Exception {
