Remove CachingPackageLocator interface from the PackageProvider hierarchy

This simplifies the code quite a bit.

PiperOrigin-RevId: 199427943

11 files changed, 28 insertions, 53 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/AbstractRecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/pkgcache/AbstractRecursivePackageProvider.java
index 4252f1d44c..80cad9fd63 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/AbstractRecursivePackageProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/AbstractRecursivePackageProvider.java
@@ -14,36 +14,17 @@
 package com.google.devtools.build.lib.pkgcache;
 
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.vfs.Path;
 
 /** Partial implementation of RecursivePackageProvider to provide some common methods. */
 public abstract class AbstractRecursivePackageProvider implements RecursivePackageProvider {
 
-  private final PathPackageLocator pkgPath;
-
-  protected AbstractRecursivePackageProvider(PathPackageLocator pkgPath) {
-    this.pkgPath = pkgPath;
-  }
-
-  @Override
-  public Path getBuildFileForPackage(PackageIdentifier packageName) {
-    try {
-      return pkgPath.getPackageBuildFile(packageName);
-    } catch (NoSuchPackageException e) {
-      return null;
-    }
+  protected AbstractRecursivePackageProvider() {
   }
 
-  public PathPackageLocator getPkgPath() {
-    return pkgPath;
-  }
-
-
   @Override
   public Target getTarget(ExtendedEventHandler eventHandler, Label label)
       throws NoSuchPackageException, NoSuchTargetException, InterruptedException {
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageManager.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageManager.java
index 08761f32be..0bbfae7ced 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageManager.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageManager.java
@@ -14,13 +14,14 @@
 package com.google.devtools.build.lib.pkgcache;
 
 import com.google.devtools.build.lib.concurrent.ThreadSafety;
+import com.google.devtools.build.lib.packages.CachingPackageLocator;
 import java.io.PrintStream;
 
 /**
  * A PackageManager keeps state about loaded packages around for quick lookup, and provides related
  * functionality: Recursive package finding, loaded package checking, etc.
  */
-public interface PackageManager extends PackageProvider {
+public interface PackageManager extends PackageProvider, CachingPackageLocator {
   PackageManagerStatistics getAndClearStatistics();
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java
index f42ea3dd9e..e04a7c6f5f 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java
@@ -16,7 +16,6 @@ package com.google.devtools.build.lib.pkgcache;
 
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
-import com.google.devtools.build.lib.packages.CachingPackageLocator;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.Package;
 
@@ -25,7 +24,7 @@ import com.google.devtools.build.lib.packages.Package;
  *
  * <p><b>Concurrency</b>: Implementations should be thread safe for {@link #getPackage}.
  */
-public interface PackageProvider extends TargetProvider, CachingPackageLocator {
+public interface PackageProvider extends TargetProvider {
 
   /**
    * Returns the {@link Package} named "packageName". If there is no such package (e.g. {@code
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
index 71eafc303b..00a51524df 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
@@ -46,6 +46,7 @@ import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.packages.BuildType;
+import com.google.devtools.build.lib.packages.CachingPackageLocator;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.Package;
@@ -276,7 +277,8 @@ public class GenQuery implements RuleConfiguredTargetFactory {
 
     ImmutableMap<PackageIdentifier, Package> packageMap = closureInfo.first;
     ImmutableMap<Label, Target> validTargetsMap = closureInfo.second;
-    PackageProvider packageProvider = new PreloadedMapPackageProvider(packageMap, validTargetsMap);
+    PreloadedMapPackageProvider packageProvider =
+        new PreloadedMapPackageProvider(packageMap, validTargetsMap);
     TargetPatternEvaluator evaluator = new SkyframeEnvTargetPatternEvaluator(env);
     Predicate<Label> labelFilter = Predicates.in(validTargetsMap.keySet());
 
@@ -287,7 +289,7 @@ public class GenQuery implements RuleConfiguredTargetFactory {
   @Nullable
   private ByteString doQuery(
       QueryOptions queryOptions,
-      PackageProvider packageProvider,
+      PreloadedMapPackageProvider packageProvider,
       Predicate<Label> labelFilter,
       TargetPatternEvaluator evaluator,
       String query,
@@ -528,7 +530,8 @@ public class GenQuery implements RuleConfiguredTargetFactory {
   /**
    * Provide packages and targets to the query operations using precomputed transitive closure.
    */
-  private static final class PreloadedMapPackageProvider implements PackageProvider {
+  private static final class PreloadedMapPackageProvider
+      implements PackageProvider, CachingPackageLocator {
 
     private final ImmutableMap<PackageIdentifier, Package> pkgMap;
     private final ImmutableMap<Label, Target> labelToTarget;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java
index 6244cc45d3..f110ef82b6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java
@@ -52,8 +52,7 @@ public final class EnvironmentBackedRecursivePackageProvider
 
   private final Environment env;
 
-  EnvironmentBackedRecursivePackageProvider(Environment env, PathPackageLocator pkgPath) {
-    super(pkgPath);
+  EnvironmentBackedRecursivePackageProvider(Environment env) {
     this.env = env;
   }
 
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 18b2c0dd9b..86effea4ac 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
@@ -55,6 +55,7 @@ import java.util.logging.Logger;
 public final class GraphBackedRecursivePackageProvider extends AbstractRecursivePackageProvider {
 
   private final WalkableGraph graph;
+  private final ImmutableList<Root> pkgRoots;
   private final RootPackageExtractor rootPackageExtractor;
   private final ImmutableList<TargetPatternKey> universeTargetPatternKeys;
 
@@ -66,8 +67,8 @@ public final class GraphBackedRecursivePackageProvider extends AbstractRecursive
       ImmutableList<TargetPatternKey> universeTargetPatternKeys,
       PathPackageLocator pkgPath,
       RootPackageExtractor rootPackageExtractor) {
-    super(pkgPath);
     this.graph = Preconditions.checkNotNull(graph);
+    this.pkgRoots = pkgPath.getPathEntries();
     this.universeTargetPatternKeys = Preconditions.checkNotNull(universeTargetPatternKeys);
     this.rootPackageExtractor = rootPackageExtractor;
   }
@@ -193,7 +194,7 @@ public final class GraphBackedRecursivePackageProvider extends AbstractRecursive
 
     List<Root> roots = new ArrayList<>();
     if (repository.isMain()) {
-      roots.addAll(getPkgPath().getPathEntries());
+      roots.addAll(pkgRoots);
     } else {
       RepositoryDirectoryValue repositoryValue =
           (RepositoryDirectoryValue) graph.getValue(RepositoryDirectoryValue.key(repository));
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
index e8a59d958f..510fc42bfe 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
@@ -140,12 +140,12 @@ public class PrepareDepsOfPatternFunction implements SkyFunction {
 
     private final EnvironmentBackedRecursivePackageProvider packageProvider;
     private final Environment env;
-    private final PathPackageLocator pkgPath;
+    private final ImmutableList<Root> pkgRoots;
 
     public DepsOfPatternPreparer(Environment env, PathPackageLocator pkgPath) {
       this.env = env;
-      this.packageProvider = new EnvironmentBackedRecursivePackageProvider(env, pkgPath);
-      this.pkgPath = pkgPath;
+      this.packageProvider = new EnvironmentBackedRecursivePackageProvider(env);
+      this.pkgRoots = pkgPath.getPathEntries();
     }
 
     @Override
@@ -248,7 +248,7 @@ public class PrepareDepsOfPatternFunction implements SkyFunction {
           rulesOnly ? FilteringPolicies.RULES_ONLY : FilteringPolicies.NO_FILTER;
       List<Root> roots = new ArrayList<>();
       if (repository.isMain()) {
-        roots.addAll(pkgPath.getPathEntries());
+        roots.addAll(pkgRoots);
       } else {
         RepositoryDirectoryValue repositoryValue =
             (RepositoryDirectoryValue) env.getValue(RepositoryDirectoryValue.key(repository));
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 6cf2bd5d77..dea750e808 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
@@ -424,7 +424,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
         SkyFunctions.SKYLARK_IMPORTS_LOOKUP,
         newSkylarkImportLookupFunction(ruleClassProvider, pkgFactory));
     map.put(SkyFunctions.GLOB, newGlobFunction());
-    map.put(SkyFunctions.TARGET_PATTERN, new TargetPatternFunction(pkgLocator));
+    map.put(SkyFunctions.TARGET_PATTERN, new TargetPatternFunction());
     map.put(SkyFunctions.PREPARE_DEPS_OF_PATTERNS, new PrepareDepsOfPatternsFunction());
     map.put(SkyFunctions.PREPARE_DEPS_OF_PATTERN, new PrepareDepsOfPatternFunction(pkgLocator));
     map.put(
@@ -439,7 +439,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
             hardcodedBlacklistedPackagePrefixes, additionalBlacklistedPackagePrefixesFile));
     map.put(SkyFunctions.TESTS_IN_SUITE, new TestsInSuiteFunction());
     map.put(SkyFunctions.TEST_SUITE_EXPANSION, new TestSuiteExpansionFunction());
-    map.put(SkyFunctions.TARGET_PATTERN_PHASE, new TargetPatternPhaseFunction(pkgLocator));
+    map.put(SkyFunctions.TARGET_PATTERN_PHASE, new TargetPatternPhaseFunction());
     map.put(SkyFunctions.RECURSIVE_PKG, new RecursivePkgFunction(directories));
     map.put(
         SkyFunctions.PACKAGE,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java
index 87675c50e4..b0e36037ba 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java
@@ -17,6 +17,7 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.packages.CachingPackageLocator;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.Package;
@@ -39,7 +40,7 @@ import java.util.concurrent.atomic.AtomicReference;
  * <p>This is essentially a compatibility shim between the native Skyframe and non-Skyframe
  * parts of Blaze and should not be long-lived.
  */
-class SkyframePackageManager implements PackageManager {
+class SkyframePackageManager implements PackageManager, CachingPackageLocator {
 
   private final SkyframePackageLoader packageLoader;
   private final SkyframeExecutor.SkyframeTransitivePackageLoader transitiveLoader;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java
index a590cf5b49..322061fe69 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java
@@ -26,7 +26,6 @@ import com.google.devtools.build.lib.concurrent.MultisetSemaphore;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.pkgcache.AbstractRecursivePackageProvider.MissingDepException;
 import com.google.devtools.build.lib.pkgcache.ParsingFailedEvent;
-import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
 import com.google.devtools.build.lib.util.BatchCallback;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.SkyFunction;
@@ -34,7 +33,6 @@ import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicReference;
 import javax.annotation.Nullable;
 
 /**
@@ -43,10 +41,7 @@ import javax.annotation.Nullable;
  */
 public class TargetPatternFunction implements SkyFunction {
 
-  private final AtomicReference<PathPackageLocator> pkgPath;
-
-  public TargetPatternFunction(AtomicReference<PathPackageLocator> pkgPath) {
-    this.pkgPath = pkgPath;
+  public TargetPatternFunction() {
   }
 
   @Override
@@ -57,7 +52,7 @@ public class TargetPatternFunction implements SkyFunction {
     ResolvedTargets<Target> resolvedTargets;
     try {
       EnvironmentBackedRecursivePackageProvider provider =
-          new EnvironmentBackedRecursivePackageProvider(env, pkgPath.get());
+          new EnvironmentBackedRecursivePackageProvider(env);
       RecursivePackageProviderBackedTargetPatternResolver resolver =
           new RecursivePackageProviderBackedTargetPatternResolver(
               provider,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java
index be4ad076ab..c087cc369c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java
@@ -29,7 +29,6 @@ import com.google.devtools.build.lib.pkgcache.CompileOneDependencyTransformer;
 import com.google.devtools.build.lib.pkgcache.FilteringPolicies;
 import com.google.devtools.build.lib.pkgcache.LoadingPhaseRunner;
 import com.google.devtools.build.lib.pkgcache.ParsingFailedEvent;
-import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
 import com.google.devtools.build.lib.pkgcache.TargetParsingCompleteEvent;
 import com.google.devtools.build.lib.pkgcache.TargetProvider;
 import com.google.devtools.build.lib.pkgcache.TestFilter;
@@ -46,7 +45,6 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicReference;
 import javax.annotation.Nullable;
 
 /**
@@ -55,10 +53,7 @@ import javax.annotation.Nullable;
  */
 final class TargetPatternPhaseFunction implements SkyFunction {
 
-  private final AtomicReference<PathPackageLocator> pkgPath;
-
-  public TargetPatternPhaseFunction(AtomicReference<PathPackageLocator> pkgPath) {
-    this.pkgPath = pkgPath;
+  public TargetPatternPhaseFunction() {
   }
 
   @Override
@@ -82,7 +77,7 @@ final class TargetPatternPhaseFunction implements SkyFunction {
     }
 
     // Determine targets to build:
-    ResolvedTargets<Target> targets = getTargetsToBuild(env, options, pkgPath.get());
+    ResolvedTargets<Target> targets = getTargetsToBuild(env, options);
 
     // If the --build_tests_only option was specified or we want to run tests, we need to determine
     // the list of targets to test. For that, we remove manual tests and apply the command-line
@@ -222,7 +217,7 @@ final class TargetPatternPhaseFunction implements SkyFunction {
    * @param options the command-line arguments in structured form
    */
   private static ResolvedTargets<Target> getTargetsToBuild(
-      Environment env, TargetPatternPhaseKey options, PathPackageLocator pkgPath)
+      Environment env, TargetPatternPhaseKey options)
           throws InterruptedException {
     List<TargetPatternKey> patternSkyKeys = new ArrayList<>();
     ResolvedTargets.Builder<Target> builder = ResolvedTargets.builder();
@@ -286,7 +281,7 @@ final class TargetPatternPhaseFunction implements SkyFunction {
         .filter(TargetUtils.tagFilter(options.getBuildTargetFilter()))
         .build();
     if (options.getCompileOneDependency()) {
-      TargetProvider targetProvider = new EnvironmentBackedRecursivePackageProvider(env, pkgPath);
+      TargetProvider targetProvider = new EnvironmentBackedRecursivePackageProvider(env);
       try {
         return new CompileOneDependencyTransformer(targetProvider)
             .transformCompileOneDependency(env.getListener(), result);