Rollback of commit a3f5f576cd35798140ba3e81d03d919dd4ecb847.

*** Reason for rollback ***

Breaks targets in the depot:
[]

*** Original change description ***

output_group is not a real Skylark provider for aspects, as well as for rules.

--
MOS_MIGRATED_REVID=139354682

2 files changed, 39 insertions, 45 deletions

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 84f6a5f359..94c507787d 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
@@ -186,47 +186,41 @@ public final class SkylarkRuleConfiguredTargetBuilder {
 
     for (String outputGroup : outputGroups.keySet()) {
       SkylarkValue objects = outputGroups.get(outputGroup);
-      NestedSet<Artifact> artifacts = convertToOutputGroupValue(loc, outputGroup, objects);
-      builder.addOutputGroup(outputGroup, artifacts);
-    }
-  }
-
-  public static NestedSet<Artifact> convertToOutputGroupValue(Location loc, String outputGroup,
-      SkylarkValue objects) throws EvalException {
-    NestedSet<Artifact> artifacts;
+      NestedSet<Artifact> artifacts;
 
-    String typeErrorMessage =
-        "Output group '%s' is of unexpected type. "
-            + "Should be list or set of Files, but got '%s' instead.";
+      String typeErrorMessage =
+          "Output group '%s' is of unexpected type. "
+              + "Should be list or set of Files, but got '%s' instead.";
 
-    if (objects instanceof SkylarkList) {
-      NestedSetBuilder<Artifact> nestedSetBuilder = NestedSetBuilder.stableOrder();
-      for (Object o : (SkylarkList) objects) {
-        if (o instanceof Artifact) {
-          nestedSetBuilder.add((Artifact) o);
-        } else {
-          throw new EvalException(
-              loc,
-              String.format(
-                  typeErrorMessage,
-                  outputGroup,
-                  "list with an element of " + EvalUtils.getDataTypeNameFromClass(o.getClass())));
+      if (objects instanceof SkylarkList) {
+        NestedSetBuilder<Artifact> nestedSetBuilder = NestedSetBuilder.stableOrder();
+        for (Object o : (SkylarkList) objects) {
+          if (o instanceof Artifact) {
+            nestedSetBuilder.add((Artifact) o);
+          } else {
+            throw new EvalException(
+                loc,
+                String.format(
+                    typeErrorMessage,
+                    outputGroup,
+                    "list with an element of " + EvalUtils.getDataTypeNameFromClass(o.getClass())));
+          }
         }
+        artifacts = nestedSetBuilder.build();
+      } else {
+        artifacts =
+            SkylarkType.cast(
+                    objects,
+                    SkylarkNestedSet.class,
+                    Artifact.class,
+                    loc,
+                    typeErrorMessage,
+                    outputGroup,
+                    EvalUtils.getDataTypeName(objects, true))
+                .getSet(Artifact.class);
       }
-      artifacts = nestedSetBuilder.build();
-    } else {
-      artifacts =
-          SkylarkType.cast(
-                  objects,
-                  SkylarkNestedSet.class,
-                  Artifact.class,
-                  loc,
-                  typeErrorMessage,
-                  outputGroup,
-                  EvalUtils.getDataTypeName(objects, true))
-              .getSet(Artifact.class);
+      builder.addOutputGroup(outputGroup, artifacts);
     }
-    return artifacts;
   }
 
   private static ConfiguredTarget addStructFieldsAndBuild(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
index eb3922ad1a..b5b8087726 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -24,13 +25,12 @@ import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.packages.SkylarkAspect;
 import com.google.devtools.build.lib.packages.SkylarkClassObject;
-import com.google.devtools.build.lib.rules.SkylarkRuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.rules.SkylarkRuleContext;
-import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
 import com.google.devtools.build.lib.syntax.Environment;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.EvalExceptionWithStackTrace;
 import com.google.devtools.build.lib.syntax.Mutability;
+import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
 import com.google.devtools.build.lib.syntax.SkylarkType;
 import java.util.Map;
 
@@ -90,9 +90,8 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory {
         for (String key : struct.getKeys()) {
           if (key.equals("output_groups")) {
             addOutputGroups(struct.getValue(key), loc, builder);
-          } else {
-            builder.addSkylarkTransitiveInfo(key, struct.getValue(key), loc);
           }
+          builder.addSkylarkTransitiveInfo(key, struct.getValue(key), loc);
         }
         ConfiguredAspect configuredAspect = builder.build();
         SkylarkProviderValidationUtil.checkOrphanArtifacts(ruleContext);
@@ -102,20 +101,21 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory {
         ruleContext.ruleError("\n" + e.print());
         return null;
       }
+
     }
   }
 
   private static void addOutputGroups(Object value, Location loc,
       ConfiguredAspect.Builder builder)
       throws EvalException {
-    Map<String, SkylarkValue> outputGroups =
-        SkylarkType.castMap(value, String.class, SkylarkValue.class, "output_groups");
+    Map<String, SkylarkNestedSet> outputGroups = SkylarkType
+        .castMap(value, String.class, SkylarkNestedSet.class, "output_groups");
 
     for (String outputGroup : outputGroups.keySet()) {
-      SkylarkValue objects = outputGroups.get(outputGroup);
-
+      SkylarkNestedSet objects = outputGroups.get(outputGroup);
       builder.addOutputGroup(outputGroup,
-          SkylarkRuleConfiguredTargetBuilder.convertToOutputGroupValue(loc, outputGroup, objects));
+          SkylarkType.cast(objects, SkylarkNestedSet.class, Artifact.class, loc,
+              "Output group '%s'", outputGroup).getSet(Artifact.class));
     }
   }