Skip to content

Commit

Permalink
Add --host_features
Browse files Browse the repository at this point in the history
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 #13839
  • Loading branch information
keith committed Feb 13, 2023
1 parent 7227fd4 commit 3163cb8
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -646,13 +646,35 @@ public OutputDirectoryNamingSchemeConverter() {
documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
effectTags = {OptionEffectTag.CHANGES_INPUTS, OptionEffectTag.AFFECTS_OUTPUTS},
help =
"The given features will be enabled or disabled by default for all packages. "
+ "Specifying -<feature> will disable the feature globally. "
"The given features will be enabled or disabled by default for targets "
+ "built in the target configuration. "
+ "Specifying -<feature> will disable the feature. "
+ "Negative features always override positive ones. "
+ "This flag is used to enable rolling out default feature changes without a "
+ "Bazel release.")
+ "See also --host_features")
public List<String> defaultFeatures;

@Option(
name = "host_features",
allowMultiple = true,
defaultValue = "null",
documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
effectTags = {OptionEffectTag.CHANGES_INPUTS, OptionEffectTag.AFFECTS_OUTPUTS},
help =
"The given features will be enabled or disabled by default for targets "
+ "built in the exec configuration. "
+ "Specifying -<feature> will disable the feature. "
+ "Negative features always override positive ones.")
public List<String> hostFeatures;

@Option(
name = "incompatible_use_host_features",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
effectTags = {OptionEffectTag.CHANGES_INPUTS, OptionEffectTag.AFFECTS_OUTPUTS},
metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
help = "If true, use --features only for the target configuration and --host_features for the exec configuration.")
public boolean incompatibleUseHostFeatures;

@Option(
name = "target_environment",
converter = LabelListConverter.class,
Expand Down Expand Up @@ -989,7 +1011,12 @@ public FragmentOptions getExec() {
exec.checkLicenses = checkLicenses;

// === Pass on C++ compiler features.
exec.defaultFeatures = ImmutableList.copyOf(defaultFeatures);
exec.incompatibleUseHostFeatures = incompatibleUseHostFeatures;
if (incompatibleUseHostFeatures) {
exec.defaultFeatures = ImmutableList.copyOf(hostFeatures);
} else {
exec.defaultFeatures = ImmutableList.copyOf(defaultFeatures);
}

// Save host options in case of a further exec->host transition.
exec.hostCpu = hostCpu;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,36 @@ public void testFeatureEnabledOnCommandLine() throws Exception {
assertThat(features).doesNotContain("other");
}

@Test
public void testTargetIgnoresHostFeatures() throws Exception {
useConfiguration("--features=feature", "--host_features=host_feature");
scratch.file("a/BUILD", "cc_library(name = 'a')");
ImmutableSet<String> features = getRuleContext(configure("//a")).getFeatures();
assertThat(features).contains("feature");
assertThat(features).doesNotContain("host_feature");
}

@Test
public void testHostFeatures() throws Exception {
useConfiguration("--features=feature", "--host_features=host_feature",
"--incompatible_use_host_features=true");
scratch.file("a/BUILD", "cc_library(name = 'a')");
ImmutableSet<String> features = getRuleContext(
getConfiguredTarget("//a", getExecConfiguration())).getFeatures();
assertThat(features).contains("host_feature");
assertThat(features).doesNotContain("feature");
}

@Test
public void testHostFeaturesIncompatibleDisabled() throws Exception {
useConfiguration("--features=feature", "--host_features=host_feature");
scratch.file("a/BUILD", "cc_library(name = 'a')");
ImmutableSet<String> features = getRuleContext(
getConfiguredTarget("//a", getExecConfiguration())).getFeatures();
assertThat(features).contains("feature");
assertThat(features).doesNotContain("host_feature");
}

@Test
public void testFeatureDisabledOnCommandLine() throws Exception {
useConfiguration("--features=-feature");
Expand Down

0 comments on commit 3163cb8

Please sign in to comment.