Use TopLevelArtifactProvider directly only within the lib/analysis package. Instead of instantiating it manually, add .addOutputGroup() method on RuleConfiguredTargetBuilder.

This does away with the awkward repetition of the source jars output group in android_binary. Note that TopLevelArtifactProvider needs to remain public because it's a provider class.

--
MOS_MIGRATED_REVID=86584080

5 files changed, 28 insertions, 35 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index 3e9ddc5958..f0742ea96f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -66,6 +66,8 @@ public final class RuleConfiguredTargetBuilder {
   private final Map<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider> providers =
       new LinkedHashMap<>();
   private final ImmutableMap.Builder<String, Object> skylarkProviders = ImmutableMap.builder();
+  private final ImmutableMap.Builder<String, NestedSet<Artifact>> outputGroups =
+      ImmutableMap.builder();
 
   /** These are supported by all configured targets and need to be specially handled. */
   private NestedSet<Artifact> filesToBuild = NestedSetBuilder.emptySet(Order.STABLE_ORDER);
@@ -91,6 +93,11 @@ public final class RuleConfiguredTargetBuilder {
       return null;
     }
 
+    ImmutableMap<String, NestedSet<Artifact>> outputGroupMap = outputGroups.build();
+    if (!outputGroupMap.isEmpty()) {
+      add(TopLevelArtifactProvider.class, new TopLevelArtifactProvider(outputGroupMap));
+    }
+
     FilesToRunProvider filesToRunProvider = new FilesToRunProvider(ruleContext.getLabel(),
         RuleContext.getFilesToRun(runfilesSupport, filesToBuild), runfilesSupport, executable);
     add(FileProvider.class, new FileProvider(ruleContext.getLabel(), filesToBuild));
@@ -396,6 +403,22 @@ public final class RuleConfiguredTargetBuilder {
   }
 
   /**
+   * Add an output group.
+   */
+  public RuleConfiguredTargetBuilder addOutputGroup(String name, NestedSet<Artifact> artifacts) {
+    outputGroups.put(name, artifacts);
+    return this;
+  }
+
+  /**
+   * Add an output group.
+   */
+  public RuleConfiguredTargetBuilder addOutputGroup(String name, Artifact artifact) {
+    outputGroups.put(name, NestedSetBuilder.create(Order.STABLE_ORDER, artifact));
+    return this;
+  }
+
+  /**
    * Set the baseline coverage Artifacts.
    */
   public RuleConfiguredTargetBuilder setBaselineCoverageArtifacts(
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactProvider.java
index b3105846c5..bab5d4a1de 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactProvider.java
@@ -19,8 +19,6 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 
-import java.util.Map;
-
 /**
  * {@code ConfiguredTarget}s implementing this interface can provide artifacts that <b>can</b> be
  * built when the target is mentioned on the command line (as opposed to being always built, like
@@ -34,32 +32,10 @@ import java.util.Map;
 public final class TopLevelArtifactProvider implements TransitiveInfoProvider {
   private final ImmutableMap<String, NestedSet<Artifact>> outputGroups;
 
-  private TopLevelArtifactProvider(ImmutableMap<String, NestedSet<Artifact>> outputGroups) {
+  TopLevelArtifactProvider(ImmutableMap<String, NestedSet<Artifact>> outputGroups) {
     this.outputGroups = outputGroups;
   }
 
-  /**
-   * Create a {@link com.google.devtools.build.lib.analysis.TopLevelArtifactProvider} with a single
-   * output group.
-   *
-   * <p>The artifacts in the second argument will be built if the {@code --output_groups=KEY}
-   * argument is present on the command line.
-   */
-  public static TopLevelArtifactProvider of(String key, NestedSet<Artifact> artifacts) {
-    return new TopLevelArtifactProvider(ImmutableMap.of(key, artifacts));
-  }
-
-  /**
-   * Create a {@link com.google.devtools.build.lib.analysis.TopLevelArtifactProvider} with multiple
-   * output groups.
-   *
-   * <p>Each output group will be built if a corresponding {@code --output_groups=KEY} argument is
-   * present on the command line.
-   */
-  public static TopLevelArtifactProvider of(Map<String, NestedSet<Artifact>> groups) {
-    return new TopLevelArtifactProvider(ImmutableMap.copyOf(groups));
-  }
-
   /** Return the artifacts in a particular output group. */
   public NestedSet<Artifact> getOutputGroup(String outputGroupName) {
     return outputGroups.get(outputGroupName);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
index b627657a0e..891f56dae9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
@@ -28,7 +28,6 @@ import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
 import com.google.devtools.build.lib.analysis.RunfilesSupport;
-import com.google.devtools.build.lib.analysis.TopLevelArtifactProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -236,8 +235,7 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
             new JavaRuntimeClasspathProvider(common.getRuntimeClasspath()))
         .add(JavaSourceJarsProvider.class,
             new JavaSourceJarsProvider(transitiveSourceJars, srcJars))
-        .add(TopLevelArtifactProvider.class, TopLevelArtifactProvider.of(
-            JavaSemantics.SOURCE_JARS_OUTPUT_GROUP, transitiveSourceJars))
+        .addOutputGroup(JavaSemantics.SOURCE_JARS_OUTPUT_GROUP, transitiveSourceJars)
         .build();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaImport.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaImport.java
index 7de35768a3..96ff4494f4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaImport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaImport.java
@@ -24,7 +24,6 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
-import com.google.devtools.build.lib.analysis.TopLevelArtifactProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -132,8 +131,7 @@ public class JavaImport implements RuleConfiguredTargetFactory {
         .add(CppCompilationContext.class, transitiveCppDeps)
         .add(JavaSourceJarsProvider.class, new JavaSourceJarsProvider(
             transitiveJavaSourceJars, srcJars))
-        .add(TopLevelArtifactProvider.class, TopLevelArtifactProvider.of(
-            JavaSemantics.SOURCE_JARS_OUTPUT_GROUP, transitiveJavaSourceJars))
+        .addOutputGroup(JavaSemantics.SOURCE_JARS_OUTPUT_GROUP, transitiveJavaSourceJars)
         .build();
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibrary.java
index dd8de30836..2ab59c4ae5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibrary.java
@@ -23,7 +23,6 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
-import com.google.devtools.build.lib.analysis.TopLevelArtifactProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -229,11 +228,10 @@ public class JavaLibrary implements RuleConfiguredTargetFactory {
             transitiveJavaNativeLibraries))
         .add(JavaSourceJarsProvider.class, new JavaSourceJarsProvider(
             transitiveSourceJars, ImmutableList.of(srcJar)))
-        .add(TopLevelArtifactProvider.class, TopLevelArtifactProvider.of(
-            JavaSemantics.SOURCE_JARS_OUTPUT_GROUP, transitiveSourceJars))
         // TODO(bazel-team): this should only happen for java_plugin
         .add(JavaPluginInfoProvider.class, new JavaPluginInfoProvider(
-            exportedProcessorClasses, exportedProcessorClasspath));
+            exportedProcessorClasses, exportedProcessorClasspath))
+        .addOutputGroup(JavaSemantics.SOURCE_JARS_OUTPUT_GROUP, transitiveSourceJars);
 
     if (ruleContext.hasErrors()) {
       return null;