Rollback of commit 34ffc4cab1b6cdde54e5945427b79c24fff39aa5.

*** Reason for rollback ***

Roll-forward of commit 01120026dc313ee7ad9ea95069a29252eb19173b with fix.

*** Original change description ***

Automated [] rollback of commit 01120026dc313ee7ad9ea95069a29252eb19173b.

--
PiperOrigin-RevId: 148897534
MOS_MIGRATED_REVID=148897534

1 files changed, 27 insertions, 51 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
index b28b08e07c..4b8c90da63 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
@@ -19,7 +19,6 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet;
 import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
@@ -274,6 +273,18 @@ public final class SkylarkAttr {
             "cfg must be either 'data', 'host', or 'target'.");
       }
     }
+
+    if (containsNonNoneKey(arguments, ASPECTS_ARG)) {
+      Object obj = arguments.get(ASPECTS_ARG);
+      SkylarkType.checkType(obj, SkylarkList.class, ASPECTS_ARG);
+
+      List<SkylarkAspect> aspects =
+          ((SkylarkList<?>) obj).getContents(SkylarkAspect.class, "aspects");
+      for (SkylarkAspect aspect : aspects) {
+        builder.aspect(aspect, ast.getLocation());
+      }
+    }
+
     return builder;
   }
 
@@ -630,12 +641,13 @@ public final class SkylarkAttr {
                     SINGLE_FILE_ARG,
                     singleFile,
                     CONFIGURATION_ARG,
-                    cfg),
+                    cfg,
+                    ASPECTS_ARG,
+                    aspects
+                    ),
                 ast,
                 env);
-            ImmutableList<SkylarkAspect> skylarkAspects =
-                ImmutableList.copyOf(aspects.getContents(SkylarkAspect.class, "aspects"));
-            return new Descriptor(attribute, skylarkAspects);
+            return new Descriptor(attribute);
           } catch (EvalException e) {
             throw new EvalException(ast.getLocation(), e.getMessage(), e);
           }
@@ -908,13 +920,14 @@ public final class SkylarkAttr {
                   ALLOW_EMPTY_ARG,
                   allowEmpty,
                   CONFIGURATION_ARG,
-                  cfg);
+                  cfg,
+                  ASPECTS_ARG,
+                  aspects
+                  );
           try {
             Attribute.Builder<?> attribute =
                 createAttribute(BuildType.LABEL_LIST, kwargs, ast, env);
-            ImmutableList<SkylarkAspect> skylarkAspects =
-                ImmutableList.copyOf(aspects.getContents(SkylarkAspect.class, "aspects"));
-            return new Descriptor(attribute, skylarkAspects);
+            return new Descriptor(attribute);
           } catch (EvalException e) {
             throw new EvalException(ast.getLocation(), e.getMessage(), e);
           }
@@ -1060,13 +1073,13 @@ public final class SkylarkAttr {
                   ALLOW_EMPTY_ARG,
                   allowEmpty,
                   CONFIGURATION_ARG,
-                  cfg);
+                  cfg,
+                  ASPECTS_ARG,
+                  aspects);
           try {
             Attribute.Builder<?> attribute =
                 createAttribute(BuildType.LABEL_KEYED_STRING_DICT, kwargs, ast, env);
-            ImmutableList<SkylarkAspect> skylarkAspects =
-                ImmutableList.copyOf(aspects.getContents(SkylarkAspect.class, "aspects"));
-            return new Descriptor(attribute, skylarkAspects);
+            return new Descriptor(attribute);
           } catch (EvalException e) {
             throw new EvalException(ast.getLocation(), e.getMessage(), e);
           }
@@ -1425,33 +1438,17 @@ public final class SkylarkAttr {
   )
   public static final class Descriptor {
     private final Attribute.Builder<?> attributeBuilder;
-    private final ImmutableList<SkylarkAspect> aspects;
 
     /**
      * This lock guards {@code attributeBuilder} field.
      *
      * {@link Attribute.Builder} class is not thread-safe for concurrent modification.
-     * This class, together with its enclosing {@link SkylarkAttr} class, do not let
-     * anyone else access the {@code attributeBuilder}, however {@link #exportAspects(Location)}
-     * method actually modifies the {@code attributeBuilder}. Therefore all read- and write-accesses
-     * to {@code attributeBuilder} are protected with this lock.
-     *
-     * For example, {@link #hasDefault()} method only reads from {@link #attributeBuilder},
-     * but we have no guarantee that it is safe to do so concurrently with adding aspects
-     * in {@link #exportAspects(Location)}.
      */
     private final Object lock = new Object();
-    boolean exported;
-
-    private Descriptor(Attribute.Builder<?> attributeBuilder) {
-      this(attributeBuilder, ImmutableList.<SkylarkAspect>of());
-    }
 
     private Descriptor(
-        Attribute.Builder<?> attributeBuilder, ImmutableList<SkylarkAspect> aspects) {
+        Attribute.Builder<?> attributeBuilder) {
       this.attributeBuilder = attributeBuilder;
-      this.aspects = aspects;
-      exported = false;
     }
 
     public boolean hasDefault() {
@@ -1471,27 +1468,6 @@ public final class SkylarkAttr {
         return attributeBuilder.build(name);
       }
     }
-
-    public ImmutableList<SkylarkAspect> getAspects() {
-      return aspects;
-    }
-
-    public void exportAspects(Location definitionLocation) throws EvalException {
-      synchronized (lock) {
-        if (exported) {
-          // Only export an attribute definiton once.
-          return;
-        }
-        for (SkylarkAspect skylarkAspect : getAspects()) {
-          if (!skylarkAspect.isExported()) {
-            throw new EvalException(definitionLocation,
-                "All aspects applied to rule dependencies must be top-level values");
-          }
-          attributeBuilder.aspect(skylarkAspect, definitionLocation);
-        }
-        exported = true;
-      }
-    }
   }
 
   static {