Skip to content

Commit 0fe4c36

Browse files
aiutocopybara-github
authored andcommitted
Self transition constraint_setting and constraint_value to the NoConfig state.
These rules can only define constants, and all their attributes are not configurable. This reduces the number of configurations constraint values appear in, but does yet reduce that to a single instance per value. For this cquery ``` blaze cquery --compilation_mode=opt --fat_apk_cpu=x86 \ --android_platforms=//buildenv/platforms/android:x86 \ --incompatible_enable_android_toolchain_resolution=1 \ --//third_party/java/android:system_api=true \ 'allpaths(//java/com/android/dialer:dialer, //third_party/bazel_platforms/...)' \ | grep bazel_platforms/os | sort -u ``` We see 71 different combinations of constraint values and cquery configs before this change and 37 after. That is better, but inexplicably much higher than the 8 or so I would expect. RELNOTES: None PiperOrigin-RevId: 501909093 Change-Id: Ie66ac2b144837050fdb3e1fd18f5dc1ba155e480
1 parent d429313 commit 0fe4c36

File tree

5 files changed

+49
-4
lines changed

5 files changed

+49
-4
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ java_library(
2020
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
2121
"//src/main/java/com/google/devtools/build/lib/analysis:config/auto_cpu_converter",
2222
"//src/main/java/com/google/devtools/build/lib/analysis:config/config_matching_provider",
23+
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/no_config_transition",
2324
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
2425
"//src/main/java/com/google/devtools/build/lib/analysis:file_provider",
2526
"//src/main/java/com/google/devtools/build/lib/analysis:platform_configuration",

src/main/java/com/google/devtools/build/lib/rules/platform/ConstraintSettingRule.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,16 @@
1616

1717
import static com.google.devtools.build.lib.packages.Attribute.attr;
1818

19+
import com.google.common.collect.ImmutableList;
20+
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
1921
import com.google.devtools.build.lib.analysis.RuleDefinition;
2022
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
23+
import com.google.devtools.build.lib.analysis.config.transitions.NoConfigTransition;
2124
import com.google.devtools.build.lib.analysis.platform.ConstraintSettingInfo;
2225
import com.google.devtools.build.lib.packages.BuildType;
2326
import com.google.devtools.build.lib.packages.RuleClass;
27+
import com.google.devtools.build.lib.packages.RuleClass.ToolchainResolutionMode;
28+
import com.google.devtools.build.lib.packages.Type;
2429

2530
/** Rule definition for {@link ConstraintSetting}. */
2631
public class ConstraintSettingRule implements RuleDefinition {
@@ -31,6 +36,20 @@ public class ConstraintSettingRule implements RuleDefinition {
3136
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
3237
return builder
3338
.advertiseStarlarkProvider(ConstraintSettingInfo.PROVIDER.id())
39+
.cfg(NoConfigTransition.createFactory())
40+
.exemptFromConstraintChecking("this rule helps *define* a constraint")
41+
.useToolchainResolution(ToolchainResolutionMode.DISABLED)
42+
.override(
43+
attr("applicable_licenses", BuildType.LABEL_LIST)
44+
// This is a constant which is never linked into a target
45+
.value(ImmutableList.of())
46+
.allowedFileTypes()
47+
.nonconfigurable("fundamental constant, used in platform configuration"))
48+
.override(
49+
attr("tags", Type.STRING_LIST)
50+
// No need to show up in ":all", etc. target patterns.
51+
.value(ImmutableList.of("manual"))
52+
.nonconfigurable("low-level attribute, used in platform configuration"))
3453
/* <!-- #BLAZE_RULE(constraint_setting).ATTRIBUTE(default_constraint_value) -->
3554
The label of the default value for this setting, to be used if no value is given. If this
3655
attribute is present, the <code>constraint_value</code> it points to must be defined in the
@@ -53,7 +72,7 @@ constraint list (such as for a <code>config_setting</code>) that requires a part
5372
public RuleDefinition.Metadata getMetadata() {
5473
return RuleDefinition.Metadata.builder()
5574
.name(RULE_NAME)
56-
.ancestors(PlatformBaseRule.class)
75+
.ancestors(BaseRuleClasses.NativeBuildRule.class)
5776
.factoryClass(ConstraintSetting.class)
5877
.build();
5978
}

src/main/java/com/google/devtools/build/lib/rules/platform/ConstraintValueRule.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,17 @@
1616

1717
import static com.google.devtools.build.lib.packages.Attribute.attr;
1818

19+
import com.google.common.collect.ImmutableList;
20+
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
1921
import com.google.devtools.build.lib.analysis.RuleDefinition;
2022
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
23+
import com.google.devtools.build.lib.analysis.config.transitions.NoConfigTransition;
2124
import com.google.devtools.build.lib.analysis.platform.ConstraintSettingInfo;
2225
import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
2326
import com.google.devtools.build.lib.packages.BuildType;
2427
import com.google.devtools.build.lib.packages.RuleClass;
28+
import com.google.devtools.build.lib.packages.RuleClass.ToolchainResolutionMode;
29+
import com.google.devtools.build.lib.packages.Type;
2530
import com.google.devtools.build.lib.util.FileTypeSet;
2631

2732
/** Rule definition for {@link ConstraintValue}. */
@@ -33,6 +38,20 @@ public class ConstraintValueRule implements RuleDefinition {
3338
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
3439
return builder
3540
.advertiseStarlarkProvider(ConstraintValueInfo.PROVIDER.id())
41+
.cfg(NoConfigTransition.createFactory())
42+
.exemptFromConstraintChecking("this rule helps *define* a constraint")
43+
.useToolchainResolution(ToolchainResolutionMode.DISABLED)
44+
.override(
45+
attr("applicable_licenses", BuildType.LABEL_LIST)
46+
// This is a constant which is never linked into a target
47+
.value(ImmutableList.of())
48+
.allowedFileTypes()
49+
.nonconfigurable("fundamental constant, used in platform configuration"))
50+
.override(
51+
attr("tags", Type.STRING_LIST)
52+
// No need to show up in ":all", etc. target patterns.
53+
.value(ImmutableList.of("manual"))
54+
.nonconfigurable("low-level attribute, used in platform configuration"))
3655
/* <!-- #BLAZE_RULE(constraint_value).ATTRIBUTE(constraint_setting) -->
3756
The <code>constraint_setting</code> for which this <code>constraint_value</code> is a
3857
possible choice.
@@ -51,7 +70,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
5170
public Metadata getMetadata() {
5271
return Metadata.builder()
5372
.name(RULE_NAME)
54-
.ancestors(PlatformBaseRule.class)
73+
.ancestors(BaseRuleClasses.NativeBuildRule.class)
5574
.factoryClass(ConstraintValue.class)
5675
.build();
5776
}

src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,9 @@ public ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments()
671671

672672
@Override
673673
public BuildOptions patch(BuildOptionsView options, EventHandler eventHandler) {
674+
if (options.underlying().hasNoConfig()) {
675+
return options.underlying();
676+
}
674677
BuildOptionsView cloned = options.clone();
675678
cloned.get(DiffResetOptions.class).probablyIrrelevantOption = "(cleared)";
676679
cloned.get(DiffResetOptions.class).alsoIrrelevantOption = "(cleared)";

src/test/java/com/google/devtools/build/lib/runtime/commands/ConfigCommandTest.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,9 @@ private static String getOptionValue(
168168
}
169169

170170
private static boolean isTargetConfig(ConfigurationForOutput config) {
171+
if (config.mnemonic.endsWith("-noconfig")) {
172+
return false;
173+
}
171174
return !Boolean.parseBoolean(getOptionValue(config, "CoreOptions", "is exec configuration"));
172175
}
173176

@@ -189,7 +192,7 @@ public void showConfigIds() throws Exception {
189192
// Should be: target configuration, target configuration without test.
190193
assertThat(fullJson).isNotNull();
191194
assertThat(fullJson.has("configuration-IDs")).isTrue();
192-
assertThat(fullJson.get("configuration-IDs").getAsJsonArray().size()).isEqualTo(2);
195+
assertThat(fullJson.get("configuration-IDs").getAsJsonArray().size()).isEqualTo(3);
193196
}
194197

195198
@Test
@@ -279,7 +282,7 @@ public void showAllConfigs() throws Exception {
279282
assertThat(config).isNotNull();
280283
numConfigs++;
281284
}
282-
assertThat(numConfigs).isEqualTo(2); // Target + target w/o test.
285+
assertThat(numConfigs).isEqualTo(3); // Target + target w/o test + nonConfig.
283286
}
284287

285288
@Test

0 commit comments

Comments
 (0)