Clean up ConfiguredTargetValueAccessor and ConfiguredTargetAccessor

katre · copybara-github · commit 27e15ad11410 · 2020-11-24T16:20:37.000-08:00
- Fix looking up a target from the walkable graph. - This prevents ConfiguredTargetValueAccessor from needing to look into ConfiguredTargetAccessor. - And removes duplicated code. - Move CTVA into the correct package. Part of bazelbuild#11993. Closes bazelbuild#12549. PiperOrigin-RevId: 344151199
diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphQueryEnvironment.java
@@ -30,7 +30,6 @@
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.pkgcache.PackageManager;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
-import com.google.devtools.build.lib.query2.ConfiguredTargetValueAccessor;
 import com.google.devtools.build.lib.query2.NamedThreadSafeOutputFormatterCallback;
 import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment;
 import com.google.devtools.build.lib.query2.SkyQueryEnvironment;
@@ -102,7 +101,7 @@ public ActionGraphQueryEnvironment(
                 .build();
     this.accessor =
         new ConfiguredTargetValueAccessor(
-            walkableGraphSupplier.get(), this.configuredTargetKeyExtractor);
+            walkableGraphSupplier.get(), this::getTarget, this.configuredTargetKeyExtractor);
   }
 
   public ActionGraphQueryEnvironment(
diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/AqueryThreadsafeCallback.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/AqueryThreadsafeCallback.java
@@ -14,7 +14,6 @@
 package com.google.devtools.build.lib.query2.aquery;
 
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
-import com.google.devtools.build.lib.query2.ConfiguredTargetValueAccessor;
 import com.google.devtools.build.lib.query2.NamedThreadSafeOutputFormatterCallback;
 import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetValue;
diff --git a/src/main/java/com/google/devtools/build/lib/query2/aquery/ConfiguredTargetValueAccessor.java b/src/main/java/com/google/devtools/build/lib/query2/aquery/ConfiguredTargetValueAccessor.java
@@ -11,7 +11,7 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
-package com.google.devtools.build.lib.query2;
+package com.google.devtools.build.lib.query2.aquery;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
@@ -21,9 +21,10 @@
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.TargetUtils;
-import com.google.devtools.build.lib.query2.cquery.ConfiguredTargetAccessor;
 import com.google.devtools.build.lib.query2.engine.KeyExtractor;
 import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor;
+import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetLookup;
+import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetNotFoundException;
 import com.google.devtools.build.lib.query2.engine.QueryException;
 import com.google.devtools.build.lib.query2.engine.QueryExpression;
 import com.google.devtools.build.lib.query2.engine.QueryVisibility;
@@ -48,13 +49,16 @@
 public class ConfiguredTargetValueAccessor implements TargetAccessor<ConfiguredTargetValue> {
 
   private final WalkableGraph walkableGraph;
+  private final TargetLookup targetLookup;
   private final KeyExtractor<ConfiguredTargetValue, ConfiguredTargetKey>
       configuredTargetKeyExtractor;
 
   public ConfiguredTargetValueAccessor(
       WalkableGraph walkableGraph,
+      TargetLookup targetLookup,
       KeyExtractor<ConfiguredTargetValue, ConfiguredTargetKey> configuredTargetKeyExtractor) {
     this.walkableGraph = walkableGraph;
+    this.targetLookup = targetLookup;
     this.configuredTargetKeyExtractor = configuredTargetKeyExtractor;
   }
 
@@ -138,8 +142,15 @@ public ImmutableSet<QueryVisibility<ConfiguredTargetValue>> getVisibility(
   }
 
   private Target getTargetFromConfiguredTargetValue(ConfiguredTargetValue configuredTargetValue) {
-    return ConfiguredTargetAccessor.getTargetFromConfiguredTarget(
-        configuredTargetValue.getConfiguredTarget(), walkableGraph);
+    // Dereference any aliases that might be present.
+    Label label = configuredTargetValue.getConfiguredObject().getOriginalLabel();
+    try {
+      return targetLookup.getTarget(label);
+    } catch (InterruptedException e) {
+      throw new IllegalStateException("Thread interrupted in the middle of getting a Target.", e);
+    } catch (TargetNotFoundException e) {
+      throw new IllegalStateException("Unable to get target from package in accessor.", e);
+    }
   }
 
   /** Returns the AspectValues that are attached to the given configuredTarget. */
diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java
@@ -32,19 +32,18 @@
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper;
 import com.google.devtools.build.lib.packages.ExecGroup;
-import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor;
+import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetNotFoundException;
 import com.google.devtools.build.lib.query2.engine.QueryException;
 import com.google.devtools.build.lib.query2.engine.QueryExpression;
 import com.google.devtools.build.lib.query2.engine.QueryVisibility;
 import com.google.devtools.build.lib.server.FailureDetails.ConfigurableQuery;
 import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetValue;
-import com.google.devtools.build.lib.skyframe.PackageValue;
 import com.google.devtools.build.lib.skyframe.ToolchainContextKey;
 import com.google.devtools.build.lib.skyframe.UnloadedToolchainContext;
 import com.google.devtools.build.skyframe.WalkableGraph;
@@ -169,25 +168,15 @@ public ImmutableSet<QueryVisibility<ConfiguredTarget>> getVisibility(
   }
 
   public Target getTargetFromConfiguredTarget(ConfiguredTarget configuredTarget) {
-    return getTargetFromConfiguredTarget(configuredTarget, walkableGraph);
-  }
-
-  public static Target getTargetFromConfiguredTarget(
-      ConfiguredTarget configuredTarget, WalkableGraph walkableGraph) {
-    Target target = null;
+    // Dereference any aliases that might be present.
+    Label label = configuredTarget.getOriginalLabel();
     try {
-      // Dereference any aliases that might be present.
-      Label label = configuredTarget.getOriginalLabel();
-      target =
-          ((PackageValue) walkableGraph.getValue(PackageValue.key(label.getPackageIdentifier())))
-              .getPackage()
-              .getTarget(label.getName());
-    } catch (NoSuchTargetException e) {
+      return queryEnvironment.getTarget(label);
+    } catch (InterruptedException e) {
+      throw new IllegalStateException("Thread interrupted in the middle of getting a Target.", e);
+    } catch (TargetNotFoundException e) {
       throw new IllegalStateException("Unable to get target from package in accessor.", e);
-    } catch (InterruptedException e2) {
-      throw new IllegalStateException("Thread interrupted in the middle of getting a Target.", e2);
     }
-    return target;
   }
 
   /** Returns the rule that generates the given output file. */
diff --git a/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java
@@ -17,7 +17,9 @@
 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.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.util.DetailedExitCode;
 import java.util.Collection;
 import java.util.List;
@@ -159,6 +161,12 @@ public final FilteringQueryFunction asFilteringFunction() {
     public abstract int getExpressionToFilterIndex();
   }
 
+  /** Functional interface for classes that need to look up a Target from a Label. */
+  @FunctionalInterface
+  interface TargetLookup {
+    Target getTarget(Label label) throws TargetNotFoundException, InterruptedException;
+  }
+
   /**
    * Exception type for the case where a target cannot be found. It's basically a wrapper for
    * whatever exception is internally thrown.
