Skip to content

Commit

Permalink
Deprecate the experimental_action_resource_set flag.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 715819891
Change-Id: I1c2e582a3d4c6e87e5c76837abb7a175eed3e850
  • Loading branch information
bigelephant29 authored and copybara-github committed Jan 15, 2025
1 parent 15f9d85 commit c570f97
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -832,8 +832,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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,14 @@ public static final class AllCommandGraveyardOptions extends OptionsBase {
effectTags = {OptionEffectTag.NO_OP},
help = "No-op.")
public boolean incompatibleNoPackageDistribs;

@Option(
name = "experimental_action_resource_set",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.NO_OP},
help = "No-op.")
public boolean experimentalActionResourceSet;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,20 +431,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_always_check_depset_elements",
defaultValue = "true",
Expand Down Expand Up @@ -840,7 +826,6 @@ public StarlarkSemantics toStarlarkSemantics() {
.setBool(ENABLE_WORKSPACE, enableWorkspace)
.setBool(EXPERIMENTAL_ISOLATED_EXTENSION_USAGES, experimentalIsolatedExtensionUsages)
.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)
Expand Down Expand Up @@ -959,7 +944,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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -512,10 +512,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 = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,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(),
"--experimental_dormant_deps=" + rand.nextBoolean(),
"--incompatible_always_check_depset_elements=" + rand.nextBoolean(),
"--incompatible_depset_for_libraries_to_link_getter=" + rand.nextBoolean(),
Expand Down Expand Up @@ -180,7 +179,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.EXPERIMENTAL_DORMANT_DEPS, rand.nextBoolean())
.setBool(BuildLanguageOptions.INCOMPATIBLE_ALWAYS_CHECK_DEPSET_ELEMENTS, rand.nextBoolean())
.setBool(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,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(
Expand All @@ -444,7 +444,7 @@ public void testPackageBoundaryError_externalRepository_entirelyInside() throws
scratch.overwriteFile(
"MODULE.bazel", "bazel_dep(name = 'r')", "local_path_override(module_name='r', path='/r')");
invalidatePackages(
/*alsoConfigs=*/ false); // Repository shuffling messes with toolchain labels.
/* alsoConfigs= */ false); // Repository shuffling messes with toolchain labels.
reporter.removeHandler(failFastHandler);
getConfiguredTarget("@@r+//:cclib");
assertContainsEvent(
Expand Down Expand Up @@ -733,7 +733,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);

Expand All @@ -759,8 +758,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);

Expand All @@ -772,7 +770,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)
Expand All @@ -785,7 +783,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);

Expand All @@ -806,7 +803,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);

Expand All @@ -832,7 +828,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);

Expand All @@ -855,7 +850,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);

Expand All @@ -878,7 +872,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);

Expand All @@ -904,7 +897,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);

Expand All @@ -931,7 +923,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);

Expand Down Expand Up @@ -1159,7 +1150,6 @@ public void testDeriveTreeArtifactNextToSibling() throws Exception {
assertThat(artifact.isTreeArtifact()).isTrue();
}


@Test
public void testParamFileSuffix() throws Exception {
setRuleContext(createRuleContext("//foo:foo"));
Expand Down Expand Up @@ -1750,7 +1740,7 @@ def _impl(ctx):
"MODULE.bazel", "bazel_dep(name='r')", "local_path_override(module_name='r', path='/r')");

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"));
Expand All @@ -1761,7 +1751,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);
setBuildLanguageOptions("--enable_workspace");
scratch.file(
"/r1/BUILD",
Expand Down Expand Up @@ -1802,7 +1792,7 @@ def other_macro(name, path):
.build());

invalidatePackages(
/*alsoConfigs=*/ false); // Repository shuffling messes with toolchain labels.
/* alsoConfigs= */ false); // Repository shuffling messes with toolchain labels.
assertThat(getConfiguredTarget("@r1//:test")).isNotNull();
}

Expand All @@ -1825,7 +1815,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")
Expand All @@ -1846,7 +1836,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 "
Expand Down

0 comments on commit c570f97

Please sign in to comment.