From 7a29480f5491036f1e48e54873f0eb47875e984f Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Wed, 15 Feb 2023 05:58:41 -0800 Subject: [PATCH] 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 https://github.com/bazelbuild/bazel/issues/13839 Closes #16626. PiperOrigin-RevId: 509809427 Change-Id: I3fadb1e28267c096a25d8841648175306ee73f2e (cherry picked from commit 0ef9c7c2ad097b555f0dc25a5e65d5440a20d7df) --- .../lib/analysis/config/CoreOptions.java | 39 ++++++++++++++++--- .../analysis/RuleConfiguredTargetTest.java | 32 +++++++++++++++ 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java index 6df4ac7b7c66a7..3a75df94bfc999 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java @@ -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 - will disable the feature globally. " + "The given features will be enabled or disabled by default for targets " + + "built in the target configuration. " + + "Specifying - 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 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 - will disable the feature. " + + "Negative features always override positive ones.") + public List 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, @@ -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; diff --git a/src/test/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetTest.java index f379378004effc..cb7c937564555b 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetTest.java @@ -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 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 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 features = + getRuleContext(getConfiguredTarget("//a", getExecConfiguration())).getFeatures(); + assertThat(features).contains("feature"); + assertThat(features).doesNotContain("host_feature"); + } + @Test public void testFeatureDisabledOnCommandLine() throws Exception { useConfiguration("--features=-feature");