Skip to content

Commit 9f3190f

Browse files
committed
Add support for config_settings in target_compatible_with
Previously you could only put constraint_values directly in target_compatible_with. Therefore for other configs you had to do something like: ```bzl target_compatible_with = select({ ":my_config_setting": [], "//conditions:default": ["@platforms//:incompatible"], }) ``` Now you can inline that just like config settings: ```bzl target_compatible_with = [":my_config_setting"], ``` Fixes #21857
1 parent 6c95102 commit 9f3190f

8 files changed

Lines changed: 235 additions & 54 deletions

File tree

src/main/java/com/google/devtools/build/docgen/templates/attributes/common/target_compatible_with.html

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,15 @@
55
<p>
66
A list of
77
<code><a href="platforms-and-toolchains.html#constraint_value">constraint_value</a></code>s
8-
that must be present in the target platform for this target to be considered
8+
that must be present in the target platform and
9+
<code><a href="general.html#config_setting">config_setting</a></code>s that
10+
must match the target configuration for this target to be considered
911
<em>compatible</em>. This is in addition to any constraints already set by the
10-
rule type. If the target platform does not satisfy all listed constraints then
11-
the target is considered <em>incompatible</em>. Incompatible targets are
12-
skipped for building and testing when the target pattern is expanded
13-
(e.g. <code>//...</code>, <code>:all</code>). When explicitly specified on the
12+
rule type. If the target platform does not satisfy all listed constraints, or
13+
the target configuration does not match all listed config_settings, then the
14+
target is considered <em>incompatible</em>. Incompatible targets are skipped
15+
for building and testing when the target pattern is expanded (e.g.
16+
<code>//...</code>, <code>:all</code>). When explicitly specified on the
1417
command line, incompatible targets cause Bazel to print an error and cause a
1518
build or test failure.
1619
</p>

src/main/java/com/google/devtools/build/lib/analysis/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1671,6 +1671,7 @@ java_library(
16711671
":visibility_provider",
16721672
"//src/main/java/com/google/devtools/build/lib/analysis/config:build_configuration",
16731673
"//src/main/java/com/google/devtools/build/lib/analysis/config:config_conditions",
1674+
"//src/main/java/com/google/devtools/build/lib/analysis/config:config_matching_provider",
16741675
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
16751676
"//src/main/java/com/google/devtools/build/lib/analysis/platform:utils",
16761677
"//src/main/java/com/google/devtools/build/lib/cmdline",

src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import com.google.common.collect.ImmutableList;
2929
import com.google.common.collect.ImmutableMap;
3030
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
31+
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
3132
import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
3233
import com.google.devtools.build.lib.analysis.config.RunUnder.LabelRunUnder;
3334
import com.google.devtools.build.lib.analysis.config.transitions.NoConfigTransition;
@@ -324,6 +325,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
324325
// has to be excluded in `IncompatibleTargetChecker`.
325326
.add(
326327
attr(RuleClass.TARGET_COMPATIBLE_WITH_ATTR, LABEL_LIST)
328+
.mandatoryBuiltinProviders(ImmutableList.of(ConfigMatchingProvider.class))
327329
.mandatoryProviders(ConstraintValueInfo.PROVIDER.id())
328330
// This should be configurable to allow for complex types of restrictions.
329331
.tool(

src/main/java/com/google/devtools/build/lib/analysis/IncompatiblePlatformProvider.java

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,28 +38,35 @@
3838
* <p>This provider is able to keep track of _why_ the corresponding target is considered
3939
* incompatible. If the target is incompatible because the target platform didn't satisfy one of the
4040
* constraints in target_compatible_with, then all the relevant constraints are accessible via
41-
* {@code getConstraintsResponsibleForIncompatibility()}. On the other hand, if the corresponding
42-
* target is incompatible because one of its dependencies is incompatible, then all the incompatible
43-
* dependencies are available via {@code getTargetResponsibleForIncompatibility()}.
41+
* {@code getConstraintsResponsibleForIncompatibility()}. If the target is incompatible because one
42+
* of the <code>config_setting</code> targets in target_compatible_with didn't match, then all the relevant
43+
* config_setting labels are accessible via {@code getConfigSettingsResponsibleForIncompatibility()}.
44+
* On the other hand, if the corresponding target is incompatible because one of its dependencies is
45+
* incompatible, then all the incompatible dependencies are available via {@code
46+
* getTargetResponsibleForIncompatibility()}.
4447
*
4548
* @param targetPlatform Returns the target platform of the target that was incompatible.
4649
* @param targetsResponsibleForIncompatibility Returns the incompatible dependencies that caused
4750
* this provider to be present.
48-
* <p>This may be null. If it is null, then {@code
49-
* getConstraintsResponsibleForIncompatibility()} is guaranteed to be non-null. It will have at
50-
* least one element in it if it is not null.
51+
* <p>This may be null. If it is null, then at least one of {@code
52+
* getConstraintsResponsibleForIncompatibility()} and {@code
53+
* getConfigSettingsResponsibleForIncompatibility()} is guaranteed to be non-null. It will have
54+
* at least one element in it if it is not null.
5155
* @param constraintsResponsibleForIncompatibility Returns the constraints that the target platform
5256
* didn't satisfy.
53-
* <p>This may be null. If it is null, then {@code getTargetsResponsibleForIncompatibility()} is
54-
* guaranteed to be non-null. It will have at least one element in it if it is not null.
57+
* <p>This may be null. It will have at least one element in it if it is not null.
5558
* <p>The list is sorted based on the stringified label of each constraint.
59+
* @param configSettingsResponsibleForIncompatibility Returns the config_settings that didn't match.
60+
* <p>This may be null. It will have at least one element in it if it is not null.
61+
* <p>The list is sorted based on the stringified label of each config_setting.
5662
*/
5763
@Immutable
5864
@AutoCodec
5965
public record IncompatiblePlatformProvider(
6066
@Nullable Label targetPlatform,
6167
@Nullable ImmutableList<ConfiguredTarget> targetsResponsibleForIncompatibility,
62-
@Nullable ImmutableList<ConstraintValueInfo> constraintsResponsibleForIncompatibility)
68+
@Nullable ImmutableList<ConstraintValueInfo> constraintsResponsibleForIncompatibility,
69+
@Nullable ImmutableList<Label> configSettingsResponsibleForIncompatibility)
6370
implements Info, IncompatiblePlatformProviderApi {
6471
/** Name used in Starlark for accessing this provider. */
6572
public static final String STARLARK_NAME = "IncompatiblePlatformProvider";
@@ -80,13 +87,28 @@ public static IncompatiblePlatformProvider incompatibleDueToTargets(
8087
Preconditions.checkNotNull(targetsResponsibleForIncompatibility);
8188
Preconditions.checkArgument(!targetsResponsibleForIncompatibility.isEmpty());
8289
return new IncompatiblePlatformProvider(
83-
targetPlatform, targetsResponsibleForIncompatibility, null);
90+
targetPlatform, targetsResponsibleForIncompatibility, null, null);
8491
}
8592

8693
public static IncompatiblePlatformProvider incompatibleDueToConstraints(
8794
@Nullable Label targetPlatform, ImmutableList<ConstraintValueInfo> constraints) {
95+
return incompatibleDueToConstraintsAndConfigSettings(
96+
targetPlatform, constraints, ImmutableList.of());
97+
}
98+
99+
public static IncompatiblePlatformProvider incompatibleDueToConfigSettings(
100+
@Nullable Label targetPlatform, ImmutableList<Label> configSettings) {
101+
return incompatibleDueToConstraintsAndConfigSettings(
102+
targetPlatform, ImmutableList.of(), configSettings);
103+
}
104+
105+
public static IncompatiblePlatformProvider incompatibleDueToConstraintsAndConfigSettings(
106+
@Nullable Label targetPlatform,
107+
ImmutableList<ConstraintValueInfo> constraints,
108+
ImmutableList<Label> configSettings) {
88109
Preconditions.checkNotNull(constraints);
89-
Preconditions.checkArgument(!constraints.isEmpty());
110+
Preconditions.checkNotNull(configSettings);
111+
Preconditions.checkArgument(!constraints.isEmpty() || !configSettings.isEmpty());
90112

91113
// Deduplicate and sort the list of incompatible constraints. Doing it here means that everyone
92114
// inspecting this provider doesn't have to deal with it.
@@ -96,7 +118,17 @@ public static IncompatiblePlatformProvider incompatibleDueToConstraints(
96118
.distinct()
97119
.collect(toImmutableList());
98120

99-
return new IncompatiblePlatformProvider(targetPlatform, null, constraints);
121+
configSettings =
122+
configSettings.stream()
123+
.sorted(Comparator.naturalOrder())
124+
.distinct()
125+
.collect(toImmutableList());
126+
127+
return new IncompatiblePlatformProvider(
128+
targetPlatform,
129+
null,
130+
constraints.isEmpty() ? null : constraints,
131+
configSettings.isEmpty() ? null : configSettings);
100132
}
101133

102134
@Override

src/main/java/com/google/devtools/build/lib/analysis/constraints/IncompatibleTargetChecker.java

Lines changed: 48 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
package com.google.devtools.build.lib.analysis.constraints;
1515

1616
import static com.google.common.collect.ImmutableList.toImmutableList;
17+
import static java.util.stream.Collectors.joining;
1718

1819
import com.google.common.collect.ImmutableList;
1920
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -30,6 +31,8 @@
3031
import com.google.devtools.build.lib.analysis.VisibilityProvider;
3132
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
3233
import com.google.devtools.build.lib.analysis.config.ConfigConditions;
34+
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
35+
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider.AccumulateResults;
3336
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
3437
import com.google.devtools.build.lib.analysis.platform.ConstraintCollection;
3538
import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
@@ -65,7 +68,8 @@
6568
*
6669
* <ol>
6770
* <li>The target's <code>target_compatible_with</code> attribute specifies a constraint that is
68-
* not present in the target platform. The target is said to be "directly incompatible".
71+
* not present in the target platform or a config_setting that does not match the target
72+
* configuration. The target is said to be "directly incompatible".
6973
* <li>One or more of the target's dependencies is incompatible. The target is said to be
7074
* "indirectly incompatible."
7175
* </ol>
@@ -84,8 +88,8 @@ public class IncompatibleTargetChecker {
8488
/**
8589
* Creates an incompatible configured target if it is "directly incompatible".
8690
*
87-
* <p>In other words, this state machine checks if a target is incompatible because of its
88-
* "target_compatible_with" attribute.
91+
* <p>In other words, this state machine checks if a target is incompatible because of constraints
92+
* or <code>config_settings</code> in its "target_compatible_with" attribute.
8993
*
9094
* <p>Outputs an {@code Optional} {@link RuleConfiguredTargetValue} as follows.
9195
*
@@ -111,6 +115,8 @@ public static class IncompatibleTargetProducer implements StateMachine, Consumer
111115
new ImmutableList.Builder<>();
112116
private final ImmutableList.Builder<ConstraintValueInfo> invalidConstraintValuesBuilder =
113117
new ImmutableList.Builder<>();
118+
private final ImmutableList.Builder<ConfigMatchingProvider> configSettingsBuilder =
119+
new ImmutableList.Builder<>();
114120

115121
/** Sink for the output of this state machine. */
116122
public interface ResultSink {
@@ -173,17 +179,28 @@ public StateMachine step(Tasks tasks) {
173179
public void accept(SkyValue value) {
174180
var configuredTarget = ((ConfiguredTargetValue) value).getConfiguredTarget();
175181
@Nullable ConstraintValueInfo info = PlatformProviderUtils.constraintValue(configuredTarget);
176-
if (info == null) {
177-
return;
182+
if (info != null) {
183+
allConstraintValuesBuilder.add(info);
184+
if (!platformInfo.constraints().hasConstraintValue(info)) {
185+
invalidConstraintValuesBuilder.add(info);
186+
}
178187
}
179-
allConstraintValuesBuilder.add(info);
180-
if (!platformInfo.constraints().hasConstraintValue(info)) {
181-
invalidConstraintValuesBuilder.add(info);
188+
@Nullable ConfigMatchingProvider configSetting =
189+
configuredTarget.getProvider(ConfigMatchingProvider.class);
190+
if (configSetting != null) {
191+
configSettingsBuilder.add(
192+
ConfigMatchingProvider.create(
193+
configuredTarget.getOriginalLabel(),
194+
configSetting.settingsMap(),
195+
configSetting.flagSettingsMap(),
196+
configSetting.constraintValuesSetting(),
197+
configSetting.result()));
182198
}
183199
}
184200

185201
private StateMachine processResult(Tasks tasks) {
186202
var allConstraintValues = allConstraintValuesBuilder.build();
203+
var configSettings = configSettingsBuilder.build();
187204

188205
// Validate that there are no duplicate constraint values from the same constraint setting
189206
try {
@@ -193,16 +210,26 @@ private StateMachine processResult(Tasks tasks) {
193210
return runAfter;
194211
}
195212

213+
AccumulateResults configSettingResults =
214+
ConfigMatchingProvider.accumulateMatchResults(configSettings);
215+
if (!configSettingResults.errors().isEmpty()) {
216+
sink.acceptValidationException(
217+
new ValidationException(formatConfigSettingErrors(configSettingResults)));
218+
return runAfter;
219+
}
220+
196221
var invalidConstraintValues = invalidConstraintValuesBuilder.build();
197-
if (!invalidConstraintValues.isEmpty()) {
222+
if (!invalidConstraintValues.isEmpty() || !configSettingResults.nonMatching().isEmpty()) {
198223
sink.acceptIncompatibleTarget(
199224
Optional.of(
200225
createIncompatibleRuleConfiguredTarget(
201226
configuredTargetKey,
202227
targetAndConfiguration.getConfiguration(),
203228
configConditions,
204-
IncompatiblePlatformProvider.incompatibleDueToConstraints(
205-
platformInfo.label(), invalidConstraintValues),
229+
IncompatiblePlatformProvider.incompatibleDueToConstraintsAndConfigSettings(
230+
platformInfo.label(),
231+
invalidConstraintValues,
232+
configSettingResults.nonMatching()),
206233
targetAndConfiguration
207234
.getTarget()
208235
.getAssociatedRule()
@@ -214,6 +241,16 @@ private StateMachine processResult(Tasks tasks) {
214241
sink.acceptIncompatibleTarget(Optional.empty());
215242
return runAfter;
216243
}
244+
245+
private static String formatConfigSettingErrors(AccumulateResults configSettingResults) {
246+
return configSettingResults.errors().asMap().entrySet().stream()
247+
.map(
248+
entry ->
249+
String.format(
250+
"For config_setting %s: %s",
251+
entry.getKey(), String.join(", ", entry.getValue())))
252+
.collect(joining("; "));
253+
}
217254
}
218255

219256
/** Rules where it doesn't make sense to check for platform compatibility. */

src/main/java/com/google/devtools/build/lib/analysis/constraints/TopLevelConstraintSemantics.java

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,8 @@ public PlatformRestrictionsResult checkPlatformRestrictions(
275275
*
276276
* <p>This is useful when trying to explain to the user why an explicitly requested target on the
277277
* command line is considered incompatible. The goal is to print out the dependency chain and the
278-
* constraint that wasn't satisfied so that the user can immediately figure out what happened.
278+
* constraint or <code>config_setting</code> that wasn't satisfied so that the user can immediately figure out
279+
* what happened.
279280
*
280281
* @param target the incompatible target that was explicitly requested on the command line.
281282
* @return the verbose error message to show to the user.
@@ -304,28 +305,50 @@ private static String reportOnIncompatibility(ConfiguredTarget target) {
304305

305306
message +=
306307
String.format(
307-
" <-- target platform (%s) didn't satisfy constraint", provider.targetPlatform());
308-
if (provider.constraintsResponsibleForIncompatibility().size() == 1) {
309-
message += " " + provider.constraintsResponsibleForIncompatibility().get(0).label();
310-
return message;
311-
}
312-
313-
message += "s [";
308+
" <--%s",
309+
formatIncompatibilityReasons(
310+
provider.targetPlatform(),
311+
provider.constraintsResponsibleForIncompatibility(),
312+
provider.configSettingsResponsibleForIncompatibility()));
313+
return message;
314+
}
314315

315-
boolean first = true;
316-
for (ConstraintValueInfo constraintValueInfo :
317-
provider.constraintsResponsibleForIncompatibility()) {
318-
if (first) {
319-
first = false;
320-
} else {
321-
message += ", ";
322-
}
323-
message += constraintValueInfo.label();
316+
private static String formatIncompatibilityReasons(
317+
@Nullable Label targetPlatform,
318+
@Nullable ImmutableList<ConstraintValueInfo> constraints,
319+
@Nullable ImmutableList<Label> configSettings) {
320+
StringJoiner reasons = new StringJoiner(" and");
321+
if (constraints != null) {
322+
reasons.add(formatConstraintIncompatibility(targetPlatform, constraints));
324323
}
324+
if (configSettings != null) {
325+
reasons.add(formatConfigSettingIncompatibility(configSettings));
326+
}
327+
return reasons.toString();
328+
}
325329

326-
message += "]";
330+
private static String formatConstraintIncompatibility(
331+
@Nullable Label targetPlatform, ImmutableList<ConstraintValueInfo> constraints) {
332+
String message =
333+
String.format(" target platform (%s) didn't satisfy constraint", targetPlatform);
334+
if (constraints.size() == 1) {
335+
return message + " " + constraints.get(0).label();
336+
}
337+
return message
338+
+ "s ["
339+
+ constraints.stream()
340+
.map(constraintValueInfo -> constraintValueInfo.label().toString())
341+
.collect(joining(", "))
342+
+ "]";
343+
}
327344

328-
return message;
345+
private static String formatConfigSettingIncompatibility(ImmutableList<Label> configSettings) {
346+
if (configSettings.size() == 1) {
347+
return " target configuration didn't match config_setting " + configSettings.get(0);
348+
}
349+
return " target configuration didn't match config_settings ["
350+
+ configSettings.stream().map(Label::toString).collect(joining(", "))
351+
+ "]";
329352
}
330353

331354
/**

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import com.google.devtools.build.lib.analysis.PackageSpecificationProvider;
4646
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
4747
import com.google.devtools.build.lib.analysis.TemplateVariableInfo;
48+
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
4849
import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
4950
import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
5051
import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
@@ -183,6 +184,7 @@ public class StarlarkRuleClassFunctions implements StarlarkRuleFunctionsApi {
183184
.value(ImmutableMap.of()))
184185
.add(
185186
attr(RuleClass.TARGET_COMPATIBLE_WITH_ATTR, LABEL_LIST)
187+
.mandatoryBuiltinProviders(ImmutableList.of(ConfigMatchingProvider.class))
186188
.mandatoryProviders(ConstraintValueInfo.PROVIDER.id())
187189
// This should be configurable to allow for complex types of restrictions.
188190
.tool(

0 commit comments

Comments
 (0)