commit af27046f8c74d8fb43c8db91428d0da2e1607a06
author Michael Staib <mstaib@google.com> Wed May 18 16:22:07 2016 +0000
committer Kristina Chodorow <kchodorow@google.com> Thu May 19 16:27:12 2016 +0000
tree 665510535117fb3b21ce448e7eeaf25e4870223e
parent 3be65b833d712b5d92f18be2a346f071739ea44a

Always restrict aspects to only access requested configuration fragments.

This completes the introduction of aspect configuration fragment enforcement
for static configuration builds; as of this change, it is no longer possible to
fall back to the base rule's set of requested configuration fragments.

This sort of fallback may become possible later, likely in a more controlled way.

--
MOS_MIGRATED_REVID=122638152

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

diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
index 4e3f08a..606b177 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
@@ -125,13 +125,9 @@
   }
 
   /**
-   * Returns the set of configuration fragments required by this Aspect, or {@code null} if it has
-   * not set a configuration fragment policy, meaning it should inherit from the attached rule.
+   * Returns the set of configuration fragments required by this Aspect.
    */
-  @Nullable public ConfigurationFragmentPolicy getConfigurationFragmentPolicy() {
-    // TODO(mstaib): When all existing aspects properly set their configuration fragment policy,
-    // this method and the associated member should no longer be nullable.
-    // "inherit from the attached rule" should go away.
+  public ConfigurationFragmentPolicy getConfigurationFragmentPolicy() {
     return configurationFragmentPolicy;
   }
 
@@ -219,14 +215,6 @@
     private final Multimap<String, AspectClass> attributeAspects = LinkedHashMultimap.create();
     private final ConfigurationFragmentPolicy.Builder configurationFragmentPolicy =
         new ConfigurationFragmentPolicy.Builder();
-    // TODO(mstaib): When all existing aspects properly set their configuration fragment policy,
-    // remove this flag and the code that interacts with it.
-    /**
-     * True if the aspect definition has intentionally specified a configuration fragment policy by
-     * calling any of the methods which set up the policy, and thus needs the built AspectDefinition
-     * to retain the policy.
-     */
-    private boolean hasConfigurationFragmentPolicy = false;
 
     public Builder(String name) {
       this.name = name;
@@ -306,7 +294,6 @@
      * <p>The value is inherited by subclasses.
      */
     public Builder requiresConfigurationFragments(Class<?>... configurationFragments) {
-      hasConfigurationFragmentPolicy = true;
       configurationFragmentPolicy
           .requiresConfigurationFragments(ImmutableSet.copyOf(configurationFragments));
       return this;
@@ -319,7 +306,6 @@
      * <p>The value is inherited by subclasses.
      */
     public Builder requiresHostConfigurationFragments(Class<?>... configurationFragments) {
-      hasConfigurationFragmentPolicy = true;
       configurationFragmentPolicy
           .requiresHostConfigurationFragments(ImmutableSet.copyOf(configurationFragments));
       return this;
@@ -334,11 +320,6 @@
      */
     public Builder requiresConfigurationFragmentsBySkylarkModuleName(
         Collection<String> configurationFragmentNames) {
-      // This method is unconditionally called from Skylark code, so only consider the user to have
-      // specified a configuration policy if the collection actually has anything in it.
-      // TODO(mstaib): Stop caring about this as soon as all aspects have configuration policies.
-      hasConfigurationFragmentPolicy =
-          hasConfigurationFragmentPolicy || !configurationFragmentNames.isEmpty();
       configurationFragmentPolicy
           .requiresConfigurationFragmentsBySkylarkModuleName(configurationFragmentNames);
       return this;
@@ -353,11 +334,6 @@
      */
     public Builder requiresHostConfigurationFragmentsBySkylarkModuleName(
         Collection<String> configurationFragmentNames) {
-      // This method is unconditionally called from Skylark code, so only consider the user to have
-      // specified a configuration policy if the collection actually has anything in it.
-      // TODO(mstaib): Stop caring about this as soon as all aspects have configuration policies.
-      hasConfigurationFragmentPolicy =
-          hasConfigurationFragmentPolicy || !configurationFragmentNames.isEmpty();
       configurationFragmentPolicy
           .requiresHostConfigurationFragmentsBySkylarkModuleName(configurationFragmentNames);
       return this;
@@ -368,7 +344,6 @@
      * {@link #requiresConfigurationFragments}).
      */
     public Builder setMissingFragmentPolicy(MissingFragmentPolicy missingFragmentPolicy) {
-      hasConfigurationFragmentPolicy = true;
       configurationFragmentPolicy.setMissingFragmentPolicy(missingFragmentPolicy);
       return this;
     }
@@ -381,7 +356,7 @@
     public AspectDefinition build() {
       return new AspectDefinition(name, ImmutableSet.copyOf(requiredProviders),
           ImmutableMap.copyOf(attributes), ImmutableSetMultimap.copyOf(attributeAspects),
-          hasConfigurationFragmentPolicy ? configurationFragmentPolicy.build() : null);
+          configurationFragmentPolicy.build());
     }
   }
 }