[6.1.0] Add --host_features (bazelbuild#17528)

* 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 <[email protected]>
fmeum · Feb 19, 2023 · 5932b3b · 5932b3b
1 parent ec36595
commit 5932b3b
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 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
@@ -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,
@@ -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
@@ -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");