From d90f8d2f388893eae39c724f145097f6c6bf0d57 Mon Sep 17 00:00:00 2001
From: tanzhengwei <tanzhengwei@google.com>
Date: Tue, 8 Dec 2020 11:05:07 -0800
Subject: [PATCH] Automated rollback of commit
 ba5bf5f794750e47f3ccb7b349f5ed8789324fb2.

*** Reason for rollback ***

Uses ImmutableSet that breaks validation of targets when it attempts to remove the target from the immutable iterator when invalid.

*** Original change description ***

Add scope validation for GraphlessBlazeQueryEnvironment.

PiperOrigin-RevId: 346365154
---
 .../common/AbstractBlazeQueryEnvironment.java    | 16 ----------------
 .../lib/query2/query/BlazeQueryEnvironment.java  | 14 +++++++++++++-
 .../query/GraphlessBlazeQueryEnvironment.java    |  3 ---
 3 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/query2/common/AbstractBlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/common/AbstractBlazeQueryEnvironment.java
index 2e49f801cf59b0..0dea641b92b748 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/common/AbstractBlazeQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/common/AbstractBlazeQueryEnvironment.java
@@ -16,7 +16,6 @@
 import static java.util.concurrent.TimeUnit.MINUTES;
 
 import com.google.common.base.Preconditions;
-import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
@@ -46,7 +45,6 @@
 import com.google.devtools.build.lib.util.DetailedExitCode;
 import java.io.IOException;
 import java.util.Collection;
-import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
@@ -279,20 +277,6 @@ public Map<Label, Target> getTargets(Iterable<Label> labels)
     return resultBuilder.build();
   }
 
-  protected void validateScopeOfTargets(Set<Target> targets) throws QueryException {
-    // Sets.filter would be more convenient here, but can't deal with exceptions.
-    if (labelFilter != Predicates.<Label>alwaysTrue()) {
-      // The labelFilter is always true for bazel query; it's only used for genquery rules.
-      Iterator<Target> targetIterator = targets.iterator();
-      while (targetIterator.hasNext()) {
-        Target target = targetIterator.next();
-        if (!validateScope(target.getLabel(), strictScope)) {
-          targetIterator.remove();
-        }
-      }
-    }
-  }
-
   protected boolean validateScope(Label label, boolean strict) throws QueryException {
     if (!labelFilter.test(label)) {
       String error = String.format("target '%s' is not within the scope of the query", label);
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
index 8312c340786458..05fc3bd43fab9d 100644
--- 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
@@ -17,6 +17,7 @@
 
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
@@ -62,6 +63,7 @@
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
@@ -170,7 +172,17 @@ private void getTargetsMatchingPatternImpl(String pattern, Callback<Target> call
     // becomes quadratic in runtime.
     Set<Target> targets = new LinkedHashSet<>(resolvedTargetPatterns.get(pattern));
 
-    validateScopeOfTargets(targets);
+    // Sets.filter would be more convenient here, but can't deal with exceptions.
+    if (labelFilter != Predicates.<Label>alwaysTrue()) {
+      // The labelFilter is always true for bazel query; it's only used for genquery rules.
+      Iterator<Target> targetIterator = targets.iterator();
+      while (targetIterator.hasNext()) {
+        Target target = targetIterator.next();
+        if (!validateScope(target.getLabel(), strictScope)) {
+          targetIterator.remove();
+        }
+      }
+    }
 
     Set<PackageIdentifier> packages = CompactHashSet.create();
     for (Target target : targets) {
diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/GraphlessBlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/query/GraphlessBlazeQueryEnvironment.java
index 905c8960c351d8..1b4ca951920387 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/query/GraphlessBlazeQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/query/GraphlessBlazeQueryEnvironment.java
@@ -13,7 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.query2.query;
 
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
@@ -154,8 +153,6 @@ public QueryTaskFuture<Void> getTargetsMatchingPattern(
 
   private void getTargetsMatchingPatternImpl(String pattern, Callback<Target> callback)
       throws QueryException, InterruptedException {
-    Set<Target> targets = ImmutableSet.copyOf(resolvedTargetPatterns.get(pattern));
-    validateScopeOfTargets(targets);
     callback.process(resolvedTargetPatterns.get(pattern));
   }