android_test binary_under_test/deps must have the same flags if any are set.

After this CL, if the feature_flags attribute of android_test or
android_binary is not set, no transition takes place when entering that
rule. This means that if it is depended upon by another test or binary, it
will use the enclosing test or binary's flags.

This permits users of feature flags to depend on non-users of feature flags.
The opposite is still not permitted. If a dep sets feature flags, then the
target depending on it must have the exact same feature flags set.

This way, all targets used in an android_test are built the same way, but
it's possible to interoperate with targets which are agnostic to feature
flags.

Note that "not set" is different from "set to the empty dictionary"; the
former reuses the definitions set higher up in the build graph, while the
latter clears all feature flag values and resets them to their defaults.

RELNOTES: None.
PiperOrigin-RevId: 167035122

4 files changed, 48 insertions, 27 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingRuleTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingRuleTransitionFactory.java
index 3cc2ff71f4..d8e1c55332 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingRuleTransitionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingRuleTransitionFactory.java
@@ -38,6 +38,17 @@ public class ComposingRuleTransitionFactory implements RuleTransitionFactory {
 
   @Override
   public Attribute.Transition buildTransitionFor(Rule rule) {
+    PatchTransition transition1 = (PatchTransition) rtf1.buildTransitionFor(rule);
+    PatchTransition transition2 = (PatchTransition) rtf2.buildTransitionFor(rule);
+
+    if (transition1 == null) {
+      return transition2;
+    }
+
+    if (transition2 == null) {
+      return transition1;
+    }
+
     return new ComposingPatchTransition(
         (PatchTransition) rtf1.buildTransitionFor(rule),
         (PatchTransition) rtf2.buildTransitionFor(rule));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java
index 66767b8942..de91e27fc3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.rules.android;
 
 import com.google.auto.value.AutoValue;
+import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.AliasProvider;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
@@ -27,7 +28,6 @@ import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.rules.config.ConfigFeatureFlag;
-import com.google.devtools.build.lib.rules.config.ConfigFeatureFlagProvider;
 import java.util.Map;
 
 /**
@@ -48,23 +48,29 @@ public abstract class AndroidFeatureFlagSetProvider implements TransitiveInfoPro
   AndroidFeatureFlagSetProvider() {}
 
   /** Creates a new AndroidFeatureFlagSetProvider with the given flags. */
-  public static AndroidFeatureFlagSetProvider create(Map<Label, String> flags) {
-    return new AutoValue_AndroidFeatureFlagSetProvider(ImmutableMap.copyOf(flags));
+  public static AndroidFeatureFlagSetProvider create(Optional<? extends Map<Label, String>> flags) {
+    return new AutoValue_AndroidFeatureFlagSetProvider(flags.transform(ImmutableMap::copyOf));
   }
 
   /**
    * Builds a map which can be used with create, confirming that the desired flag values were
-   * actually received, and producing an error if they were not (because dynamic configurations are
-   * not enabled, or because aliases were used).
+   * actually received, and producing an error if they were not (because aliases were used).
+   *
+   * <p>If the attribute which defines feature flags was not specified, an empty {@link Optional}
+   * instance is returned.
    */
-  public static ImmutableMap<Label, String> getAndValidateFlagMapFromRuleContext(
+  public static Optional<ImmutableMap<Label, String>> getAndValidateFlagMapFromRuleContext(
       RuleContext ruleContext) throws RuleErrorException {
-    Map<Label, String> expectedValues =
-        NonconfigurableAttributeMapper.of(ruleContext.getRule())
-            .get(FEATURE_FLAG_ATTR, BuildType.LABEL_KEYED_STRING_DICT);
+    NonconfigurableAttributeMapper attrs = NonconfigurableAttributeMapper.of(ruleContext.getRule());
+
+    if (!attrs.isAttributeValueExplicitlySpecified(FEATURE_FLAG_ATTR)) {
+      return Optional.absent();
+    }
 
+    Map<Label, String> expectedValues =
+        attrs.get(FEATURE_FLAG_ATTR, BuildType.LABEL_KEYED_STRING_DICT);
     if (expectedValues.isEmpty()) {
-      return ImmutableMap.of();
+      return Optional.of(ImmutableMap.of());
     }
 
     if (!ConfigFeatureFlag.isAvailable(ruleContext)) {
@@ -90,23 +96,23 @@ public abstract class AndroidFeatureFlagSetProvider implements TransitiveInfoPro
                 target.getLabel(), label));
         aliasFound = true;
       }
-
-      String expectedValue = expectedValues.get(label);
-      String actualValue = ConfigFeatureFlagProvider.fromTarget(target).getValue();
-
-      if (!expectedValue.equals(actualValue)) {
-        // TODO(mstaib): when static configurations are removed, remove this error case
-        ruleContext.attributeError(
-            FEATURE_FLAG_ATTR,
-            "Setting " + FEATURE_FLAG_ATTR + " requires dynamic configurations to be enabled");
-        throw new RuleErrorException();
-      }
     }
     if (aliasFound) {
       throw new RuleErrorException();
     }
-    return ImmutableMap.copyOf(expectedValues);
+    return Optional.of(ImmutableMap.copyOf(expectedValues));
+  }
+
+  /**
+   * Returns whether it is acceptable to have a dependency with flags {@code depFlags} if the target
+   * has flags {@code targetFlags}.
+   */
+  public static boolean isValidDependency(
+      Optional<? extends Map<Label, String>> targetFlags,
+      Optional<? extends Map<Label, String>> depFlags) {
+    return !depFlags.isPresent()
+        || (targetFlags.isPresent() && targetFlags.get().equals(depFlags.get()));
   }
 
-  public abstract ImmutableMap<Label, String> getFlags();
+  public abstract Optional<ImmutableMap<Label, String>> getFlags();
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
index cc6aff0791..01c4e6b89b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
@@ -305,8 +305,7 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor
     javaCommon.addTransitiveInfoProviders(builder, filesToBuild, classJar);
     javaCommon.addGenJarsProvider(builder, genClassJar, genSourceJar);
 
-    // No need to use the flag map here - just confirming that dynamic configurations are in use.
-    // TODO(mstaib): remove when static configurations are removed.
+    // Just confirming that there are no aliases being used here.
     AndroidFeatureFlagSetProvider.getAndValidateFlagMapFromRuleContext(ruleContext);
 
     if (oneVersionOutputArtifact != null) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactory.java
index 757ca9e2a5..ea4f875dd3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactory.java
@@ -90,8 +90,13 @@ public class ConfigFeatureFlagTransitionFactory implements RuleTransitionFactory
 
   @Override
   public PatchTransition buildTransitionFor(Rule rule) {
-    return new ConfigFeatureFlagValuesTransition(
-        NonconfigurableAttributeMapper.of(rule).get(attributeName, LABEL_KEYED_STRING_DICT));
+    NonconfigurableAttributeMapper attrs = NonconfigurableAttributeMapper.of(rule);
+    if (attrs.isAttributeValueExplicitlySpecified(attributeName)) {
+      return new ConfigFeatureFlagValuesTransition(
+          attrs.get(attributeName, LABEL_KEYED_STRING_DICT));
+    } else {
+      return null;
+    }
   }
 
   @Override