Fix the classic query package-loading cutoff optimization with external workspaces.

benjaminp · copybara-github · commit 8555789dd239 · 2020-12-07T02:39:28.000-08:00
A package's path fragment is not unambiguous when external workspaces are involved. Fixes #12497. Closes #12595. PiperOrigin-RevId: 346048937
diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/BlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/query/BlazeQueryEnvironment.java
@@ -184,9 +184,9 @@ private void getTargetsMatchingPatternImpl(String pattern, Callback<Target> call
       }
     }
 
-    Set<PathFragment> packages = CompactHashSet.create();
+    Set<PackageIdentifier> packages = CompactHashSet.create();
     for (Target target : targets) {
-      packages.add(target.getLabel().getPackageFragment());
+      packages.add(target.getLabel().getPackageIdentifier());
     }
 
     for (Target target : targets) {
@@ -205,7 +205,7 @@ private void getTargetsMatchingPatternImpl(String pattern, Callback<Target> call
       } else if (target instanceof Rule) {
         Rule rule = (Rule) target;
         for (Label label : rule.getLabels(dependencyFilter)) {
-          if (!packages.contains(label.getPackageFragment())) {
+          if (!packages.contains(label.getPackageIdentifier())) {
             continue;  // don't cause additional package loading
           }
           try {
diff --git a/src/test/shell/integration/bazel_query_test.sh b/src/test/shell/integration/bazel_query_test.sh
@@ -853,4 +853,20 @@ function test_query_failure_exit_code_behavior() {
   assert_equals 7 "$exit_code"
 }
 
+function test_unnecessary_external_workspaces_not_loaded() {
+  cat > WORKSPACE <<'EOF'
+local_repository(
+    name = "notthere",
+    path = "/nope",
+)
+EOF
+  cat > BUILD <<'EOF'
+filegroup(
+    name = "something",
+    srcs = ["@notthere"],
+)
+EOF
+  bazel query '//:*' || fail "Expected success"
+}
+
 run_suite "${PRODUCT_NAME} query tests"

Original file line number	Diff line number	Diff line change
`@@ -184,9 +184,9 @@ private void getTargetsMatchingPatternImpl(String pattern, Callback<Target> call`
`184`	`184`	`}`
`185`	`185`	`}`
`186`	`186`
`187`		`- Set<PathFragment> packages = CompactHashSet.create();`
	`187`	`+ Set<PackageIdentifier> packages = CompactHashSet.create();`
`188`	`188`	`for (Target target : targets) {`
`189`		`- packages.add(target.getLabel().getPackageFragment());`
	`189`	`+ packages.add(target.getLabel().getPackageIdentifier());`
`190`	`190`	`}`
`191`	`191`
`192`	`192`	`for (Target target : targets) {`
`@@ -205,7 +205,7 @@ private void getTargetsMatchingPatternImpl(String pattern, Callback<Target> call`
`205`	`205`	`} else if (target instanceof Rule) {`
`206`	`206`	`Rule rule = (Rule) target;`
`207`	`207`	`for (Label label : rule.getLabels(dependencyFilter)) {`
`208`		`- if (!packages.contains(label.getPackageFragment())) {`
	`208`	`+ if (!packages.contains(label.getPackageIdentifier())) {`
`209`	`209`	`continue; // don't cause additional package loading`
`210`	`210`	`}`
`211`	`211`	`try {`