Keep exec-configured --run_under targets behind an incompatible flag.

bazelbuild@b03e4c5  is an incompatible change. Repo testing showed failures changing this
unconditionally. So adding `--incompatible_bazel_test_exec_run_under` to gate it.

Even though `$ bazel test --run_under=//foo` invokes `//foo` on exec machines, `//foo`'s build may still expect target configurations. For example:

 - custom toolchain settings
 - target-focused `select()`s
 - user-defined flags that don't propagate to the exec config

See bazelbuild#23179.

PiperOrigin-RevId: 658472773
Change-Id: I1bdf30432a6e3c74fc0355f1a96fee44ba2d453a

src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java

@@ -32,7 +32,6 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
 import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
-import com.google.devtools.build.lib.analysis.config.RunUnder;
 import com.google.devtools.build.lib.analysis.constraints.ConstraintConstants;
 import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
 import com.google.devtools.build.lib.analysis.test.CoverageConfiguration;
@@ -162,15 +161,48 @@ public static LabelLateBoundDefault<CoverageConfiguration> getCoverageOutputGene
       (rule, attributes, configuration) -> configuration.outputGenerator();
 
   // TODO(b/65746853): provide a way to do this without passing the entire configuration
-  /** Implementation for the :run_under attribute. */
+  /**
+   * Resolves the latebound exec-configured :run_under label. This only exists if --run_under is set
+   * to a label and --incompatible_bazel_test_exec_run_under is true. Else it's null.
+   *
+   * <p>{@link RUN_UNDER_EXEC_CONFIG} and {@link RUN_UNDER_TARGET_CONFIG} cannot both be non-null in
+   * a build: if {@code --run_under} is set it must connect to a single dependency.
+   */
   @SerializationConstant @VisibleForSerialization
-  public static final LabelLateBoundDefault<?> RUN_UNDER =
+  public static final LabelLateBoundDefault<?> RUN_UNDER_EXEC_CONFIG =
+      LabelLateBoundDefault.fromTargetConfiguration(
+          BuildConfigurationValue.class,
+          null,
+          (rule, attributes, config) -> {
+            if (config.isExecConfiguration()
+                // This is the opposite of RUN_UNDER_TARGET_CONFIG, so both can't be non-null.
+                || !config.runUnderExecConfigForTests()
+                || config.getRunUnder() == null) {
+              return null;
+            }
+            return config.getRunUnder().getLabel();
+          });
+
+  // TODO(b/65746853): provide a way to do this without passing the entire configuration
+  /**
+   * Resolves the latebound target-configured :run_under label. This only exists if --run_under is
+   * set to a label and --incompatible_bazel_test_exec_run_under is false. Else it's null.
+   *
+   * <p>{@link RUN_UNDER_EXEC_CONFIG} and {@link RUN_UNDER_TARGET_CONFIG} cannot both be non-null in
+   * a build: if {@code --run_under} is set it must connect to a single dependency.
+   */
+  public static final LabelLateBoundDefault<?> RUN_UNDER_TARGET_CONFIG =
       LabelLateBoundDefault.fromTargetConfiguration(
           BuildConfigurationValue.class,
           null,
-          (rule, attributes, configuration) -> {
-            RunUnder runUnder = configuration.getRunUnder();
-            return runUnder != null ? runUnder.getLabel() : null;
+          (rule, attributes, config) -> {
+            if (config.isExecConfiguration()
+                // This is the opposite of RUN_UNDER_EXEC_CONFIG, so both can't be non-null.
+                || config.runUnderExecConfigForTests()
+                || config.getRunUnder() == null) {
+              return null;
+            }
+            return config.getRunUnder().getLabel();
           });
 
   /**
@@ -255,22 +287,28 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
                   .value(
                       coverageReportGeneratorAttribute(
                           env.getToolsLabel(DEFAULT_COVERAGE_REPORT_GENERATOR_VALUE))))
-          // --run_under targets always run on exec machines:
+          // Bazel runs --run_under targets on exec machines:
           //   * For "$ bazel run", they run directly on the machine running bazel.
           //   * For "$ bazel test", they run on the build machine that executes tests.
           //
-          // This may be different than the target platform. For example, if testing embedded device
-          // code where the test runner's job is to upload the target binary to the embedded device.
+          // This means they should be configured for the exec configuration.
+          // --incompatible_bazel_test_exec_run_under supports this. But we still need to support
+          // legacy invocations that incorrectly configure it with the target configuration. To
+          // support both modes, we define both an exec-configured and target-configured --run_under
+          // dep and have consuming logic choose the right one based on the flag.
           //
-          // TODO: https://github.com/bazelbuild/bazel/discussions/21805 Setting cfg() here makes
-          // this work for "$ bazel test" but not "$ bazel run". Make this work for "$ bazel run"
-          // by updating RunCommand.java to self-transition --run_under to the exec configuration.
+          // TODO: https://github.com/bazelbuild/bazel/discussions/21805 this works for
+          // "$ bazel test" but not "$ bazel run". Make this work for "$ bazel run" by updating
+          // RunCommand.java to self-transition --run_under to the exec configuration.
           .add(
-              attr(":run_under", LABEL)
+              attr(":run_under_exec_config", LABEL)
                   .cfg(ExecutionTransitionFactory.createFactory())
-                  .value(RUN_UNDER)
+                  .value(RUN_UNDER_EXEC_CONFIG)
+                  .skipPrereqValidatorCheck())
+          .add(
+              attr(":run_under_target_config", LABEL)
+                  .value(RUN_UNDER_TARGET_CONFIG)
                   .skipPrereqValidatorCheck());
-
       env.getNetworkAllowlistForTests()
           .ifPresent(
               label ->

src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java

@@ -881,6 +881,18 @@ public <T> T getPrerequisite(String attributeName, StarlarkProviderWrapper<T> ke
     return getOwningPrerequisitesCollection(attributeName).getPrerequisite(attributeName, key);
   }
 
+  /**
+   * Returns the {@code --run_under} prerequisite based on the value of {@code
+   * --incompatible_bazel_test_exec_run_under}.
+   */
+  @Nullable
+  public TransitiveInfoCollection getRunUnderPrerequisite() {
+    return getPrerequisite(
+        getConfiguration().runUnderExecConfigForTests()
+            ? ":run_under_exec_config"
+            : ":run_under_target_config");
+  }
+
   /**
    * Returns the list of transitive info collections that feed into this target through the
    * specified attribute.

src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java

@@ -231,7 +231,7 @@ private static RunfilesSupport create(
     if (runUnder != null
         && runUnder.getLabel() != null
         && TargetUtils.isTestRule(ruleContext.getRule())) {
-      TransitiveInfoCollection runUnderTarget = ruleContext.getPrerequisite(":run_under");
+      TransitiveInfoCollection runUnderTarget = ruleContext.getRunUnderPrerequisite();
       runfiles =
           new Runfiles.Builder(
                   ruleContext.getWorkspaceName(),

src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationValue.java

@@ -718,10 +718,16 @@ public boolean isCodeCoverageEnabled() {
     return options.collectCodeCoverage;
   }
 
+  @Nullable
   public RunUnder getRunUnder() {
     return options.runUnder;
   }
 
+  /** Should the {@code --run_under} be configured in the exec configuration? */
+  public boolean runUnderExecConfigForTests() {
+    return options.bazelTestExecRunUnder;
+  }
+
   /** Returns true if this is an execution configuration. */
   public boolean isExecConfiguration() {
     return options.isExec;

src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java

@@ -848,6 +848,22 @@ public OutputPathsConverter() {
               + " When disabled, only the last flag is taken into account.")
   public boolean additiveModifyExecutionInfo;
 
+  @Option(
+      name = "incompatible_bazel_test_exec_run_under",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
+      effectTags = {
+        OptionEffectTag.AFFECTS_OUTPUTS,
+      },
+      metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+      help =
+          "If enabled, \"bazel test --run_under=//:runner\" builds \"//:runner\" in the exec"
+              + " configuration. If disabled, it builds \"//:runner\" in the target configuration."
+              + " Bazel executes tests on exec machines, so the former is more correct. This"
+              + " doesn't affect \"bazel run\", which always builds \"`--run_under=//foo\" in the"
+              + " target configuration.")
+  public boolean bazelTestExecRunUnder;
+
   @Option(
       name = "include_config_fragments_provider",
       defaultValue = "off",

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

@@ -17,7 +17,8 @@
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.ImmutableList.toImmutableList;
-import static com.google.devtools.build.lib.analysis.BaseRuleClasses.RUN_UNDER;
+import static com.google.devtools.build.lib.analysis.BaseRuleClasses.RUN_UNDER_EXEC_CONFIG;
+import static com.google.devtools.build.lib.analysis.BaseRuleClasses.RUN_UNDER_TARGET_CONFIG;
 import static com.google.devtools.build.lib.analysis.BaseRuleClasses.TIMEOUT_DEFAULT;
 import static com.google.devtools.build.lib.analysis.BaseRuleClasses.getTestRuntimeLabelList;
 import static com.google.devtools.build.lib.analysis.test.ExecutionInfo.DEFAULT_TEST_RUNNER_EXEC_GROUP;
@@ -275,10 +276,16 @@ public static RuleClass getTestBaseRule(RuleDefinitionEnvironment env) {
                             labelCache.get(
                                 toolsRepository
                                     + BaseRuleClasses.DEFAULT_COVERAGE_REPORT_GENERATOR_VALUE))))
+            // See similar definitions in BaseRuleClasses for context.
             .add(
-                attr(":run_under", LABEL)
+                attr(":run_under_exec_config", LABEL)
                     .cfg(ExecutionTransitionFactory.createFactory())
-                    .value(RUN_UNDER))
+                    .value(RUN_UNDER_EXEC_CONFIG)
+                    .skipPrereqValidatorCheck())
+            .add(
+                attr(":run_under_target_config", LABEL)
+                    .value(RUN_UNDER_TARGET_CONFIG)
+                    .skipPrereqValidatorCheck())
             .addAttribute(
                 attr(Rule.IS_EXECUTABLE_ATTRIBUTE_NAME, BOOLEAN)
                     .value(true)

src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetExecutionSettings.java

@@ -83,7 +83,7 @@ public final class TestTargetExecutionSettings {
 
   @Nullable
   private static Artifact getRunUnderExecutable(RuleContext ruleContext) {
-    TransitiveInfoCollection runUnderTarget = ruleContext.getPrerequisite(":run_under");
+    TransitiveInfoCollection runUnderTarget = ruleContext.getRunUnderPrerequisite();
     return runUnderTarget == null
         ? null
         : runUnderTarget.getProvider(FilesToRunProvider.class).getExecutable();

src/main/starlark/builtins_bzl/common/builtin_exec_platforms.bzl

@@ -231,6 +231,7 @@ bazel_fragments["CoreOptions"] = fragment(
         "//command_line_option:experimental_debug_selects_always_succeed",
         "//command_line_option:incompatible_check_testonly_for_output_files",
         "//command_line_option:incompatible_auto_exec_groups",
+        "//command_line_option:incompatible_bazel_test_exec_run_under",
         "//command_line_option:experimental_writable_outputs",
         "//command_line_option:build_runfile_manifests",
         "//command_line_option:build_runfile_links",