Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.5.0] Deprecate the experimental_action_resource_set flag. #24939

Merged
merged 1 commit into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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);

Expand All @@ -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);

Expand All @@ -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)
Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

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


@Test
public void testParamFileSuffix() throws Exception {
setRuleContext(createRuleContext("//foo:foo"));
Expand Down Expand Up @@ -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"));
Expand All @@ -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',",
Expand Down Expand Up @@ -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();
}

Expand All @@ -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")
Expand All @@ -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 "
Expand Down
Loading