Make --[no]distinct_host_configuration a no-op.

It has already been mostly a no-op due to the recent work to migrate to the exec transition from the host transition; this change only makes that official, but does not result in any user-visible changes. RELNOTES: None. PiperOrigin-RevId: 435351987
bazelbuild · Mar 17, 2022 · 78d0fc9 · 78d0fc9 · jak1110p0 · Mar 12, 2023
1 parent 1270f74
commit 78d0fc9
Show file tree

Hide file tree

Showing 8 changed files with 13 additions and 94 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
@@ -467,29 +467,6 @@ public ExecConfigurationDistinguisherSchemeConverter() {
               + " '//package:target --options'.")
   public RunUnder runUnder;
 
-  @Option(
-      name = "distinct_host_configuration",
-      defaultValue = "true",
-      documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
-      effectTags = {
-        OptionEffectTag.LOSES_INCREMENTAL_STATE,
-        OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION,
-        OptionEffectTag.LOADING_AND_ANALYSIS,
-      },
-      help =
-          "Build all the tools used during the build for a distinct configuration from that used "
-              + "for the target program. When this is disabled, the same configuration is used for "
-              + "host and target programs. This may cause undesirable rebuilds of tools such as "
-              + "the protocol compiler (and then everything downstream) whenever a minor change "
-              + "is made to the target configuration, such as setting the linker options. When "
-              + "this is enabled (the default), a distinct configuration will be used to build the "
-              + "tools, preventing undesired rebuilds. However, certain libraries will then need "
-              + "to be compiled twice, once for each configuration, which may cause some builds "
-              + "to be slower. As a rule of thumb, this option is likely to benefit users that "
-              + "make frequent changes in configuration (e.g. opt/dbg).  "
-              + "Please read the user manual for the full explanation.")
-  public boolean useDistinctHostConfiguration;
-
   @Option(
       name = "check_visibility",
       defaultValue = "true",

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java
@@ -86,15 +86,10 @@ public BuildOptions patch(BuildOptionsView originalOptions, EventHandler eventHa
         // nothing to do, already trimmed this fragment
         return originalOptions.underlying();
       }
-      CoreOptions originalCoreOptions = originalOptions.get(CoreOptions.class);
       TestOptions originalTestOptions = originalOptions.get(TestOptions.class);
       if (!originalTestOptions.trimTestConfiguration
-          || (originalTestOptions.experimentalRetainTestConfigurationAcrossTestonly && testonly)
-          || !originalCoreOptions.useDistinctHostConfiguration) {
+          || (originalTestOptions.experimentalRetainTestConfigurationAcrossTestonly && testonly)) {
         // nothing to do, trimming is disabled
-        // Due to repercussions of b/117932061, do not trim when `--nodistinct_host_configuration`
-        // TODO(twigg): See if can remove distinct_host_configuration read here and thus
-        // dependency on CoreOptions above.
         return originalOptions.underlying();
       }
       // No context needed, use the constant Boolean.TRUE.

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
@@ -516,6 +516,15 @@ public static final class AllCommandGraveyardOptions extends OptionsBase {
         effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
         help = "No-op")
     public boolean besBestEffort;
+
+    @Option(
+        name = "distinct_host_configuration",
+        defaultValue = "false",
+        documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+        effectTags = {OptionEffectTag.NO_OP},
+        metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+        help = "No-op.")
+    public boolean useDistinctHostConfiguration;
   }
 
   @Override

diff --git a/...gle/devtools/build/lib/rules/config/ConfigFeatureFlagTaggedTrimmingTransitionFactory.java b/...gle/devtools/build/lib/rules/config/ConfigFeatureFlagTaggedTrimmingTransitionFactory.java
@@ -61,8 +61,7 @@ public ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments()
     public BuildOptions patch(BuildOptionsView options, EventHandler eventHandler) {
       if (!(options.contains(ConfigFeatureFlagOptions.class)
           && options.get(ConfigFeatureFlagOptions.class)
-              .enforceTransitiveConfigsForConfigFeatureFlag
-          && options.get(CoreOptions.class).useDistinctHostConfiguration)) {
+              .enforceTransitiveConfigsForConfigFeatureFlag)) {
         return options.underlying();
       }
       return FeatureFlagValue.trimFlagValues(options.underlying(), flags);

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareAnalysisPhaseFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareAnalysisPhaseFunction.java
@@ -75,10 +75,7 @@ public PrepareAnalysisPhaseValue compute(SkyKey key, Environment env)
     BuildOptionsView hostTransitionOptionsView =
         new BuildOptionsView(targetOptions, HostTransition.INSTANCE.requiresOptionFragments());
     BuildOptions hostOptions =
-        targetOptions.get(CoreOptions.class).useDistinctHostConfiguration
-            ? HostTransition.INSTANCE.patch(hostTransitionOptionsView, env.getListener())
-            : targetOptions;
-
+        HostTransition.INSTANCE.patch(hostTransitionOptionsView, env.getListener());
     PathFragment platformMappingPath = targetOptions.get(PlatformOptions.class).platformMappings;
     PlatformMappingValue platformMappingValue =
         (PlatformMappingValue) env.getValue(PlatformMappingValue.Key.create(platformMappingPath));

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -1474,14 +1474,11 @@ public BuildConfigurationCollection createConfigurations(
 
     BuildConfigurationValue firstTargetConfig = topLevelTargetConfigs.get(0);
 
-    BuildOptions targetOptions = firstTargetConfig.getOptions();
     BuildOptionsView hostTransitionOptionsView =
         new BuildOptionsView(
             firstTargetConfig.getOptions(), HostTransition.INSTANCE.requiresOptionFragments());
     BuildOptions hostOptions =
-        targetOptions.get(CoreOptions.class).useDistinctHostConfiguration
-            ? HostTransition.INSTANCE.patch(hostTransitionOptionsView, eventHandler)
-            : targetOptions;
+        HostTransition.INSTANCE.patch(hostTransitionOptionsView, eventHandler);
     BuildConfigurationValue hostConfig = getConfiguration(eventHandler, hostOptions, keepGoing);
 
     // TODO(gregce): cache invalid option errors in BuildConfigurationFunction, then use a dedicated

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/constraints/ConstraintsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/constraints/ConstraintsTest.java
@@ -750,25 +750,6 @@ public void hostDependenciesAreNotChecked_customRule() throws Exception {
     assertNoEvents();
   }
 
-  @Test
-  public void hostDependenciesNotCheckedNoDistinctHostConfiguration() throws Exception {
-    useConfiguration("--nodistinct_host_configuration");
-    new EnvironmentGroupMaker("buildenv/foo").setEnvironments("a", "b").setDefaults("a").make();
-    scratch.file("hello/BUILD",
-        "sh_binary(name = 'host_tool',",
-        "    srcs = ['host_tool.sh'],",
-        "    restricted_to = ['//buildenv/foo:b'])",
-        "genrule(",
-        "    name = 'hello',",
-        "    srcs = [],",
-        "    outs = ['hello.out'],",
-        "    cmd = '',",
-        "    tools = [':host_tool'],",
-        "    compatible_with = ['//buildenv/foo:a'])");
-    assertThat(getConfiguredTarget("//hello:hello")).isNotNull();
-    assertNoEvents();
-  }
-
   @Test
   public void execDependenciesAreNotChecked() throws Exception {
     new EnvironmentGroupMaker("buildenv/foo").setEnvironments("a", "b").setDefaults("a").make();

diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagManualTrimmingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagManualTrimmingTest.java
@@ -697,42 +697,6 @@ public void featureFlagInHostConfiguration_HasNoTransitiveConfigEnforcement() th
     assertNoEvents();
   }
 
-  @Test
-  public void noDistinctHostConfiguration_DisablesEnforcementForBothHostAndTargetConfigs()
-      throws Exception {
-    scratch.file(
-        "test/BUILD",
-        "load(':host_transition.bzl', 'host_transition')",
-        "load(':read_flags.bzl', 'read_flags')",
-        "feature_flag_setter(",
-        "    name = 'target',",
-        "    deps = [':host', ':reader'],",
-        "    flag_values = {",
-        "        ':used_flag': 'configured',",
-        "    },",
-        // no transitive_configs
-        ")",
-        "host_transition(",
-        "    name = 'host',",
-        "    srcs = [':reader'],",
-        // no transitive_configs
-        ")",
-        "read_flags(",
-        "    name = 'reader',",
-        "    flags = [':used_flag'],",
-        // no transitive_configs
-        ")",
-        "config_feature_flag(",
-        "    name = 'used_flag',",
-        "    allowed_values = ['default', 'configured', 'other'],",
-        "    default_value = 'default',",
-        ")");
-
-    enableManualTrimmingAnd("--nodistinct_host_configuration");
-    getConfiguredTarget("//test:target");
-    assertNoEvents();
-  }
-
   @Test
   public void featureFlagAccessedDirectly_ReturnsDefaultValue() throws Exception {
     scratch.file(