Implement OutputGroupInfo provider.

Work towards #2894.

RELNOTES: None.
PiperOrigin-RevId: 154829065

18 files changed, 207 insertions, 45 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java
index 01004d1a3c..715939500e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java
@@ -82,14 +82,15 @@ public final class AnalysisUtils {
    * Returns the list of declared providers (native and Skylark) of the specified Skylark key from a
    * set of transitive info collections.
    */
-  public static Iterable<SkylarkClassObject> getProviders(
+  public static <T extends SkylarkClassObject>  Iterable<T> getProviders(
       Iterable<? extends TransitiveInfoCollection> prerequisites,
-      final ClassObjectConstructor.Key skylarkKey) {
-    ImmutableList.Builder<SkylarkClassObject> result = ImmutableList.builder();
+      final ClassObjectConstructor.Key skylarkKey,
+      Class<T> resultClass) {
+    ImmutableList.Builder<T> result = ImmutableList.builder();
     for (TransitiveInfoCollection prerequisite : prerequisites) {
       SkylarkClassObject prerequisiteProvider = prerequisite.get(skylarkKey);
       if (prerequisiteProvider != null) {
-        result.add(prerequisiteProvider);
+        result.add(resultClass.cast(prerequisiteProvider));
       }
     }
     return result.build();
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
index 6b7c031888..18c00f9156 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
@@ -30,11 +30,13 @@ import com.google.devtools.build.lib.packages.AspectClass;
 import com.google.devtools.build.lib.packages.AspectDescriptor;
 import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.packages.ClassObjectConstructor;
+import com.google.devtools.build.lib.packages.ClassObjectConstructor.Key;
 import com.google.devtools.build.lib.packages.SkylarkClassObject;
 import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.util.Preconditions;
 import java.util.Arrays;
+import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.TreeMap;
 import javax.annotation.Nullable;
@@ -116,8 +118,8 @@ public final class ConfiguredAspect implements Iterable<TransitiveInfoProvider>
     private final Map<String, NestedSetBuilder<Artifact>> outputGroupBuilders = new TreeMap<>();
     private final ImmutableMap.Builder<String, Object> skylarkProviderBuilder =
         ImmutableMap.builder();
-    private final ImmutableMap.Builder<SkylarkClassObjectConstructor.Key, SkylarkClassObject>
-        skylarkDeclaredProvidersBuilder = ImmutableMap.builder();
+    private final LinkedHashMap<Key, SkylarkClassObject>
+        skylarkDeclaredProvidersBuilder = new LinkedHashMap<>();
     private final RuleContext ruleContext;
     private final AspectDescriptor descriptor;
 
@@ -212,6 +214,14 @@ public final class ConfiguredAspect implements Iterable<TransitiveInfoProvider>
       return this;
     }
 
+    public Builder addNativeDeclaredProvider(SkylarkClassObject declaredProvider) {
+      ClassObjectConstructor constructor = declaredProvider.getConstructor();
+      Preconditions.checkState(constructor.isExported());
+      skylarkDeclaredProvidersBuilder.put(constructor.getKey(), declaredProvider);
+      return this;
+    }
+
+
     public ConfiguredAspect build() {
       if (!outputGroupBuilders.isEmpty()) {
         ImmutableMap.Builder<String, NestedSet<Artifact>> outputGroups = ImmutableMap.builder();
@@ -219,16 +229,19 @@ public final class ConfiguredAspect implements Iterable<TransitiveInfoProvider>
           outputGroups.put(entry.getKey(), entry.getValue().build());
         }
 
-        if (providers.contains(OutputGroupProvider.class)) {
+        if (skylarkDeclaredProvidersBuilder.containsKey(
+            OutputGroupProvider.SKYLARK_CONSTRUCTOR.getKey())) {
           throw new IllegalStateException(
               "OutputGroupProvider was provided explicitly; do not use addOutputGroup");
         }
-        addProvider(new OutputGroupProvider(outputGroups.build()));
+        skylarkDeclaredProvidersBuilder.put(
+            OutputGroupProvider.SKYLARK_CONSTRUCTOR.getKey(),
+            new OutputGroupProvider(outputGroups.build()));
       }
 
       ImmutableMap<String, Object> skylarkProvidersMap = skylarkProviderBuilder.build();
       ImmutableMap<SkylarkClassObjectConstructor.Key, SkylarkClassObject>
-          skylarkDeclaredProvidersMap = skylarkDeclaredProvidersBuilder.build();
+          skylarkDeclaredProvidersMap = ImmutableMap.copyOf(skylarkDeclaredProvidersBuilder);
       if (!skylarkProvidersMap.isEmpty() || !skylarkDeclaredProvidersMap.isEmpty()) {
         providers.add(new SkylarkProviders(skylarkProvidersMap, skylarkDeclaredProvidersMap));
       }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java
index fcd4e91e35..2c0fcfb178 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java
@@ -17,6 +17,8 @@ import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.packages.SkylarkClassObject;
+import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -85,16 +87,23 @@ public final class MergedConfiguredTarget extends AbstractConfiguredTarget {
 
     // Merge output group providers.
     OutputGroupProvider mergedOutputGroupProvider =
-        OutputGroupProvider.merge(getAllProviders(base, aspects, OutputGroupProvider.class));
+        OutputGroupProvider.merge(getAllOutputGroupProviders(base, aspects));
 
     // Merge Skylark providers.
-    ImmutableMap<String, Object> premergedProviders =
+    ImmutableMap<String, Object> premergedLegacyProviders =
         mergedOutputGroupProvider == null
-        ? ImmutableMap.<String, Object>of()
-        : ImmutableMap.<String, Object>of(
-            OutputGroupProvider.SKYLARK_NAME, mergedOutputGroupProvider);
+            ? ImmutableMap.<String, Object>of()
+            : ImmutableMap.<String, Object>of(
+                OutputGroupProvider.SKYLARK_NAME, mergedOutputGroupProvider);
+
+    ImmutableMap<SkylarkClassObjectConstructor.Key, SkylarkClassObject> premergedProviders =
+        mergedOutputGroupProvider == null
+        ? ImmutableMap.<SkylarkClassObjectConstructor.Key, SkylarkClassObject>of()
+        : ImmutableMap.<SkylarkClassObjectConstructor.Key, SkylarkClassObject>of(
+            OutputGroupProvider.SKYLARK_CONSTRUCTOR.getKey(), mergedOutputGroupProvider);
     SkylarkProviders mergedSkylarkProviders =
         SkylarkProviders.merge(
+            premergedLegacyProviders,
             premergedProviders,
             getAllProviders(base, aspects, SkylarkProviders.class));
 
@@ -103,9 +112,6 @@ public final class MergedConfiguredTarget extends AbstractConfiguredTarget {
         getAllProviders(base, aspects, ExtraActionArtifactsProvider.class));
 
     TransitiveInfoProviderMap.Builder aspectProviders = TransitiveInfoProviderMap.builder();
-    if (mergedOutputGroupProvider != null) {
-      aspectProviders.add(mergedOutputGroupProvider);
-    }
     if (mergedSkylarkProviders != null) {
       aspectProviders.add(mergedSkylarkProviders);
     }
@@ -117,8 +123,7 @@ public final class MergedConfiguredTarget extends AbstractConfiguredTarget {
       for (Map.Entry<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider> entry :
           aspect.getProviders().entrySet()) {
         Class<? extends TransitiveInfoProvider> providerClass = entry.getKey();
-        if (OutputGroupProvider.class.equals(providerClass)
-            || SkylarkProviders.class.equals(providerClass)
+        if (SkylarkProviders.class.equals(providerClass)
             || ExtraActionArtifactsProvider.class.equals(providerClass)) {
           continue;
         }
@@ -133,6 +138,24 @@ public final class MergedConfiguredTarget extends AbstractConfiguredTarget {
     return new MergedConfiguredTarget(base, aspectProviders.build());
   }
 
+  private static ImmutableList<OutputGroupProvider> getAllOutputGroupProviders(
+      ConfiguredTarget base, Iterable<ConfiguredAspect> aspects) {
+    OutputGroupProvider baseProvider = OutputGroupProvider.get(base);
+    ImmutableList.Builder<OutputGroupProvider> providers = ImmutableList.builder();
+    if (baseProvider != null) {
+      providers.add(baseProvider);
+    }
+
+    for (ConfiguredAspect configuredAspect : aspects) {
+      OutputGroupProvider aspectProvider = OutputGroupProvider.get(configuredAspect);;
+      if (aspectProvider == null) {
+        continue;
+      }
+      providers.add(aspectProvider);
+    }
+    return providers.build();
+  }
+
   private static <T extends TransitiveInfoProvider> List<T> getAllProviders(
       ConfiguredTarget base, Iterable<ConfiguredAspect> aspects, Class<T> providerClass) {
     T baseProvider = base.getProvider(providerClass);
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupProvider.java
index 7ba2eff806..397d7fdd1b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupProvider.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.analysis;
 
 import static com.google.devtools.build.lib.syntax.EvalUtils.SKYLARK_COMPARATOR;
 
+import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedSet;
@@ -27,6 +28,9 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 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.Location;
+import com.google.devtools.build.lib.packages.NativeClassObjectConstructor;
+import com.google.devtools.build.lib.packages.SkylarkClassObject;
+import com.google.devtools.build.lib.rules.SkylarkRuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.EvalUtils;
 import com.google.devtools.build.lib.syntax.SkylarkIndexable;
@@ -34,6 +38,8 @@ import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 import javax.annotation.Nullable;
 
@@ -51,10 +57,12 @@ import javax.annotation.Nullable;
  * not mentioned on the output.
  */
 @Immutable
-public final class OutputGroupProvider implements
-    TransitiveInfoProvider, SkylarkIndexable, Iterable<String> {
+public final class OutputGroupProvider extends SkylarkClassObject
+    implements SkylarkIndexable, Iterable<String> {
   public static final String SKYLARK_NAME = "output_groups";
 
+  public static NativeClassObjectConstructor SKYLARK_CONSTRUCTOR = new Constructor();
+
   /**
    * Prefix for output groups that are not reported to the user on the terminal output of Blaze when
    * they are built.
@@ -113,9 +121,26 @@ public final class OutputGroupProvider implements
   private final ImmutableMap<String, NestedSet<Artifact>> outputGroups;
 
   public OutputGroupProvider(ImmutableMap<String, NestedSet<Artifact>> outputGroups) {
+    super(SKYLARK_CONSTRUCTOR, ImmutableMap.<String, Object>of());
     this.outputGroups = outputGroups;
   }
 
+  @Nullable
+  public static OutputGroupProvider get(TransitiveInfoCollection collection) {
+    return (OutputGroupProvider) collection.get(SKYLARK_CONSTRUCTOR.getKey());
+  }
+
+  @Nullable
+  public static OutputGroupProvider get(ConfiguredAspect aspect) {
+    SkylarkProviders skylarkProviders = aspect.getProvider(SkylarkProviders.class);
+
+
+    return skylarkProviders != null
+        ? (OutputGroupProvider) skylarkProviders.getDeclaredProvider(SKYLARK_CONSTRUCTOR.getKey())
+        : null;
+  }
+
+
   /** Return the artifacts in a particular output group.
    *
    * @return the artifacts in the output group with the given name. The return value is never null.
@@ -210,7 +235,6 @@ public final class OutputGroupProvider implements
           "Output group %s not present", key
       ));
     }
-
   }
 
   @Override
@@ -222,4 +246,51 @@ public final class OutputGroupProvider implements
   public Iterator<String> iterator() {
     return SKYLARK_COMPARATOR.sortedCopy(outputGroups.keySet()).iterator();
   }
+
+  @Override
+  public Object getValue(String name) {
+    NestedSet<Artifact> result = outputGroups.get(name);
+    if (result == null) {
+      return null;
+    }
+    return SkylarkNestedSet.of(Artifact.class, result);
+  }
+
+  @Override
+  public ImmutableCollection<String> getKeys() {
+    return outputGroups.keySet();
+  }
+
+  /**
+   * A constructor callable from Skylark for OutputGroupProvider.
+   */
+  private static class Constructor extends NativeClassObjectConstructor {
+
+    private Constructor() {
+      super("OutputGroupInfo");
+    }
+
+    @Override
+    protected SkylarkClassObject createInstanceFromSkylark(Object[] args, Location loc)
+        throws EvalException {
+
+      @SuppressWarnings("unchecked")
+      Map<String, Object> kwargs = (Map<String, Object>) args[0];
+
+      ImmutableMap.Builder<String, NestedSet<Artifact>> builder = ImmutableMap.builder();
+      for (Entry<String, Object> entry : kwargs.entrySet()) {
+        builder.put(entry.getKey(),
+            SkylarkRuleConfiguredTargetBuilder.convertToOutputGroupValue(
+                loc, entry.getKey(), entry.getValue()));
+
+
+      }
+      return new OutputGroupProvider(builder.build());
+    }
+
+    @Override
+    public String getErrorMessageFormatForInstances() {
+      return "Output group %s not present";
+    }
+  }
 }
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 6a9c6d625b..bdf8e10543 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
@@ -131,7 +131,7 @@ public final class RuleConfiguredTargetBuilder {
       }
 
       OutputGroupProvider outputGroupProvider = new OutputGroupProvider(outputGroups.build());
-      addProvider(OutputGroupProvider.class, outputGroupProvider);
+      addNativeDeclaredProvider(outputGroupProvider);
       addSkylarkTransitiveInfo(OutputGroupProvider.SKYLARK_NAME, outputGroupProvider);
     }
 
@@ -294,6 +294,9 @@ public final class RuleConfiguredTargetBuilder {
    * Adds a "declared provider" defined in Skylark to the rule.
    * Use this method for declared providers defined in Skyark.
    *
+   * Has special handling for {@link OutputGroupProvider}: that provider is not added
+   * from Skylark directly, instead its outpuyt groups are added.
+   *
    * Use {@link #addNativeDeclaredProvider(SkylarkClassObject)} in definitions of
    * native rules.
    */
@@ -304,7 +307,14 @@ public final class RuleConfiguredTargetBuilder {
       throw new EvalException(constructor.getLocation(),
           "All providers must be top level values");
     }
-    skylarkDeclaredProviders.put(constructor.getKey(), provider);
+    if (OutputGroupProvider.SKYLARK_CONSTRUCTOR.getKey().equals(constructor.getKey())) {
+      OutputGroupProvider outputGroupProvider = (OutputGroupProvider) provider;
+      for (String outputGroup : outputGroupProvider) {
+        addOutputGroup(outputGroup, outputGroupProvider.getOutputGroup(outputGroup));
+      }
+    } else {
+      skylarkDeclaredProviders.put(constructor.getKey(), provider);
+    }
     return this;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index f47b38d006..30007e6a21 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -857,9 +857,11 @@ public final class RuleContext extends TargetContext
    * Returns all the declared providers (native and Skylark) for the specified constructor under the
    * specified attribute of this target in the BUILD file.
    */
-  public Iterable<SkylarkClassObject> getPrerequisites(
-      String attributeName, Mode mode, final ClassObjectConstructor.Key skylarkKey) {
-    return AnalysisUtils.getProviders(getPrerequisites(attributeName, mode), skylarkKey);
+  public <T extends SkylarkClassObject> Iterable<T> getPrerequisites(
+      String attributeName, Mode mode,
+      final ClassObjectConstructor.Key skylarkKey,
+      Class<T> result) {
+    return AnalysisUtils.getProviders(getPrerequisites(attributeName, mode), skylarkKey, result);
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
index 05c9c347a1..0778ade163 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
@@ -20,6 +20,7 @@ import com.google.devtools.build.lib.analysis.MergedConfiguredTarget.DuplicateEx
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.ClassObjectConstructor;
 import com.google.devtools.build.lib.packages.SkylarkClassObject;
+import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor;
 import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
 import com.google.devtools.build.lib.rules.SkylarkApiProvider;
 import com.google.devtools.build.lib.syntax.EvalException;
@@ -124,7 +125,8 @@ public final class SkylarkProviders implements TransitiveInfoProvider {
    * @param providers providers to merge {@code this} with.
    */
   public static SkylarkProviders merge(
-      Map<String, Object> premergedProviders,
+      ImmutableMap<String, Object> premergedLegacyProviders,
+      ImmutableMap<SkylarkClassObjectConstructor.Key, SkylarkClassObject> premergedProviders,
       List<SkylarkProviders> providers)
       throws DuplicateException {
     if (premergedProviders.size() == 0 && providers.size() == 0) {
@@ -136,11 +138,11 @@ public final class SkylarkProviders implements TransitiveInfoProvider {
 
     ImmutableMap<String, Object> skylarkProviders = mergeMaps(providers,
         SKYLARK_PROVIDERS_MAP_FUNCTION,
-        premergedProviders);
+        premergedLegacyProviders);
 
     ImmutableMap<ClassObjectConstructor.Key, SkylarkClassObject> declaredProviders =
         mergeMaps(providers, DECLARED_PROVIDERS_MAP_FUNCTION,
-            ImmutableMap.<ClassObjectConstructor.Key, SkylarkClassObject>of());
+            premergedProviders);
 
     return new SkylarkProviders(skylarkProviders, declaredProviders);
   }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java
index 6665918167..a9b77b4074 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java
@@ -169,7 +169,7 @@ public final class TopLevelArtifactHelper {
   public static ArtifactsToBuild getAllArtifactsToBuild(TransitiveInfoCollection target,
       TopLevelArtifactContext context) {
     return getAllArtifactsToBuild(
-        target.getProvider(OutputGroupProvider.class),
+        OutputGroupProvider.get(target),
         target.getProvider(FileProvider.class),
         context
     );
@@ -179,7 +179,7 @@ public final class TopLevelArtifactHelper {
       AspectValue aspectValue, TopLevelArtifactContext context) {
     ConfiguredAspect configuredAspect = aspectValue.getConfiguredAspect();
     return getAllArtifactsToBuild(
-        configuredAspect.getProvider(OutputGroupProvider.class),
+        OutputGroupProvider.get(configuredAspect),
         configuredAspect.getProvider(FileProvider.class),
         context);
   }
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
index 8ef4a38ec9..2d036eeef2 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
@@ -33,7 +33,6 @@ import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.skyframe.AspectValue;
 import com.google.devtools.build.lib.util.io.OutErr;
-
 import java.util.ArrayList;
 import java.util.Collection;
 
@@ -129,7 +128,7 @@ class BuildResultPrinter {
       // For failed compilation, it is still useful to examine temp artifacts,
       // (ie, preprocessed and assembler files).
       OutputGroupProvider topLevelProvider =
-          target.getProvider(OutputGroupProvider.class);
+          OutputGroupProvider.get(target);
       String productName = env.getRuntime().getProductName();
       if (topLevelProvider != null) {
         for (Artifact temp : topLevelProvider.getOutputGroup(OutputGroupProvider.TEMP_FILES)) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
index aefdfaf3c9..733f331e54 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
@@ -209,6 +209,14 @@ public class SkylarkRuleClassFunctions {
   )
   private static final ClassObjectConstructor defaultInfo = DefaultProvider.SKYLARK_CONSTRUCTOR;
 
+  @SkylarkSignature(
+    name = "OutputGroupInfo",
+    returnType = ClassObjectConstructor.class,
+    doc = "todo"
+  )
+  private static final ClassObjectConstructor outputGroupInfo =
+      OutputGroupProvider.SKYLARK_CONSTRUCTOR;
+
   // TODO(bazel-team): Move to a "testing" namespace module. Normally we'd pass an objectType
   // to @SkylarkSignature to do this, but that doesn't work here because we're exposing an already-
   // configured BaseFunction, rather than defining a new BuiltinFunction. This should wait for
@@ -1047,7 +1055,7 @@ public class SkylarkRuleClassFunctions {
   )
   private static final BuiltinFunction output_group = new BuiltinFunction("output_group") {
     public SkylarkNestedSet invoke(TransitiveInfoCollection self, String group) {
-      OutputGroupProvider provider = self.getProvider(OutputGroupProvider.class);
+      OutputGroupProvider provider = OutputGroupProvider.get(self);
       NestedSet<Artifact> result = provider != null
           ? provider.getOutputGroup(group)
           : NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
index 51295707de..f6900a2e70 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
@@ -197,7 +197,7 @@ public final class SkylarkRuleConfiguredTargetBuilder {
   }
 
   public static NestedSet<Artifact> convertToOutputGroupValue(Location loc, String outputGroup,
-      SkylarkValue objects) throws EvalException {
+      Object objects) throws EvalException {
     NestedSet<Artifact> artifacts;
 
     String typeErrorMessage =
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
index 7657427885..922d87e7dc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
@@ -39,6 +39,8 @@ import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
 import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.Rule;
+import com.google.devtools.build.lib.packages.SkylarkClassObject;
+import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor;
 import com.google.devtools.build.lib.rules.android.ResourceContainer.ResourceType;
 import com.google.devtools.build.lib.rules.cpp.CcLinkParams;
 import com.google.devtools.build.lib.rules.cpp.CcLinkParamsProvider;
@@ -95,6 +97,20 @@ public class AndroidCommon {
     return builder.build();
   }
 
+  public static final <T extends SkylarkClassObject> Iterable<T> getTransitivePrerequisites(
+      RuleContext ruleContext, Mode mode, SkylarkClassObjectConstructor.Key key,
+      final Class<T> classType) {
+    IterablesChain.Builder<T> builder = IterablesChain.builder();
+    AttributeMap attributes = ruleContext.attributes();
+    for (String attr : TRANSITIVE_ATTRIBUTES) {
+      if (attributes.has(attr, BuildType.LABEL_LIST)) {
+        builder.add(ruleContext.getPrerequisites(attr, mode, key, classType));
+      }
+    }
+    return builder.build();
+  }
+
+
   public static final Iterable<TransitiveInfoCollection> collectTransitiveInfo(
       RuleContext ruleContext, Mode mode) {
     ImmutableList.Builder<TransitiveInfoCollection> builder = ImmutableList.builder();
@@ -894,7 +910,9 @@ public class AndroidCommon {
   private NestedSet<Artifact> collectHiddenTopLevelArtifacts(RuleContext ruleContext) {
     NestedSetBuilder<Artifact> builder = NestedSetBuilder.stableOrder();
     for (OutputGroupProvider provider :
-        getTransitivePrerequisites(ruleContext, Mode.TARGET, OutputGroupProvider.class)) {
+        getTransitivePrerequisites(ruleContext, Mode.TARGET,
+            OutputGroupProvider.SKYLARK_CONSTRUCTOR.getKey(),
+            OutputGroupProvider.class)) {
       builder.addTransitive(provider.getOutputGroup(OutputGroupProvider.HIDDEN_TOP_LEVEL));
     }
     return builder.build();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
index 3d727aefa1..86f3184349 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
@@ -314,7 +314,9 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory {
         ccCompilationOutputs.getFilesToCompile(
             isLipoCollector, processHeadersInDependencies, usePic));
     for (OutputGroupProvider dep :
-        ruleContext.getPrerequisites("deps", Mode.TARGET, OutputGroupProvider.class)) {
+        ruleContext.getPrerequisites("deps", Mode.TARGET,
+            OutputGroupProvider.SKYLARK_CONSTRUCTOR.getKey(),
+            OutputGroupProvider.class)) {
       artifactsToForceBuilder.addTransitive(
           dep.getOutputGroup(OutputGroupProvider.HIDDEN_TOP_LEVEL));
     }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
index 35b39eedea..3aaab4df24 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java
@@ -1484,7 +1484,8 @@ public final class CcLibraryHelper {
       RuleContext ruleContext, CcCompilationOutputs ccCompilationOutputs) {
     NestedSetBuilder<Artifact> headerTokens = NestedSetBuilder.stableOrder();
     for (OutputGroupProvider dep :
-        ruleContext.getPrerequisites("deps", Mode.TARGET, OutputGroupProvider.class)) {
+        ruleContext.getPrerequisites("deps", Mode.TARGET,
+            OutputGroupProvider.SKYLARK_CONSTRUCTOR.getKey(), OutputGroupProvider.class)) {
       headerTokens.addTransitive(dep.getOutputGroup(CcLibraryHelper.HIDDEN_HEADER_TOKENS));
     }
     if (ruleContext.getFragment(CppConfiguration.class).processHeadersInDependencies()) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
index b7395a914c..d4d94638bf 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
@@ -317,11 +317,12 @@ public class CcProtoAspect extends NativeAspectClass implements ConfiguredAspect
     }
 
     public void addProviders(ConfiguredAspect.Builder builder) {
+      OutputGroupProvider outputGroupProvider = new OutputGroupProvider(outputGroups);
       builder.addProvider(
           new CcProtoLibraryProviders(
-              filesBuilder.build(),
-              ccLibraryProviders.toBuilder().add(new OutputGroupProvider(outputGroups)).build()));
+              filesBuilder.build(), ccLibraryProviders, outputGroupProvider));
       builder.addProviders(ccLibraryProviders);
+      builder.addNativeDeclaredProvider(outputGroupProvider);
       if (headerProvider != null) {
         builder.addProvider(headerProvider);
       }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibrary.java
index 1f03ef16d0..d6a576370e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibrary.java
@@ -45,11 +45,17 @@ public class CcProtoLibrary implements RuleConfiguredTargetFactory {
         checkNotNull(ruleContext.getPrerequisite("deps", TARGET))
             .getProvider(CcProtoLibraryProviders.class);
 
-    return new RuleConfiguredTargetBuilder(ruleContext)
+    RuleConfiguredTargetBuilder ruleConfiguredTargetBuilder = new RuleConfiguredTargetBuilder(
+        ruleContext)
         .setFilesToBuild(depProviders.filesBuilder)
         .addProvider(
             RunfilesProvider.class, RunfilesProvider.withData(Runfiles.EMPTY, Runfiles.EMPTY))
-        .addProviders(depProviders.providerMap)
+        .addProviders(depProviders.providerMap);
+    for (String groupName : depProviders.outputGroupProvider) {
+      ruleConfiguredTargetBuilder.addOutputGroup(groupName,
+          depProviders.outputGroupProvider.getOutputGroup(groupName));
+    }
+    return ruleConfiguredTargetBuilder
         .addSkylarkTransitiveInfo(CcSkylarkApiProvider.NAME, new CcSkylarkApiProvider())
         .build();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryProviders.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryProviders.java
index 35b53d8894..f9bb8267d7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryProviders.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryProviders.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.rules.cpp.proto;
 
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.OutputGroupProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMap;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -28,9 +29,13 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety;
 final class CcProtoLibraryProviders implements TransitiveInfoProvider {
   final NestedSet<Artifact> filesBuilder;
   final TransitiveInfoProviderMap providerMap;
+  final OutputGroupProvider outputGroupProvider;
 
-  CcProtoLibraryProviders(NestedSet<Artifact> filesBuilder, TransitiveInfoProviderMap providerMap) {
+  CcProtoLibraryProviders(NestedSet<Artifact> filesBuilder,
+      TransitiveInfoProviderMap providerMap,
+      OutputGroupProvider outputGroupProvider) {
     this.filesBuilder = filesBuilder;
     this.providerMap = providerMap;
+    this.outputGroupProvider = outputGroupProvider;
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java b/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java
index e359e18187..02504be6c9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java
@@ -128,7 +128,7 @@ public class Filegroup implements RuleConfiguredTargetFactory {
     NestedSetBuilder<Artifact> result = NestedSetBuilder.stableOrder();
 
     for (TransitiveInfoCollection dep : deps) {
-      OutputGroupProvider outputGroupProvider = dep.getProvider(OutputGroupProvider.class);
+      OutputGroupProvider outputGroupProvider = OutputGroupProvider.get(dep);
       if (outputGroupProvider != null) {
         result.addTransitive(outputGroupProvider.getOutputGroup(outputGroupName));
       }