Skip to content

Commit

Permalink
[6.1.0] Add --host_features (bazelbuild#17528)
Browse files Browse the repository at this point in the history
* 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
(cherry picked from commit 0ef9c7c)

* Fix tests

---------

Co-authored-by: kshyanashree <109167932+kshyanashree@users.noreply.github.com>
  • Loading branch information
keith and ShreeM01 authored Feb 19, 2023
1 parent ec36595 commit 5932b3b
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -629,13 +629,37 @@ 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 @@ -960,7 +984,12 @@ public FragmentOptions getHost() {
host.checkLicenses = checkLicenses;

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

// Save host options in case of a further exec->host transition.
host.hostCpu = hostCpu;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,38 @@ 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", getHostConfiguration())).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", getHostConfiguration())).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 5932b3b

Please sign in to comment.