Skip to content

Commit 0ef9c7c

Browse files
keithcopybara-github
authored andcommitted
Add --host_features
Previously there was no way to have a feature only apply to the entire transitive target closure without `--features` which also applied to the host / exec configuration. This is undesirable for many features such as C++ sanitizers where you often only need them to affect targets in the target configuration, and you don't want to invalidate all host tools when switching between these build configurations. RELNOTES[INC]: `--features` only applies to targets built in the target configuration, and `--host_features` is used for the host / exec configuration (gated behind `--incompatible_use_host_features`) Fixes bazelbuild#13839 Closes bazelbuild#16626. PiperOrigin-RevId: 509809427 Change-Id: I3fadb1e28267c096a25d8841648175306ee73f2e
1 parent b31da0c commit 0ef9c7c

File tree

2 files changed

+66
-5
lines changed

2 files changed

+66
-5
lines changed

src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java

+34-5
Original file line numberDiff line numberDiff line change
@@ -646,13 +646,37 @@ public OutputDirectoryNamingSchemeConverter() {
646646
documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
647647
effectTags = {OptionEffectTag.CHANGES_INPUTS, OptionEffectTag.AFFECTS_OUTPUTS},
648648
help =
649-
"The given features will be enabled or disabled by default for all packages. "
650-
+ "Specifying -<feature> will disable the feature globally. "
649+
"The given features will be enabled or disabled by default for targets "
650+
+ "built in the target configuration. "
651+
+ "Specifying -<feature> will disable the feature. "
651652
+ "Negative features always override positive ones. "
652-
+ "This flag is used to enable rolling out default feature changes without a "
653-
+ "Bazel release.")
653+
+ "See also --host_features")
654654
public List<String> defaultFeatures;
655655

656+
@Option(
657+
name = "host_features",
658+
allowMultiple = true,
659+
defaultValue = "null",
660+
documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
661+
effectTags = {OptionEffectTag.CHANGES_INPUTS, OptionEffectTag.AFFECTS_OUTPUTS},
662+
help =
663+
"The given features will be enabled or disabled by default for targets "
664+
+ "built in the exec configuration. "
665+
+ "Specifying -<feature> will disable the feature. "
666+
+ "Negative features always override positive ones.")
667+
public List<String> hostFeatures;
668+
669+
@Option(
670+
name = "incompatible_use_host_features",
671+
defaultValue = "false",
672+
documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
673+
effectTags = {OptionEffectTag.CHANGES_INPUTS, OptionEffectTag.AFFECTS_OUTPUTS},
674+
metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
675+
help =
676+
"If true, use --features only for the target configuration and --host_features for the"
677+
+ " exec configuration.")
678+
public boolean incompatibleUseHostFeatures;
679+
656680
@Option(
657681
name = "target_environment",
658682
converter = LabelListConverter.class,
@@ -989,7 +1013,12 @@ public FragmentOptions getExec() {
9891013
exec.checkLicenses = checkLicenses;
9901014

9911015
// === Pass on C++ compiler features.
992-
exec.defaultFeatures = ImmutableList.copyOf(defaultFeatures);
1016+
exec.incompatibleUseHostFeatures = incompatibleUseHostFeatures;
1017+
if (incompatibleUseHostFeatures) {
1018+
exec.defaultFeatures = ImmutableList.copyOf(hostFeatures);
1019+
} else {
1020+
exec.defaultFeatures = ImmutableList.copyOf(defaultFeatures);
1021+
}
9931022

9941023
// Save host options in case of a further exec->host transition.
9951024
exec.hostCpu = hostCpu;

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

+32
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,38 @@ public void testFeatureEnabledOnCommandLine() throws Exception {
5151
assertThat(features).doesNotContain("other");
5252
}
5353

54+
@Test
55+
public void testTargetIgnoresHostFeatures() throws Exception {
56+
useConfiguration("--features=feature", "--host_features=host_feature");
57+
scratch.file("a/BUILD", "cc_library(name = 'a')");
58+
ImmutableSet<String> features = getRuleContext(configure("//a")).getFeatures();
59+
assertThat(features).contains("feature");
60+
assertThat(features).doesNotContain("host_feature");
61+
}
62+
63+
@Test
64+
public void testHostFeatures() throws Exception {
65+
useConfiguration(
66+
"--features=feature",
67+
"--host_features=host_feature",
68+
"--incompatible_use_host_features=true");
69+
scratch.file("a/BUILD", "cc_library(name = 'a')");
70+
ImmutableSet<String> features =
71+
getRuleContext(getConfiguredTarget("//a", getExecConfiguration())).getFeatures();
72+
assertThat(features).contains("host_feature");
73+
assertThat(features).doesNotContain("feature");
74+
}
75+
76+
@Test
77+
public void testHostFeaturesIncompatibleDisabled() throws Exception {
78+
useConfiguration("--features=feature", "--host_features=host_feature");
79+
scratch.file("a/BUILD", "cc_library(name = 'a')");
80+
ImmutableSet<String> features =
81+
getRuleContext(getConfiguredTarget("//a", getExecConfiguration())).getFeatures();
82+
assertThat(features).contains("feature");
83+
assertThat(features).doesNotContain("host_feature");
84+
}
85+
5486
@Test
5587
public void testFeatureDisabledOnCommandLine() throws Exception {
5688
useConfiguration("--features=-feature");

0 commit comments

Comments
 (0)