Move ExternalPackageUtil to a new lib.repository package and simplify it

Move the nested Exception classes to top-level classes, remove unused
functionality and move functionality only used in one place to that place.

PiperOrigin-RevId: 170041246

10 files changed, 120 insertions, 131 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 6bc24fa7fc..2f783f967f 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -416,6 +416,7 @@ java_library(
             "analysis/skylark/*.java",
             "analysis/test/*.java",
             "exec/*.java",
+            "repository/*.java",
             "rules/*.java",
             "rules/extra/*.java",
             "rules/filegroup/*.java",
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
index 3a9305293a..e84cd647e4 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
@@ -176,7 +176,6 @@ public class BazelRepositoryModule extends BlazeModule {
   @Override
   public void initializeRuleClasses(ConfiguredRuleClassProvider.Builder builder) {
     for (Entry<String, RepositoryFunction> handler : repositoryHandlers.entrySet()) {
-      // TODO(bazel-team): Migrate away from Class<?>
       RuleDefinition ruleDefinition;
       try {
         ruleDefinition = handler.getValue().getRuleDefinition().getDeclaredConstructor()
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java
index 45aa2bab6f..903ac93ac6 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenJarFunction.java
@@ -22,7 +22,6 @@ import com.google.devtools.build.lib.bazel.repository.MavenDownloader.JarPaths;
 import com.google.devtools.build.lib.bazel.rules.workspace.MavenJarRule;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
-import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
 import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Type;
@@ -106,7 +105,7 @@ public class MavenJarFunction extends HttpArchiveFunction {
   }
 
   private RepositoryDirectoryValue.Builder createOutputTree(Rule rule, Path outputDirectory,
-      MavenServerValue serverValue) throws RepositoryFunctionException, InterruptedException {
+      MavenServerValue serverValue) throws RepositoryFunctionException {
     Preconditions.checkState(downloader instanceof MavenDownloader);
     MavenDownloader mavenDownloader = (MavenDownloader) downloader;
 
@@ -135,7 +134,7 @@ public class MavenJarFunction extends HttpArchiveFunction {
   }
 
   private DecompressorDescriptor getDescriptorBuilder(String name, Path jar, Path outputDirectory)
-      throws RepositoryFunctionException, InterruptedException {
+      throws RepositoryFunctionException {
     return DecompressorDescriptor.builder()
         .setDecompressor(JarDecompressor.INSTANCE)
         .setTargetKind(MavenJarRule.NAME)
@@ -145,12 +144,8 @@ public class MavenJarFunction extends HttpArchiveFunction {
         .build();
   }
 
-  /**
-   * @see RepositoryFunction#getRule(RepositoryName, String, Environment)
-   */
   @Override
   public Class<? extends RuleDefinition> getRuleDefinition() {
     return MavenJarRule.class;
   }
-
 }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenServerFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenServerFunction.java
index c6c92e23a2..c0c522d697 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenServerFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/MavenServerFunction.java
@@ -19,7 +19,9 @@ import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.bazel.rules.workspace.MavenServerRule;
 import com.google.devtools.build.lib.packages.Rule;
-import com.google.devtools.build.lib.rules.ExternalPackageUtil;
+import com.google.devtools.build.lib.repository.ExternalPackageException;
+import com.google.devtools.build.lib.repository.ExternalPackageUtil;
+import com.google.devtools.build.lib.repository.ExternalRuleNotFoundException;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
 import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
 import com.google.devtools.build.lib.skyframe.FileValue;
@@ -64,13 +66,12 @@ public class MavenServerFunction implements SkyFunction {
   @Nullable
   @Override
   public SkyValue compute(SkyKey skyKey, Environment env)
-      throws InterruptedException, RepositoryFunctionException,
-          ExternalPackageUtil.ExternalPackageException {
+      throws InterruptedException, RepositoryFunctionException, ExternalPackageException {
     String repository = (String) skyKey.argument();
     Rule repositoryRule = null;
     try {
       repositoryRule = ExternalPackageUtil.getRuleByName(repository, env);
-    } catch (ExternalPackageUtil.ExternalRuleNotFoundException ex) {
+    } catch (ExternalRuleNotFoundException ex) {
       // Ignored. We throw a new one below.
     }
     if (env.valuesMissing()) {
diff --git a/src/main/java/com/google/devtools/build/lib/repository/ExternalPackageException.java b/src/main/java/com/google/devtools/build/lib/repository/ExternalPackageException.java
new file mode 100644
index 0000000000..956a6b8e8b
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/repository/ExternalPackageException.java
@@ -0,0 +1,37 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// 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.repository;
+
+import com.google.devtools.build.lib.packages.NoSuchPackageException;
+import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.skyframe.SkyFunctionException;
+import java.io.IOException;
+
+/** Exception thrown when something goes wrong accessing a rule. */
+public class ExternalPackageException extends SkyFunctionException {
+  public ExternalPackageException(NoSuchPackageException cause, Transience transience) {
+    super(cause, transience);
+  }
+
+  /** Error reading or writing to the filesystem. */
+  public ExternalPackageException(IOException cause, Transience transience) {
+    super(cause, transience);
+  }
+
+  /** For errors in WORKSPACE file rules (e.g., malformed paths or URLs). */
+  public ExternalPackageException(EvalException cause, Transience transience) {
+    super(cause, transience);
+  }
+}
\ No newline at end of file
diff --git a/src/main/java/com/google/devtools/build/lib/rules/ExternalPackageUtil.java b/src/main/java/com/google/devtools/build/lib/repository/ExternalPackageUtil.java
index 3a68b46903..33836b6f98 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/ExternalPackageUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/repository/ExternalPackageUtil.java
@@ -12,31 +12,22 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package com.google.devtools.build.lib.rules;
+package com.google.devtools.build.lib.repository;
 
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
-import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
-import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.skyframe.PackageLookupValue;
-import com.google.devtools.build.lib.skyframe.PackageValue;
 import com.google.devtools.build.lib.skyframe.WorkspaceFileValue;
-import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.RootedPath;
-import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
-import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 import com.google.devtools.build.skyframe.SkyKey;
-import java.io.IOException;
 import java.util.List;
 import javax.annotation.Nullable;
 
@@ -91,29 +82,6 @@ public class ExternalPackageUtil {
 
   /** Uses a rule name to fetch the corresponding Rule from the external package. */
   @Nullable
-  public static List<Rule> getRuleByRuleClass(final String ruleClassName, Environment env)
-      throws ExternalPackageException, InterruptedException {
-
-    List<Rule> rules =
-        getRules(
-            env,
-            false,
-            new Function<Package, Iterable<Rule>>() {
-              @Nullable
-              @Override
-              public Iterable<Rule> apply(Package externalPackage) {
-                return externalPackage.getRulesMatchingRuleClass(ruleClassName);
-              }
-            });
-
-    if (env.valuesMissing()) {
-      return null;
-    }
-    return ImmutableList.copyOf(rules);
-  }
-
-  /** Uses a rule name to fetch the corresponding Rule from the external package. */
-  @Nullable
   public static Rule getRuleByName(final String ruleName, Environment env)
       throws ExternalPackageException, InterruptedException {
 
@@ -141,80 +109,4 @@ public class ExternalPackageUtil {
     }
     return Iterables.getFirst(rules, null);
   }
-
-  @Nullable
-  public static Rule getRule(String ruleName, @Nullable String ruleClassName, Environment env)
-      throws ExternalPackageException, InterruptedException {
-    try {
-      return getRepository(RepositoryName.create("@" + ruleName), ruleClassName, env);
-    } catch (LabelSyntaxException e) {
-      throw new ExternalPackageException(
-          new IOException("Invalid rule name " + ruleName), Transience.PERSISTENT);
-    }
-  }
-
-  /**
-   * Uses a remote repository name to fetch the corresponding Rule describing how to get it. This
-   * should be called from {@link SkyFunction#compute} functions, which should return null if this
-   * returns null. If {@code ruleClassName} is set, the rule found must have a matching rule class
-   * name.
-   */
-  @Nullable
-  public static Rule getRepository(
-      RepositoryName repositoryName, @Nullable String ruleClassName, Environment env)
-      throws ExternalPackageException, InterruptedException {
-    Rule rule = getRuleByName(repositoryName.strippedName(), env);
-    Preconditions.checkState(
-        rule == null || ruleClassName == null || rule.getRuleClass().equals(ruleClassName),
-        "Got %s, was expecting a %s",
-        rule,
-        ruleClassName);
-    return rule;
-  }
-
-  /**
-   * Loads the external package and then returns the registered toolchain labels.
-   *
-   * @param env the environment to use for lookups
-   */
-  @Nullable
-  public static List<Label> getRegisteredToolchainLabels(Environment env)
-      throws ExternalPackageException, InterruptedException {
-    PackageValue externalPackageValue =
-        (PackageValue) env.getValue(PackageValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER));
-    if (externalPackageValue == null) {
-      return null;
-    }
-
-    Package externalPackage = externalPackageValue.getPackage();
-    return externalPackage.getRegisteredToolchainLabels();
-  }
-
-  /** Exception thrown when something goes wrong accessing a rule. */
-  public static class ExternalPackageException extends SkyFunctionException {
-    public ExternalPackageException(NoSuchPackageException cause, Transience transience) {
-      super(cause, transience);
-    }
-
-    /** Error reading or writing to the filesystem. */
-    public ExternalPackageException(IOException cause, Transience transience) {
-      super(cause, transience);
-    }
-
-    /** For errors in WORKSPACE file rules (e.g., malformed paths or URLs). */
-    public ExternalPackageException(EvalException cause, Transience transience) {
-      super(cause, transience);
-    }
-  }
-
-  /** Exception thrown when a rule cannot be found. */
-  public static final class ExternalRuleNotFoundException extends ExternalPackageException {
-    public ExternalRuleNotFoundException(String ruleName) {
-      super(
-          new BuildFileContainsErrorsException(
-              Label.EXTERNAL_PACKAGE_IDENTIFIER,
-              "The rule named '" + ruleName + "' could not be resolved"),
-          Transience.PERSISTENT);
-    }
-  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/repository/ExternalRuleNotFoundException.java b/src/main/java/com/google/devtools/build/lib/repository/ExternalRuleNotFoundException.java
new file mode 100644
index 0000000000..7534bcbcad
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/repository/ExternalRuleNotFoundException.java
@@ -0,0 +1,29 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// 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.repository;
+
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
+
+/** Exception thrown when a rule cannot be found. */
+public final class ExternalRuleNotFoundException extends ExternalPackageException {
+  public ExternalRuleNotFoundException(String ruleName) {
+    super(
+        new BuildFileContainsErrorsException(
+            Label.EXTERNAL_PACKAGE_IDENTIFIER,
+            "The rule named '" + ruleName + "' could not be resolved"),
+        Transience.PERSISTENT);
+  }
+}
\ No newline at end of file
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
index 15eaa374b7..eafa09fb5c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
@@ -21,7 +21,9 @@ import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleFormatter;
-import com.google.devtools.build.lib.rules.ExternalPackageUtil;
+import com.google.devtools.build.lib.repository.ExternalPackageException;
+import com.google.devtools.build.lib.repository.ExternalPackageUtil;
+import com.google.devtools.build.lib.repository.ExternalRuleNotFoundException;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
 import com.google.devtools.build.lib.skyframe.FileValue;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed;
@@ -32,6 +34,7 @@ import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.LegacySkyKey;
 import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 import com.google.devtools.build.skyframe.SkyKey;
@@ -117,8 +120,8 @@ public final class RepositoryDelegatorFunction implements SkyFunction {
 
     Rule rule;
     try {
-      rule = ExternalPackageUtil.getRepository(repositoryName, null, env);
-    } catch (ExternalPackageUtil.ExternalRuleNotFoundException e) {
+      rule = getRepository(repositoryName, env);
+    } catch (ExternalRuleNotFoundException e) {
       return RepositoryDirectoryValue.NO_SUCH_REPOSITORY_VALUE;
     }
     if (rule == null) {
@@ -224,6 +227,18 @@ public final class RepositoryDelegatorFunction implements SkyFunction {
         .setFetchingDelayed().build();
   }
 
+  /**
+   * Uses a remote repository name to fetch the corresponding Rule describing how to get it. This
+   * should be called from {@link SkyFunction#compute} functions, which should return null if this
+   * returns null.
+   */
+  @Nullable
+  private static Rule getRepository(
+      RepositoryName repositoryName, Environment env)
+      throws ExternalPackageException, InterruptedException {
+    return ExternalPackageUtil.getRuleByName(repositoryName.strippedName(), env);
+  }
+
   private String computeRuleKey(Rule rule, byte[] ruleSpecificData) {
     return new Fingerprint().addBytes(RuleFormatter.serializeRule(rule).build().toByteArray())
         .addBytes(ruleSpecificData)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
index e82ae7bd60..44395cd62c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
@@ -29,7 +29,9 @@ import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.Rule;
-import com.google.devtools.build.lib.rules.ExternalPackageUtil;
+import com.google.devtools.build.lib.repository.ExternalPackageException;
+import com.google.devtools.build.lib.repository.ExternalPackageUtil;
+import com.google.devtools.build.lib.repository.ExternalRuleNotFoundException;
 import com.google.devtools.build.lib.skyframe.ActionEnvironmentFunction;
 import com.google.devtools.build.lib.skyframe.FileStateValue.RegularFileStateValue;
 import com.google.devtools.build.lib.skyframe.FileSymlinkException;
@@ -551,14 +553,14 @@ public abstract class RepositoryFunction {
         // Invalidate external/<repo> if the repository overrides change.
         RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.get(env);
       }
-    } catch (ExternalPackageUtil.ExternalRuleNotFoundException ex) {
+    } catch (ExternalRuleNotFoundException ex) {
       // The repository we are looking for does not exist so we should depend on the whole
       // WORKSPACE file. In that case, the call to RepositoryFunction#getRuleByName(String,
       // Environment)
       // already requested all repository functions from the WORKSPACE file from Skyframe as part
       // of the resolution. Therefore we are safe to ignore that Exception.
       return;
-    } catch (ExternalPackageUtil.ExternalPackageException ex) {
+    } catch (ExternalPackageException ex) {
       // This should never happen.
       throw new IllegalStateException(
           "Repository " + repositoryName + " cannot be resolved for path " + rootedPath, ex);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java
index b7d64927bb..d8f7b1b46c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RegisteredToolchainsFunction.java
@@ -14,14 +14,14 @@
 
 package com.google.devtools.build.lib.skyframe;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.PlatformConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.platform.DeclaredToolchainInfo;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.rules.ExternalPackageUtil;
-import com.google.devtools.build.lib.rules.ExternalPackageUtil.ExternalPackageException;
+import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.ConfiguredValueCreationException;
 import com.google.devtools.build.skyframe.LegacySkyKey;
 import com.google.devtools.build.skyframe.SkyFunction;
@@ -70,14 +70,32 @@ public class RegisteredToolchainsFunction implements SkyFunction {
   }
 
   private Iterable<? extends Label> getWorkspaceToolchains(Environment env)
-      throws ExternalPackageException, InterruptedException {
-    List<Label> labels = ExternalPackageUtil.getRegisteredToolchainLabels(env);
+      throws InterruptedException {
+    List<Label> labels = getRegisteredToolchainLabels(env);
     if (labels == null) {
       return ImmutableList.of();
     }
     return labels;
   }
 
+  /**
+   * Loads the external package and then returns the registered toolchain labels.
+   *
+   * @param env the environment to use for lookups
+   */
+  @Nullable @VisibleForTesting
+  public static List<Label> getRegisteredToolchainLabels(Environment env)
+      throws InterruptedException {
+    PackageValue externalPackageValue =
+        (PackageValue) env.getValue(PackageValue.key(Label.EXTERNAL_PACKAGE_IDENTIFIER));
+    if (externalPackageValue == null) {
+      return null;
+    }
+
+    Package externalPackage = externalPackageValue.getPackage();
+    return externalPackage.getRegisteredToolchainLabels();
+  }
+
   private ImmutableList<DeclaredToolchainInfo> configureRegisteredToolchains(
       Environment env, BuildConfiguration configuration, List<Label> labels)
       throws InterruptedException, RegisteredToolchainsFunctionException {