Allow config_settings to match on constraint_values to a target_platf…

…orm. This is on the way to making select() work with constraint_values re: #305.

PiperOrigin-RevId: 169454982

src/main/java/com/google/devtools/build/lib/rules/config/BUILD

@@ -21,6 +21,8 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib:preconditions",
         "//src/main/java/com/google/devtools/build/lib:skylarkinterface",
         "//src/main/java/com/google/devtools/build/lib/actions",
+        "//src/main/java/com/google/devtools/build/lib/analysis/platform",
+        "//src/main/java/com/google/devtools/build/lib/analysis/platform:utils",
         "//src/main/java/com/google/devtools/build/lib/analysis/whitelisting",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/collect",

src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java

@@ -39,6 +39,10 @@
 import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
 import com.google.devtools.build.lib.analysis.config.TransitiveOptionDetails;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
+import com.google.devtools.build.lib.analysis.platform.ConstraintSettingInfo;
+import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
+import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
+import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.BuildType;
@@ -51,6 +55,7 @@
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingException;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -91,25 +96,37 @@ public ConfiguredTarget create(RuleContext ruleContext)
         ruleContext.getPrerequisites(
             ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE, Mode.TARGET);
 
-    if (nativeFlagSettings.size() == 0 && userDefinedFlagSettings.isEmpty()) {
-      ruleContext.ruleError(
-          String.format(
-              "Either %s or %s must be specified and non-empty",
-              ConfigSettingRule.SETTINGS_ATTRIBUTE,
-              ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE));
+    // Get the constraint values that match this rule
+    Iterable<ConstraintValueInfo> constraintValues =
+        PlatformProviderUtils.constraintValues(
+            ruleContext.getPrerequisites(
+                ConfigSettingRule.CONSTRAINT_VALUES_ATTRIBUTE, Mode.DONT_CHECK));
+
+    // Get the target platform
+    Iterable<PlatformInfo> targetPlatforms =
+        PlatformProviderUtils.platforms(
+            ruleContext.getPrerequisites(
+                ConfigSettingRule.TARGET_PLATFORMS_ATTRIBUTE, Mode.DONT_CHECK));
+
+    // Check that this config_setting contains at least one of {values, define_values,
+    // constraint_values}
+    if (!checkValidConditions(
+        nativeFlagSettings, userDefinedFlagSettings, constraintValues, ruleContext)) {
       return null;
     }
 
-    ConfigFeatureFlagMatch featureFlags =
-        ConfigFeatureFlagMatch.fromAttributeValueAndPrerequisites(
-            userDefinedFlagSettings, flagValues, ruleContext);
-
     boolean nativeFlagsMatch =
         matchesConfig(
             nativeFlagSettings.entries(),
             BuildConfigurationOptionDetails.get(ruleContext.getConfiguration()),
             ruleContext);
 
+    ConfigFeatureFlagMatch featureFlags =
+        ConfigFeatureFlagMatch.fromAttributeValueAndPrerequisites(
+            userDefinedFlagSettings, flagValues, ruleContext);
+
+    boolean constraintValuesMatch = matchesConstraints(constraintValues, targetPlatforms);
+
     if (ruleContext.hasErrors()) {
       return null;
     }
@@ -119,7 +136,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
             ruleContext.getLabel(),
             nativeFlagSettings,
             featureFlags.getSpecifiedFlagValues(),
-            nativeFlagsMatch && featureFlags.matches());
+            nativeFlagsMatch && featureFlags.matches() && constraintValuesMatch);
 
     return new RuleConfiguredTargetBuilder(ruleContext)
         .addProvider(RunfilesProvider.class, RunfilesProvider.EMPTY)
@@ -130,11 +147,79 @@ public ConfiguredTarget create(RuleContext ruleContext)
         .build();
   }
 
+  private boolean checkValidConditions(
+      ImmutableMultimap<String, String> nativeFlagSettings,
+      Map<Label, String> userDefinedFlagSettings,
+      Iterable<ConstraintValueInfo> constraintValues,
+      RuleErrorConsumer errors) {
+    // Check to make sure this config_setting contains and sets least one of {values, define_values,
+    // flag_value or constraint_values}.
+    if (!valuesAreSet(nativeFlagSettings, userDefinedFlagSettings, constraintValues, errors)) {
+      return false;
+    }
+
+    // The set of constraint_values in a config_setting should never contain multiple
+    // constraint_values that map to the same constraint_setting. This checks if there are
+    // duplicates and records an error if so.
+    if (containsDuplicateSettings(constraintValues, errors)) {
+      return false;
+    }
+
+    return true;
+  }
+
   /**
    * User error when value settings can't be properly parsed.
    */
   private static final String PARSE_ERROR_MESSAGE = "error while parsing configuration settings: ";
 
+  private boolean valuesAreSet(
+      ImmutableMultimap<String, String> nativeFlagSettings,
+      Map<Label, String> userDefinedFlagSettings,
+      Iterable<ConstraintValueInfo> constraintValues,
+      RuleErrorConsumer errors) {
+    if (nativeFlagSettings.isEmpty()
+        && userDefinedFlagSettings.isEmpty()
+        && Iterables.isEmpty(constraintValues)) {
+      errors.ruleError(
+          String.format(
+              "Either %s, %s or %s must be specified and non-empty",
+              ConfigSettingRule.SETTINGS_ATTRIBUTE,
+              ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE,
+              ConfigSettingRule.CONSTRAINT_VALUES_ATTRIBUTE));
+      return false;
+    }
+    return true;
+  }
+
+
+  /**
+   * The set of constraint_values in a config_setting should never contain multiple
+   * constraint_values that map to the same constraint_setting. This method checks if there are
+   * duplicates and records an error if so.
+   */
+  private boolean containsDuplicateSettings(
+      Iterable<ConstraintValueInfo> constraintValues, RuleErrorConsumer errors) {
+    HashMap<ConstraintSettingInfo, ConstraintValueInfo> constraints = new HashMap<>();
+    for (ConstraintValueInfo constraint : constraintValues) {
+      ConstraintSettingInfo setting = constraint.constraint();
+      if (constraints.containsKey(setting)) {
+        errors.attributeError(
+            ConfigSettingRule.TARGET_PLATFORMS_ATTRIBUTE,
+            String.format(
+                PARSE_ERROR_MESSAGE
+                    + "the target platform contains multiple values '%s' "
+                    + "and '%s' that map to the same setting '%s'",
+                constraint.label(),
+                constraints.get(setting).label(),
+                setting.label()));
+        return true;
+      }
+      constraints.put(setting, constraint);
+    }
+    return false;
+  }
+
   /**
    * Given a list of [flagName, flagValue] pairs for native Blaze flags, returns true if
    * flagName == flagValue for every item in the list under this configuration, false otherwise.
@@ -244,6 +329,33 @@ private static boolean optionMatches(
     return actualList.contains(expectedSingleValue);
   }
 
+  private boolean matchesConstraints(
+      Iterable<ConstraintValueInfo> expected, Iterable<PlatformInfo> targetPlatforms) {
+    // config_setting didn't specify any constraint values
+    if (Iterables.isEmpty(expected)) {
+      return true;
+    }
+
+    // TODO(jcater): re-evaluate this for multiple target platforms.
+    PlatformInfo targetPlatform = Iterables.getOnlyElement(targetPlatforms);
+    // config_setting DID specify constraint_value(s) but no target platforms are set
+    // in the configuration.
+    if (Iterables.isEmpty(targetPlatform.constraints())) {
+      return false;
+    }
+
+    // For every constraint in the attr check if it is (1)set aka non-null and
+    // (2)set correctly in the platform.
+    for (ConstraintValueInfo constraint : expected) {
+      ConstraintSettingInfo setting = constraint.constraint();
+      ConstraintValueInfo targetValue = targetPlatform.getConstraint(setting);
+      if (targetValue == null || !constraint.equals(targetValue)) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   private static final class ConfigFeatureFlagMatch {
     private final boolean matches;
     private final ImmutableMap<Label, String> specifiedFlagValues;
@@ -338,3 +450,4 @@ public static ConfigFeatureFlagMatch fromAttributeValueAndPrerequisites(
     }
   }
 }
+

src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java

@@ -239,9 +239,11 @@ public void invalidOptionFartherDown() throws Exception {
    */
   @Test
   public void emptySettings() throws Exception {
-    checkError("foo", "empty",
+    checkError(
+        "foo",
+        "empty",
         "in config_setting rule //foo:empty: "
-        + "Either values or flag_values must be specified and non-empty",
+            + "Either values, flag_values or constraint_values must be specified and non-empty",
         "config_setting(",
         "    name = 'empty',",
         "    values = {})");
@@ -1122,4 +1124,130 @@ public void usesAliasLabelWhenReportingErrorInFlagValues() throws Exception {
         "    default_value = 'valid',",
         ")");
   }
+
+  @Test
+  public void constraintValue() throws Exception {
+    scratch.file(
+        "test/BUILD",
+        "constraint_setting(name = 'notable_building')",
+        "constraint_value(name = 'empire_state', constraint_setting = 'notable_building')",
+        "constraint_value(name = 'space_needle', constraint_setting = 'notable_building')",
+        "platform(",
+        "    name = 'new_york_platform',",
+        "    constraint_values = [':empire_state'],",
+        ")",
+        "platform(",
+        "    name = 'seattle_platform',",
+        "    constraint_values = [':space_needle'],",
+        ")",
+        "config_setting(",
+        "    name = 'match',",
+        "    constraint_values = [':empire_state'],",
+        ");");
+
+    useConfiguration("--experimental_platforms=//test:new_york_platform");
+    assertThat(getConfigMatchingProvider("//test:match").matches()).isTrue();
+    useConfiguration("--experimental_platforms=//test:seattle_platform");
+    assertThat(getConfigMatchingProvider("//test:match").matches()).isFalse();
+    useConfiguration("");
+    assertThat(getConfigMatchingProvider("//test:match").matches()).isFalse();
+  }
+
+  @Test
+  public void multipleConstraintValues() throws Exception {
+    scratch.file(
+        "test/BUILD",
+        "constraint_setting(name = 'notable_building')",
+        "constraint_value(name = 'empire_state', constraint_setting = 'notable_building')",
+        "constraint_setting(name = 'museum')",
+        "constraint_value(name = 'cloisters', constraint_setting = 'museum')",
+        "constraint_setting(name = 'theme_park')",
+        "constraint_value(name = 'coney_island', constraint_setting = 'theme_park')",
+        "platform(",
+        "    name = 'manhattan_platform',",
+        "    constraint_values = [",
+        "        ':empire_state',",
+        "        ':cloisters',",
+        "    ],",
+        ")",
+        "platform(",
+        "    name = 'museum_platform',",
+        "    constraint_values = [':cloisters'],",
+        ")",
+        "platform(",
+        "    name = 'new_york_platform',",
+        "    constraint_values = [",
+        "        ':empire_state',",
+        "        ':cloisters',",
+        "        ':coney_island',",
+        "    ],",
+        ")",
+        "config_setting(",
+        "    name = 'match',",
+        "    constraint_values = [':empire_state', ':cloisters'],",
+        ");");
+    useConfiguration("--experimental_platforms=//test:manhattan_platform");
+    assertThat(getConfigMatchingProvider("//test:match").matches()).isTrue();
+    useConfiguration("--experimental_platforms=//test:museum_platform");
+    assertThat(getConfigMatchingProvider("//test:match").matches()).isFalse();
+    useConfiguration("--experimental_platforms=//test:new_york_platform");
+    assertThat(getConfigMatchingProvider("//test:match").matches()).isTrue();
+  }
+
+  @Test
+  public void definesAndConstraints() throws Exception {
+    scratch.file(
+        "test/BUILD",
+        "constraint_setting(name = 'notable_building')",
+        "constraint_value(name = 'empire_state', constraint_setting = 'notable_building')",
+        "constraint_value(name = 'space_needle', constraint_setting = 'notable_building')",
+        "platform(",
+        "    name = 'new_york_platform',",
+        "    constraint_values = [':empire_state'],",
+        ")",
+        "platform(",
+        "    name = 'seattle_platform',",
+        "    constraint_values = [':space_needle'],",
+        ")",
+        "config_setting(",
+        "    name = 'match',",
+        "    constraint_values = [':empire_state'],",
+        "    values = {",
+        "        'define': 'a=c',",
+        "    },",
+        "    define_values = {",
+        "        'b': 'd',",
+        "    },",
+        ");");
+
+    useConfiguration(
+        "--experimental_platforms=//test:new_york_platform", "--define", "a=c", "--define", "b=d");
+    assertThat(getConfigMatchingProvider("//test:match").matches()).isTrue();
+    useConfiguration("--experimental_platforms=//test:new_york_platform");
+    assertThat(getConfigMatchingProvider("//test:match").matches()).isFalse();
+    useConfiguration("--define", "a=c");
+    assertThat(getConfigMatchingProvider("//test:match").matches()).isFalse();
+    useConfiguration("--define", "a=c", "--experimental_platforms=//test:new_york_platform");
+    assertThat(getConfigMatchingProvider("//test:match").matches()).isFalse();
+  }
+
+  /**
+   * Tests that a config_setting doesn't allow a constraint_values list with more than one
+   * constraint value per constraint setting.
+   */
+  @Test
+  public void multipleValuesPerSetting() throws Exception {
+    checkError("foo", "bad",
+        "in :target_platforms attribute of config_setting rule //foo:bad: "
+            + "error while parsing configuration settings: "
+            + "the target platform contains multiple values '//foo:space_needle' "
+            + "and '//foo:empire_state' that map to the same setting '//foo:notable_building'",
+        "constraint_setting(name = 'notable_building')",
+        "constraint_value(name = 'empire_state', constraint_setting = 'notable_building')",
+        "constraint_value(name = 'space_needle', constraint_setting = 'notable_building')",
+        "config_setting(",
+        "    name = 'bad',",
+        "    constraint_values = [':empire_state', ':space_needle'],",
+        ");");
+  }
 }