Avoid LoadedPackageProvider in the PackageProviderForConfigs type hierarchy.

The PackageProviderForConfigurations is not a provider of loaded packages;
it loads packages unconditionally, and is executed only within SkyFunction
evaluation domains. I.e., the old type hierarchy made no sense.

Also updated the documentation and renamed getLoadedTarget to getTarget. Note
that getLoadedPackage was never called, so I removed it.

This is in preparation for removing LoadedPackageProvider.

--
MOS_MIGRATED_REVID=103754939

4 files changed, 40 insertions, 29 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/PackageProviderForConfigurations.java b/src/main/java/com/google/devtools/build/lib/analysis/config/PackageProviderForConfigurations.java
index d6846ef5d5..a5f12b554f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/PackageProviderForConfigurations.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/PackageProviderForConfigurations.java
@@ -15,16 +15,19 @@ package com.google.devtools.build.lib.analysis.config;
 
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.packages.NoSuchPackageException;
+import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.Package;
-import com.google.devtools.build.lib.pkgcache.LoadedPackageProvider;
+import com.google.devtools.build.lib.packages.Target;
 
 import java.io.IOException;
 
 /**
- * Extended LoadedPackageProvider which is used during a creation of BuildConfiguration.Fragments.
+ * A variant of PackageProvider which is used during a creation of BuildConfiguration.Fragments.
  */
-public interface PackageProviderForConfigurations extends LoadedPackageProvider {
+public interface PackageProviderForConfigurations {
   /**
    * Adds dependency to fileName if needed. Used only in skyframe, for creating correct dependencies
    * for {@link com.google.devtools.build.lib.skyframe.ConfigurationCollectionValue}.
@@ -46,4 +49,15 @@ public interface PackageProviderForConfigurations extends LoadedPackageProvider
    * Returns true if any dependency is missing (value of some node hasn't been evaluated yet).
    */
   boolean valuesMissing();
+
+  /**
+   * Returns the Target identified by "label", loading, parsing and evaluating the package if it is
+   * not already loaded. May return {@code null} if the corresponding Skyframe entry requires
+   * function evaluation.
+   *
+   * @throws NoSuchPackageException if the package could not be found
+   * @throws NoSuchTargetException if the package was loaded successfully, but
+   *         the specified {@link Target} was not found in it
+   */
+  Target getTarget(Label label) throws NoSuchPackageException, NoSuchTargetException;
 }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
index e25faa63de..c05d6994b0 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
@@ -60,13 +60,13 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact
   public BuildConfiguration createConfigurations(
       ConfigurationFactory configurationFactory,
       Cache<String, BuildConfiguration> cache,
-      PackageProviderForConfigurations loadedPackageProvider,
+      PackageProviderForConfigurations packageProvider,
       BuildOptions buildOptions,
       EventHandler errorEventListener,
       boolean performSanityCheck) throws InvalidConfigurationException {
     // Target configuration
     BuildConfiguration targetConfiguration = configurationFactory.getConfiguration(
-        loadedPackageProvider, buildOptions, false, cache);
+        packageProvider, buildOptions, false, cache);
     if (targetConfiguration == null) {
       return null;
     }
@@ -77,7 +77,7 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact
     // Note that this passes in the dataConfiguration, not the target
     // configuration. This is intentional.
     BuildConfiguration hostConfiguration = getHostConfigurationFromRequest(configurationFactory,
-        loadedPackageProvider, dataConfiguration, buildOptions, cache);
+        packageProvider, dataConfiguration, buildOptions, cache);
     if (hostConfiguration == null) {
       return null;
     }
@@ -99,11 +99,11 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact
 
       for (BuildOptions splitOptions : splitOptionsList) {
         BuildConfiguration splitConfig = configurationFactory.getConfiguration(
-            loadedPackageProvider, splitOptions, false, cache);
+            packageProvider, splitOptions, false, cache);
         splitTransitionsTable.put(transition, splitConfig);
       }
     }
-    if (loadedPackageProvider.valuesMissing()) {
+    if (packageProvider.valuesMissing()) {
       return null;
     }
 
@@ -115,7 +115,7 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact
       // We allow the package provider to be null for testing.
       for (Label label : buildOptions.getAllLabels().values()) {
         try {
-          collectTransitiveClosure(loadedPackageProvider, reachableLabels, label);
+          collectTransitiveClosure(packageProvider, reachableLabels, label);
         } catch (NoSuchThingException e) {
           // We've loaded the transitive closure of the labels-to-load above, and made sure that
           // there are no errors loading it, so this can't happen.
@@ -265,12 +265,12 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact
     }
   }
 
-  private void collectTransitiveClosure(PackageProviderForConfigurations loadedPackageProvider,
+  private void collectTransitiveClosure(PackageProviderForConfigurations packageProvider,
       Set<Label> reachableLabels, Label from) throws NoSuchThingException {
     if (!reachableLabels.add(from)) {
       return;
     }
-    Target fromTarget = loadedPackageProvider.getLoadedTarget(from);
+    Target fromTarget = packageProvider.getTarget(from);
     if (fromTarget instanceof Rule) {
       Rule rule = (Rule) fromTarget;
       if (rule.getRuleClassObject().hasAttr("srcs", BuildType.LABEL_LIST)) {
@@ -281,7 +281,7 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact
         for (List<Label> labelsForConfiguration :
             AggregatingAttributeMapper.of(rule).visitAttribute("srcs", BuildType.LABEL_LIST)) {
           for (Label label : labelsForConfiguration) {
-            collectTransitiveClosure(loadedPackageProvider, reachableLabels, label);
+            collectTransitiveClosure(packageProvider, reachableLabels, label);
           }
         }
       }
@@ -289,7 +289,7 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact
       if (rule.getRuleClass().equals("bind")) {
         Label actual = AggregatingAttributeMapper.of(rule).get("actual", BuildType.LABEL);
         if (actual != null) {
-          collectTransitiveClosure(loadedPackageProvider, reachableLabels, actual);
+          collectTransitiveClosure(packageProvider, reachableLabels, actual);
         }
       }
     }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java
index 65d77a9227..524d8034ad 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java
@@ -57,9 +57,9 @@ public class ConfigurationFragmentFunction implements SkyFunction {
     BuildOptions buildOptions = configurationFragmentKey.getBuildOptions();
     ConfigurationFragmentFactory factory = getFactory(configurationFragmentKey.getFragmentType());
     try {
-      PackageProviderForConfigurations loadedPackageProvider = 
+      PackageProviderForConfigurations packageProvider = 
           new SkyframePackageLoaderWithValueEnvironment(env);
-      ConfigurationEnvironment confEnv = new ConfigurationBuilderEnvironment(loadedPackageProvider);
+      ConfigurationEnvironment confEnv = new ConfigurationBuilderEnvironment(packageProvider);
       Fragment fragment = factory.create(confEnv, buildOptions);
       
       if (env.valuesMissing()) {
@@ -95,23 +95,22 @@ public class ConfigurationFragmentFunction implements SkyFunction {
    * A {@link ConfigurationEnvironment} implementation that can create dependencies on files.
    */
   private final class ConfigurationBuilderEnvironment implements ConfigurationEnvironment {
-    private final PackageProviderForConfigurations loadedPackageProvider;
+    private final PackageProviderForConfigurations packageProvider;
 
-    ConfigurationBuilderEnvironment(
-        PackageProviderForConfigurations loadedPackageProvider) {
-      this.loadedPackageProvider = loadedPackageProvider;
+    ConfigurationBuilderEnvironment(PackageProviderForConfigurations packageProvider) {
+      this.packageProvider = packageProvider;
     }
 
     @Override
     public Target getTarget(Label label) throws NoSuchPackageException, NoSuchTargetException {
-      return loadedPackageProvider.getLoadedTarget(label);
+      return packageProvider.getTarget(label);
     }
 
     @Override
     public Path getPath(Package pkg, String fileName) {
       Path result = pkg.getPackageDirectory().getRelative(fileName);
       try {
-        loadedPackageProvider.addDependency(pkg, fileName);
+        packageProvider.addDependency(pkg, fileName);
       } catch (IOException | LabelSyntaxException e) {
         return null;
       }
@@ -121,12 +120,12 @@ public class ConfigurationFragmentFunction implements SkyFunction {
     @Override
     public <T extends Fragment> T getFragment(BuildOptions buildOptions, Class<T> fragmentType) 
         throws InvalidConfigurationException {
-      return loadedPackageProvider.getFragment(buildOptions, fragmentType);
+      return packageProvider.getFragment(buildOptions, fragmentType);
     }
 
     @Override
     public BlazeDirectories getBlazeDirectories() {
-      return loadedPackageProvider.getDirectories();
+      return packageProvider.getDirectories();
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java
index d566c7667d..ae2dd8dd14 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageLoaderWithValueEnvironment.java
@@ -38,16 +38,14 @@ import java.io.IOException;
  * {@link SkyFunction.Environment#getValue} instead of {@link MemoizingEvaluator#evaluate}
  * for node evaluation
  */
-class SkyframePackageLoaderWithValueEnvironment implements
-    PackageProviderForConfigurations {
+class SkyframePackageLoaderWithValueEnvironment implements PackageProviderForConfigurations {
   private final SkyFunction.Environment env;
 
   public SkyframePackageLoaderWithValueEnvironment(SkyFunction.Environment env) {
     this.env = env;
   }
 
-  @Override
-  public Package getLoadedPackage(final PackageIdentifier pkgIdentifier)
+  private Package getPackage(final PackageIdentifier pkgIdentifier)
       throws NoSuchPackageException {
     SkyKey key = PackageValue.key(pkgIdentifier);
     PackageValue value = (PackageValue) env.getValueOrThrow(key, NoSuchPackageException.class);
@@ -58,9 +56,9 @@ class SkyframePackageLoaderWithValueEnvironment implements
   }
 
   @Override
-  public Target getLoadedTarget(Label label) throws NoSuchPackageException,
+  public Target getTarget(Label label) throws NoSuchPackageException,
       NoSuchTargetException {
-    Package pkg = getLoadedPackage(label.getPackageIdentifier());
+    Package pkg = getPackage(label.getPackageIdentifier());
     return pkg == null ? null : pkg.getTarget(label.getName());
   }