Do not create instance of ExternalPackageBuilder

This class is a legacy remains of an actual builder and this
is a first step to clean that up.

Change-Id: Id54360641e9f779259c3819fdde286643928cca4
PiperOrigin-RevId: 173378325

src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java

@@ -24,6 +24,7 @@
 import com.google.devtools.build.lib.analysis.skylark.SkylarkAttr.Descriptor;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.packages.AttributeValueSource;
+import com.google.devtools.build.lib.packages.ExternalPackageBuilder;
 import com.google.devtools.build.lib.packages.Package.NameConflictException;
 import com.google.devtools.build.lib.packages.PackageFactory;
 import com.google.devtools.build.lib.packages.PackageFactory.PackageContext;
@@ -173,11 +174,8 @@ public Object call(
         PackageContext context = PackageFactory.getContext(env, ast);
         @SuppressWarnings("unchecked")
         Map<String, Object> attributeValues = (Map<String, Object>) args[0];
-        return context
-            .getBuilder()
-            .externalPackageData()
-            .createAndAddRepositoryRule(
-                context.getBuilder(), ruleClass, null, attributeValues, ast);
+        return ExternalPackageBuilder.createAndAddRepositoryRule(
+            context.getBuilder(), ruleClass, null, attributeValues, ast);
       } catch (InvalidRuleException | NameConflictException | LabelSyntaxException e) {
         throw new EvalException(ast.getLocation(), e.getMessage());
       }

src/main/java/com/google/devtools/build/lib/packages/ExternalPackageBuilder.java

@@ -31,7 +31,7 @@
  */
 public class ExternalPackageBuilder {
 
-  public Rule createAndAddRepositoryRule(
+  public static Rule createAndAddRepositoryRule(
       Package.Builder pkg,
       RuleClass ruleClass,
       RuleClass bindRuleClass,

src/main/java/com/google/devtools/build/lib/packages/Package.java

@@ -766,8 +766,6 @@ public void onLoadingComplete(Package pkg) {
     protected Map<Label, Path> subincludes = null;
     protected ImmutableList<Label> skylarkFileDependencies = ImmutableList.of();
 
-    protected ExternalPackageBuilder externalPackageData = new ExternalPackageBuilder();
-
     protected List<Label> registeredToolchainLabels = new ArrayList<>();
 
     /**
@@ -1396,10 +1394,6 @@ public Package finishBuild() {
       return pkg;
     }
 
-    public ExternalPackageBuilder externalPackageData() {
-      return externalPackageData;
-    }
-
     public Package build() throws InterruptedException, NoSuchPackageException {
       return build(/*discoverAssumedInputFiles=*/ true);
     }

src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java

@@ -255,10 +255,10 @@ private static boolean isLegalWorkspaceName(String name) {
   }
 
   /**
-   * Adds the various values returned by the parsing of the previous workspace file parts.
-   * {@code aPackage} is the package returned by the parent WorkspaceFileFunction, {@code importMap}
-   * is the list of load statements imports computed by the parent WorkspaceFileFunction and
-   * {@code variableBindings} the list of top level variable bindings of that same call.
+   * Adds the various values returned by the parsing of the previous workspace file parts. {@code
+   * aPackage} is the package returned by the parent WorkspaceFileFunction, {@code importMap} is the
+   * list of load statements imports computed by the parent WorkspaceFileFunction and {@code
+   * variableBindings} the list of top level variable bindings of that same call.
    */
   public void setParent(
       Package aPackage,
@@ -305,9 +305,7 @@ public void setParent(
             + "description of the project, using underscores as separators, e.g., "
             + "github.com/bazelbuild/bazel should use com_github_bazelbuild_bazel. Names must "
             + "start with a letter and can only contain letters, numbers, and underscores.",
-    parameters = {
-      @Param(name = "name", type = String.class, doc = "the name of the workspace.")
-    },
+    parameters = {@Param(name = "name", type = String.class, doc = "the name of the workspace.")},
     useAst = true,
     useEnvironment = true
   )
@@ -331,19 +329,13 @@ public Object invoke(String name, FuncallExpression ast, Environment env)
                 Package.Builder builder = PackageFactory.getContext(env, ast).pkgBuilder;
                 RuleClass localRepositoryRuleClass = ruleFactory.getRuleClass("local_repository");
                 RuleClass bindRuleClass = ruleFactory.getRuleClass("bind");
-                Map<String, Object> kwargs = ImmutableMap.<String, Object>of(
-                    "name", name, "path", ".");
+                Map<String, Object> kwargs =
+                    ImmutableMap.<String, Object>of("name", name, "path", ".");
                 try {
                   // This effectively adds a "local_repository(name = "<ws>", path = ".")"
                   // definition to the WORKSPACE file.
-                  builder
-                      .externalPackageData()
-                      .createAndAddRepositoryRule(
-                          builder,
-                          localRepositoryRuleClass,
-                          bindRuleClass,
-                          kwargs,
-                          ast);
+                  ExternalPackageBuilder.createAndAddRepositoryRule(
+                      builder, localRepositoryRuleClass, bindRuleClass, kwargs, ast);
                 } catch (InvalidRuleException | NameConflictException | LabelSyntaxException e) {
                   throw new EvalException(ast.getLocation(), e.getMessage());
                 }
@@ -374,15 +366,13 @@ public Object invoke(String name, String actual, FuncallExpression ast, Environm
           try {
             Package.Builder builder = PackageFactory.getContext(env, ast).pkgBuilder;
             RuleClass ruleClass = ruleFactory.getRuleClass("bind");
-            builder
-                .externalPackageData()
-                .addBindRule(
-                    builder,
-                    ruleClass,
-                    nameLabel,
-                    actual == null ? null : Label.parseAbsolute(actual),
-                    ast.getLocation(),
-                    ruleFactory.getAttributeContainer(ruleClass));
+            ExternalPackageBuilder.addBindRule(
+                builder,
+                ruleClass,
+                nameLabel,
+                actual == null ? null : Label.parseAbsolute(actual),
+                ast.getLocation(),
+                ruleFactory.getAttributeContainer(ruleClass));
           } catch (RuleFactory.InvalidRuleException
               | Package.NameConflictException
               | LabelSyntaxException e) {
@@ -474,16 +464,15 @@ public Object invoke(Map<String, Object> kwargs, FuncallExpression ast, Environm
           RuleClass ruleClass = ruleFactory.getRuleClass(ruleClassName);
           RuleClass bindRuleClass = ruleFactory.getRuleClass("bind");
           Rule rule =
-              builder
-                  .externalPackageData()
-                  .createAndAddRepositoryRule(builder, ruleClass, bindRuleClass, kwargs, ast);
+              ExternalPackageBuilder.createAndAddRepositoryRule(
+                  builder, ruleClass, bindRuleClass, kwargs, ast);
           if (!isLegalWorkspaceName(rule.getName())) {
             throw new EvalException(
                 ast.getLocation(), rule + "'s name field must be a legal workspace name");
           }
-        } catch (
-            RuleFactory.InvalidRuleException | Package.NameConflictException | LabelSyntaxException
-                e) {
+        } catch (RuleFactory.InvalidRuleException
+            | Package.NameConflictException
+            | LabelSyntaxException e) {
           throw new EvalException(ast.getLocation(), e.getMessage());
         }
         return NONE;

src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java

@@ -23,6 +23,7 @@
 import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.ExternalPackageBuilder;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
@@ -84,10 +85,8 @@ protected void setUpContextForRule(Map<String, Object> kwargs, Attribute... attr
         new FuncallExpression(new Identifier("test"), ImmutableList.<Passed>of());
     ast.setLocation(Location.BUILTIN);
     Rule rule =
-        packageBuilder
-            .externalPackageData()
-            .createAndAddRepositoryRule(
-                packageBuilder, buildRuleClass(attributes), null, kwargs, ast);
+        ExternalPackageBuilder.createAndAddRepositoryRule(
+            packageBuilder, buildRuleClass(attributes), null, kwargs, ast);
     HttpDownloader downloader = Mockito.mock(HttpDownloader.class);
     context =
         new SkylarkRepositoryContext(