From 2574818b86c239d1eb844ef2d4bc41ca588b38b8 Mon Sep 17 00:00:00 2001
From: Keith Smiley <keithbsmiley@gmail.com>
Date: Tue, 1 Nov 2022 15:36:44 -0700
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.

Fixes https://github.com/bazelbuild/bazel/issues/13839
---
 .../lib/analysis/config/CoreOptions.java      | 23 +++++++++++++++----
 .../analysis/RuleConfiguredTargetTest.java    | 21 +++++++++++++++++
 2 files changed, 39 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 5a9090644f9352..ccff48b916850c 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
@@ -643,13 +643,26 @@ 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 = "target_environment",
       converter = LabelListConverter.class,
@@ -986,7 +999,7 @@ public FragmentOptions getExec() {
     exec.checkLicenses = checkLicenses;
 
     // === Pass on C++ compiler features.
-    exec.defaultFeatures = ImmutableList.copyOf(defaultFeatures);
+    exec.defaultFeatures = ImmutableList.copyOf(hostFeatures);
 
     // Save host options in case of a further exec->host transition.
     exec.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..6f8b1d5665beee 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,27 @@ public void testFeatureEnabledOnCommandLine() throws Exception {
     assertThat(features).doesNotContain("other");
   }
 
+  @Test
+  public void testTargetIgnoresHostFeatures() throws Exception {
+    useConfiguration("--features=feature", "--host_features=other");
+    scratch.file("a/BUILD",
+        "cc_library(name = 'a')");
+    ImmutableSet<String> features = getRuleContext(configure("//a")).getFeatures();
+    assertThat(features).contains("feature");
+    assertThat(features).doesNotContain("other");
+  }
+
+  @Test
+  public void testHostFeatures() 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("host_feature");
+    assertThat(features).doesNotContain("feature");
+  }
+
   @Test
   public void testFeatureDisabledOnCommandLine() throws Exception {
     useConfiguration("--features=-feature");