Skip to content

Commit

Permalink
Allow select() specialization resolution to work with constraint_values.
Browse files Browse the repository at this point in the history
#14604

RELNOTES: Allow specialization to work with constraint_values.
PiperOrigin-RevId: 440367861
  • Loading branch information
Googler authored and copybara-github committed Apr 8, 2022
1 parent 5179d03 commit 240af80
Show file tree
Hide file tree
Showing 5 changed files with 237 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -616,10 +616,8 @@ 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 and constraint values.
</li>
<li>If multiple conditions match and one is not a specialization of all the
others, Bazel fails with an error.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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.
Expand All @@ -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. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -130,6 +131,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
ruleContext.shouldIncludeRequiredConfigFragmentsProvider()
? ruleContext.getRequiredConfigFragments()
: RequiredConfigFragmentsProvider.EMPTY,
ImmutableSet.copyOf(constraintValueSettings),
nativeFlagsMatch && userDefinedFlags.matches() && constraintValuesMatch);

return new RuleConfiguredTargetBuilder(ruleContext)
Expand Down
Loading

0 comments on commit 240af80

Please sign in to comment.