Roll forward to allow select() specialization resolution to work with constraint_values.

Googler · copybara-github · commit 108c17859c25 · 2022-04-26T07:15:18.000-07:00
PiperOrigin-RevId: 444550390
diff --git a/src/main/java/com/google/devtools/build/docgen/templates/be/functions.vm b/src/main/java/com/google/devtools/build/docgen/templates/be/functions.vm
@@ -616,10 +616,10 @@ sh_binary(
   </li>
   <li>If multiple conditions match and one is a specialization of the others,
     the specialization takes precedence. Condition B is considered a
-    specialization of condition A if B has all the same flags as A plus some
-    additional flags. However, the number of constraint values that A and B have
-    are not considered in this comparison -- one condition cannot match a
-    platform <i>more than</i> another condition does.
+    specialization of condition A if B has all the same flags and constraint
+    values as A plus some additional flags or constraint values. This also
+    means that specialization resolution is not designed to create an ordering as
+    demonstrated in Example 2 below.
   </li>
   <li>If multiple conditions match and one is not a specialization of all the
     others, Bazel fails with an error.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigMatchingProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigMatchingProvider.java
@@ -50,9 +50,15 @@ public static ConfigMatchingProvider create(
       ImmutableMultimap<String, String> settingsMap,
       ImmutableMap<Label, String> flagSettingsMap,
       RequiredConfigFragmentsProvider requiredFragmentOptions,
+      ImmutableSet<Label> constraintValueSettings,
       boolean matches) {
     return new AutoValue_ConfigMatchingProvider(
-        label, settingsMap, flagSettingsMap, requiredFragmentOptions, matches);
+        label,
+        settingsMap,
+        flagSettingsMap,
+        requiredFragmentOptions,
+        constraintValueSettings,
+        matches);
   }
 
   /** The target's label. */
@@ -64,6 +70,8 @@ public static ConfigMatchingProvider create(
 
   public abstract RequiredConfigFragmentsProvider requiredFragmentOptions();
 
+  abstract ImmutableSet<Label> constraintValuesSetting();
+
   /**
    * Whether or not the configuration criteria defined by this target match its actual
    * configuration.
@@ -81,11 +89,18 @@ public boolean refines(ConfigMatchingProvider other) {
     ImmutableSet<Map.Entry<Label, String>> flagSettings = flagSettingsMap().entrySet();
     ImmutableSet<Map.Entry<Label, String>> otherFlagSettings = other.flagSettingsMap().entrySet();
 
-    if (!settings.containsAll(otherSettings) || !flagSettings.containsAll(otherFlagSettings)) {
+    ImmutableSet<Label> constraintValueSettings = constraintValuesSetting();
+    ImmutableSet<Label> otherConstraintValueSettings = other.constraintValuesSetting();
+
+    if (!settings.containsAll(otherSettings)
+        || !flagSettings.containsAll(otherFlagSettings)
+        || !constraintValueSettings.containsAll(otherConstraintValueSettings)) {
       return false; // Not a superset.
     }
 
-    return settings.size() > otherSettings.size() || flagSettings.size() > otherFlagSettings.size();
+    return settings.size() > otherSettings.size()
+        || flagSettings.size() > otherFlagSettings.size()
+        || constraintValueSettings.size() > otherConstraintValueSettings.size();
   }
 
   /** Format this provider as its label. */
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintValueInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintValueInfo.java
@@ -16,6 +16,7 @@
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableMultimap;
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.RequiredConfigFragmentsProvider;
 import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -82,6 +83,7 @@ public ConfigMatchingProvider configMatchingProvider(PlatformInfo platformInfo)
         // the owning target already depends on PlatformConfiguration. And we can't reference
         // PlatformConfiguration.class here without a build dependency cycle.
         RequiredConfigFragmentsProvider.EMPTY,
+        ImmutableSet.of(),
         platformInfo.constraints().hasConstraintValue(this));
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java
@@ -23,6 +23,7 @@
 import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableMultimap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Lists;
@@ -132,6 +133,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
             ruleContext.shouldIncludeRequiredConfigFragmentsProvider()
                 ? ruleContext.getRequiredConfigFragments()
                 : RequiredConfigFragmentsProvider.EMPTY,
+            ImmutableSet.copyOf(constraintValueSettings),
             nativeFlagsMatch && userDefinedFlags.matches() && constraintValuesMatch);
 
     return new RuleConfiguredTargetBuilder(ruleContext)
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
