Deprecate the experimental_action_resource_set flag. (#24939)

PiperOrigin-RevId: 715819891
Change-Id: I1c2e582a3d4c6e87e5c76837abb7a175eed3e850 
(cherry picked from commit c570f97)

Fixes #24933

Co-authored-by: Googler <[email protected]>

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

@@ -866,8 +866,7 @@ && getSemantics()
       builder.setShadowedAction(Optional.of((Action) shadowedActionUnchecked));
     }
 
-    if (getSemantics().getBool(BuildLanguageOptions.EXPERIMENTAL_ACTION_RESOURCE_SET)
-        && resourceSetUnchecked != Starlark.NONE) {
+    if (resourceSetUnchecked != Starlark.NONE) {
       validateResourceSetBuilder(resourceSetUnchecked);
       builder.setResources(
           new StarlarkActionResourceSetBuilder(

src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java

@@ -480,6 +480,14 @@ public static final class AllCommandGraveyardOptions extends OptionsBase {
         effectTags = {OptionEffectTag.UNKNOWN},
         help = "Do not use.")
     public String javaOptimizationMode;
+
+    @Option(
+        name = "experimental_action_resource_set",
+        defaultValue = "true",
+        documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+        effectTags = {OptionEffectTag.NO_OP},
+        help = "No-op.")
+    public boolean experimentalActionResourceSet;
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java

@@ -439,20 +439,6 @@ public final class BuildLanguageOptions extends OptionsBase {
               + " https://github.com/bazelbuild/bazel/issues/8830 for details.")
   public boolean experimentalAllowTagsPropagation;
 
-  @Option(
-      name = "experimental_action_resource_set",
-      defaultValue = "true",
-      documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
-      effectTags = {OptionEffectTag.EXECUTION, OptionEffectTag.BUILD_FILE_SEMANTICS},
-      metadataTags = {
-        OptionMetadataTag.EXPERIMENTAL,
-      },
-      help =
-          "If set to true, ctx.actions.run() and ctx.actions.run_shell() accept a resource_set"
-              + " parameter for local execution. Otherwise it will default to 250 MB for memory"
-              + " and 1 cpu.")
-  public boolean experimentalActionResourceSet;
-
   @Option(
       name = "incompatible_struct_has_no_methods",
       defaultValue = "false",
@@ -861,7 +847,6 @@ public StarlarkSemantics toStarlarkSemantics() {
             .setBool(
                 INCOMPATIBLE_EXISTING_RULES_IMMUTABLE_VIEW, incompatibleExistingRulesImmutableView)
             .setBool(INCOMPATIBLE_NO_IMPLICIT_WATCH_LABEL, incompatibleNoImplicitWatchLabel)
-            .setBool(EXPERIMENTAL_ACTION_RESOURCE_SET, experimentalActionResourceSet)
             .setBool(EXPERIMENTAL_GOOGLE_LEGACY_API, experimentalGoogleLegacyApi)
             .setBool(EXPERIMENTAL_PLATFORMS_API, experimentalPlatformsApi)
             .setBool(EXPERIMENTAL_CC_SHARED_LIBRARY, experimentalCcSharedLibrary)
@@ -982,7 +967,6 @@ public StarlarkSemantics toStarlarkSemantics() {
   public static final String EXPERIMENTAL_REPO_REMOTE_EXEC = "-experimental_repo_remote_exec";
   public static final String EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT =
       "-experimental_sibling_repository_layout";
-  public static final String EXPERIMENTAL_ACTION_RESOURCE_SET = "+experimental_action_resource_set";
   public static final String INCOMPATIBLE_ALWAYS_CHECK_DEPSET_ELEMENTS =
       "+incompatible_always_check_depset_elements";
   public static final String INCOMPATIBLE_DEPSET_FOR_LIBRARIES_TO_LINK_GETTER =

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkActionFactoryApi.java

@@ -514,10 +514,9 @@ void write(FileApi output, Object content, Boolean isExecutable)
                     + " action. The returned dictionary may contain the following entries, each of"
                     + " which may be a float or an int:<ul><li>\"cpu\": number of CPUs; default"
                     + " 1<li>\"memory\": in MB; default 250<li>\"local_test\": number of local"
-                    + " tests; default 1</ul><p>If this parameter is set to <code>None</code> or if"
-                    + " <code>--experimental_action_resource_set</code> is false, the default"
-                    + " values are used.<p>The callback must be top-level (lambda and nested"
-                    + " functions aren't allowed)."),
+                    + " tests; default 1</ul><p>If this parameter is set to <code>None</code> , the"
+                    + " default values are used.<p>The callback must be top-level (lambda and"
+                    + " nested functions aren't allowed)."),
         @Param(
             name = "toolchain",
             allowedTypes = {

src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java

@@ -137,7 +137,6 @@ private static BuildLanguageOptions buildRandomOptions(Random rand) throws Excep
         "--incompatible_allow_tags_propagation=" + rand.nextBoolean(), // flag, Java names differ
         "--experimental_cc_shared_library=" + rand.nextBoolean(),
         "--experimental_repo_remote_exec=" + rand.nextBoolean(),
-        "--experimental_action_resource_set=" + rand.nextBoolean(),
         "--incompatible_always_check_depset_elements=" + rand.nextBoolean(),
         "--incompatible_depset_for_libraries_to_link_getter=" + rand.nextBoolean(),
         "--incompatible_disable_target_provider_fields=" + rand.nextBoolean(),
@@ -187,7 +186,6 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
         .setBool(BuildLanguageOptions.INCOMPATIBLE_ALLOW_TAGS_PROPAGATION, rand.nextBoolean())
         .setBool(BuildLanguageOptions.EXPERIMENTAL_CC_SHARED_LIBRARY, rand.nextBoolean())
         .setBool(BuildLanguageOptions.EXPERIMENTAL_REPO_REMOTE_EXEC, rand.nextBoolean())
-        .setBool(BuildLanguageOptions.EXPERIMENTAL_ACTION_RESOURCE_SET, rand.nextBoolean())
         .setBool(BuildLanguageOptions.INCOMPATIBLE_ALWAYS_CHECK_DEPSET_ELEMENTS, rand.nextBoolean())
         .setBool(
             BuildLanguageOptions.INCOMPATIBLE_DEPSET_FOR_LIBRARIES_TO_LINK_GETTER,

src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java

@@ -392,7 +392,7 @@ public void testPackageBoundaryError_externalRepository_boundary() throws Except
             .build());
     scratch.file("BUILD", "cc_library(name = 'cclib',", "  srcs = ['r/my_sub_lib.h'])");
     invalidatePackages(
-        /*alsoConfigs=*/ false); // Repository shuffling messes with toolchain labels.
+        /* alsoConfigs= */ false); // Repository shuffling messes with toolchain labels.
     reporter.removeHandler(failFastHandler);
     getConfiguredTarget("//:cclib");
     assertContainsEvent(
@@ -413,7 +413,7 @@ public void testPackageBoundaryError_externalRepository_entirelyInside() throws
             .add("local_repository(name='r', path='/r')")
             .build());
     invalidatePackages(
-        /*alsoConfigs=*/ false); // Repository shuffling messes with toolchain labels.
+        /* alsoConfigs= */ false); // Repository shuffling messes with toolchain labels.
     reporter.removeHandler(failFastHandler);
     getConfiguredTarget("@r//:cclib");
     assertContainsEvent(
@@ -688,7 +688,6 @@ public void testCreateStarlarkActionArgumentsWithUnusedInputsList() throws Excep
 
   @Test
   public void testCreateStarlarkActionArgumentsWithResourceSet_success() throws Exception {
-    setBuildLanguageOptions("--experimental_action_resource_set");
     StarlarkRuleContext ruleContext = createRuleContext("//foo:foo");
     setRuleContext(ruleContext);
 
@@ -714,8 +713,7 @@ public void testCreateStarlarkActionArgumentsWithResourceSet_success() throws Ex
   }
 
   @Test
-  public void testCreateStarlarkActionArgumentsWithResourceSet_flagDisabled() throws Exception {
-    setBuildLanguageOptions("--noexperimental_action_resource_set");
+  public void testCreateStarlarkActionArgumentsWithResourceSet_noneResourceSet() throws Exception {
     StarlarkRuleContext ruleContext = createRuleContext("//foo:foo");
     setRuleContext(ruleContext);
 
@@ -727,7 +725,7 @@ public void testCreateStarlarkActionArgumentsWithResourceSet_flagDisabled() thro
         "ruleContext.actions.run(",
         "  inputs = ruleContext.files.srcs,",
         "  outputs = ruleContext.files.srcs,",
-        "  resource_set = get_resources,",
+        "  resource_set = None,",
         "  executable = 'executable')");
     StarlarkAction action =
         (StarlarkAction)
@@ -740,7 +738,6 @@ public void testCreateStarlarkActionArgumentsWithResourceSet_flagDisabled() thro
 
   @Test
   public void testCreateStarlarkActionArgumentsWithResourceSet_lambdaForbidden() throws Exception {
-    setBuildLanguageOptions("--experimental_action_resource_set");
     StarlarkRuleContext ruleContext = createRuleContext("//foo:foo");
     setRuleContext(ruleContext);
 
@@ -761,7 +758,6 @@ public void testCreateStarlarkActionArgumentsWithResourceSet_lambdaForbidden() t
 
   @Test
   public void testCreateStarlarkActionArgumentsWithResourceSet_illegalResource() throws Exception {
-    setBuildLanguageOptions("--experimental_action_resource_set");
     StarlarkRuleContext ruleContext = createRuleContext("//foo:foo");
     setRuleContext(ruleContext);
 
@@ -787,7 +783,6 @@ public void testCreateStarlarkActionArgumentsWithResourceSet_illegalResource() t
 
   @Test
   public void testCreateStarlarkActionArgumentsWithResourceSet_defaultValue() throws Exception {
-    setBuildLanguageOptions("--experimental_action_resource_set");
     StarlarkRuleContext ruleContext = createRuleContext("//foo:foo");
     setRuleContext(ruleContext);
 
@@ -810,7 +805,6 @@ public void testCreateStarlarkActionArgumentsWithResourceSet_defaultValue() thro
 
   @Test
   public void testCreateStarlarkActionArgumentsWithResourceSet_intDict() throws Exception {
-    setBuildLanguageOptions("--experimental_action_resource_set");
     StarlarkRuleContext ruleContext = createRuleContext("//foo:foo");
     setRuleContext(ruleContext);
 
@@ -833,7 +827,6 @@ public void testCreateStarlarkActionArgumentsWithResourceSet_intDict() throws Ex
 
   @Test
   public void testCreateStarlarkActionArgumentsWithResourceSet_notDict() throws Exception {
-    setBuildLanguageOptions("--experimental_action_resource_set");
     StarlarkRuleContext ruleContext = createRuleContext("//foo:foo");
     setRuleContext(ruleContext);
 
@@ -859,7 +852,6 @@ public void testCreateStarlarkActionArgumentsWithResourceSet_notDict() throws Ex
 
   @Test
   public void testCreateStarlarkActionArgumentsWithResourceSet_wrongDict() throws Exception {
-    setBuildLanguageOptions("--experimental_action_resource_set");
     StarlarkRuleContext ruleContext = createRuleContext("//foo:foo");
     setRuleContext(ruleContext);
 
@@ -886,7 +878,6 @@ public void testCreateStarlarkActionArgumentsWithResourceSet_wrongDict() throws
   @Test
   public void testCreateStarlarkActionArgumentsWithResourceSet_incorrectSignature()
       throws Exception {
-    setBuildLanguageOptions("--experimental_action_resource_set");
     StarlarkRuleContext ruleContext = createRuleContext("//foo:foo");
     setRuleContext(ruleContext);
 
@@ -1083,7 +1074,6 @@ public void testDeriveTreeArtifactNextToSibling() throws Exception {
     assertThat(artifact.isTreeArtifact()).isTrue();
   }
 
-
   @Test
   public void testParamFileSuffix() throws Exception {
     setRuleContext(createRuleContext("//foo:foo"));
@@ -1637,7 +1627,7 @@ public void testRelativeLabelInExternalRepository() throws Exception {
             .build());
 
     invalidatePackages(
-        /*alsoConfigs=*/ false); // Repository shuffling messes with toolchain labels.
+        /* alsoConfigs= */ false); // Repository shuffling messes with toolchain labels.
     setRuleContext(createRuleContext("@r//a:r"));
     Label depLabel = (Label) ev.eval("ruleContext.attr.internal_dep.label");
     assertThat(depLabel).isEqualTo(Label.parseCanonical("//:dep"));
@@ -1648,7 +1638,7 @@ public void testExternalWorkspaceLoad() throws Exception {
     // RepositoryDelegatorFunction deletes and creates symlink for the repository and as such is not
     // safe to execute in parallel. Disable checks with package loader to avoid parallel
     // evaluations.
-    initializeSkyframeExecutor(/*doPackageLoadingChecks=*/ false);
+    initializeSkyframeExecutor(/* doPackageLoadingChecks= */ false);
     scratch.file(
         "/r1/BUILD",
         "filegroup(name = 'test',",
@@ -1680,7 +1670,7 @@ public void testExternalWorkspaceLoad() throws Exception {
             .build());
 
     invalidatePackages(
-        /*alsoConfigs=*/ false); // Repository shuffling messes with toolchain labels.
+        /* alsoConfigs= */ false); // Repository shuffling messes with toolchain labels.
     assertThat(getConfiguredTarget("@r1//:test")).isNotNull();
   }
 
@@ -1702,7 +1692,7 @@ public void testLoadBlockRepositoryRedefinition() throws Exception {
             .build());
 
     invalidatePackages(
-        /*alsoConfigs=*/ false); // Repository shuffling messes with toolchain labels.
+        /* alsoConfigs= */ false); // Repository shuffling messes with toolchain labels.
     assertThat(
             (List)
                 getConfiguredTargetAndData("@foo//:baz")
@@ -1723,7 +1713,7 @@ public void testLoadBlockRepositoryRedefinition() throws Exception {
             .add("local_repository(name = 'foo', path = '/baz')")
             .build());
 
-    invalidatePackages(/*alsoConfigs=*/ false); // Repository shuffling messes with toolchains.
+    invalidatePackages(/* alsoConfigs= */ false); // Repository shuffling messes with toolchains.
     assertThrows(Exception.class, () -> createRuleContext("@foo//:baz"));
     assertContainsEvent(
         "Cannot redefine repository after any load statement in the WORKSPACE file "