Skip to content

Commit

Permalink
Remove --experimental_action_args
Browse files Browse the repository at this point in the history
The flag is enabled by default, it is no longer experimental.

RELNOTES: Removed the flag --experimental_action_args.
PiperOrigin-RevId: 326461054
  • Loading branch information
laurentlb authored and copybara-github committed Aug 13, 2020
1 parent f6ad35f commit 39bc97e
Show file tree
Hide file tree
Showing 10 changed files with 18 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,6 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl

// <== Add new options here in alphabetic order ==>

@Option(
name = "experimental_action_args",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION},
metadataTags = {OptionMetadataTag.EXPERIMENTAL},
help =
"If set to true, Action objects support an `args` field: "
+ "a frozen Args object which contains all action arguments.")
public boolean experimentalActionArgs;

@Option(
name = "experimental_build_setting_api",
defaultValue = "true",
Expand Down Expand Up @@ -631,7 +620,6 @@ public StarlarkSemantics toStarlarkSemantics() {
StarlarkSemantics semantics =
StarlarkSemantics.builder()
// <== Add new options here in alphabetic order ==>
.experimentalActionArgs(experimentalActionArgs)
.experimentalAllowTagsPropagation(experimentalAllowTagsPropagation)
.experimentalBuiltinsBzlPath(experimentalBuiltinsBzlPath)
.experimentalCcStarlarkApiEnabledPackages(experimentalCcStarlarkApiEnabledPackages)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ public interface ActionApi extends StarlarkValue {
+ " <p>Note that some types of actions do not yet support exposure of this field."
+ " For such action types, this is <code>None</code>.",
structField = true,
allowReturnNones = true,
enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_ACTION_ARGS)
allowReturnNones = true)
Sequence<CommandLineArgsApi> getStarlarkArgs() throws EvalException;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ private FlagIdentifier() {} // uninstantiable
// consistent as they may appear in error messages.
// TODO(adonovan): move these constants up into the relevant packages of
// Bazel, and make them identical to the strings used in flag declarations.
public static final String EXPERIMENTAL_ACTION_ARGS = "experimental_action_args";
public static final String EXPERIMENTAL_DISABLE_EXTERNAL_PACKGE =
"experimental_disable_external_package";
public static final String EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT =
Expand Down Expand Up @@ -113,8 +112,6 @@ private FlagIdentifier() {} // uninstantiable
// return Boolean.TRUE.equals(map.get(flag)).
public boolean flagValue(String flag) {
switch (flag) {
case FlagIdentifier.EXPERIMENTAL_ACTION_ARGS:
return experimentalActionArgs();
case FlagIdentifier.EXPERIMENTAL_DISABLE_EXTERNAL_PACKGE:
return experimentalDisableExternalPackage();
case FlagIdentifier.EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT:
Expand Down Expand Up @@ -194,8 +191,6 @@ boolean isFeatureEnabledBasedOnTogglingFlags(String enablingFlag, String disabli
AutoValue_StarlarkSemantics.class;

// <== Add new options here in alphabetic order ==>
public abstract boolean experimentalActionArgs();

public abstract String experimentalBuiltinsBzlPath();

public abstract ImmutableList<String> experimentalCcStarlarkApiEnabledPackages();
Expand Down Expand Up @@ -307,7 +302,6 @@ public static Builder builderWithDefaults() {
public static final StarlarkSemantics DEFAULT =
builder()
// <== Add new options here in alphabetic order ==>
.experimentalActionArgs(true)
.experimentalAllowTagsPropagation(false)
.experimentalBuiltinsBzlPath("")
.experimentalCcStarlarkApiEnabledPackages(ImmutableList.of())
Expand Down Expand Up @@ -354,8 +348,6 @@ public static Builder builderWithDefaults() {
public abstract static class Builder {

// <== Add new options here in alphabetic order ==>
public abstract Builder experimentalActionArgs(boolean value);

public abstract Builder experimentalAllowTagsPropagation(boolean value);

public abstract Builder experimentalBuiltinsBzlPath(String value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ public void canGetBuilderFromInstance() {
private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws Exception {
return parseOptions(
// <== Add new options here in alphabetic order ==>
"--experimental_action_args=" + rand.nextBoolean(),
"--experimental_disable_external_package=" + rand.nextBoolean(),
"--experimental_sibling_repository_layout=" + rand.nextBoolean(),
"--experimental_builtins_bzl_path=" + rand.nextDouble(),
Expand Down Expand Up @@ -171,7 +170,6 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
private static StarlarkSemantics buildRandomSemantics(Random rand) {
return StarlarkSemantics.builder()
// <== Add new options here in alphabetic order ==>
.experimentalActionArgs(rand.nextBoolean())
.experimentalDisableExternalPackage(rand.nextBoolean())
.experimentalSiblingRepositoryLayout(rand.nextBoolean())
.experimentalBuiltinsBzlPath(String.valueOf(rand.nextDouble()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ public class EnablingAndDisablingFlagParam implements StarlarkValue {
name = "two",
type = Integer.class,
named = true,
enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_ACTION_ARGS,
disableWithFlag = FlagIdentifier.EXPERIMENTAL_ACTION_ARGS),
enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_GOOGLE_LEGACY_API,
disableWithFlag = FlagIdentifier.EXPERIMENTAL_GOOGLE_LEGACY_API),
})
public String someMethod(String one, Integer two) {
return "foo";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ public class ToggledKwargsParam implements StarlarkValue {
},
extraPositionals = @Param(name = "args"),
extraKeywords =
@Param(name = "kwargs", enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_ACTION_ARGS),
@Param(
name = "kwargs",
enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_GOOGLE_LEGACY_API),
useStarlarkThread = true)
public String toggledKwargsMethod(
String one, Integer two, Sequence<?> args, Dict<?, ?> kwargs, StarlarkThread thread) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class ToggledParamNoDisabledValue implements StarlarkValue {
@Param(
name = "two",
named = true,
enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_ACTION_ARGS,
enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_GOOGLE_LEGACY_API,
positional = true)
})
public Integer noDisabledValueMethod(Integer one, Integer two) {
Expand Down
12 changes: 6 additions & 6 deletions src/test/shell/bazel/cc_integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ EOF

bazel build "${package}:x" \
--aspects="//${package}:lib.bzl%actions_test_aspect" \
--output_groups=out --experimental_action_args
--output_groups=out

cat "bazel-bin/${package}/aspect_out" | grep "\(ar\|libtool\)" \
|| fail "args didn't contain the tool path"
Expand Down Expand Up @@ -741,7 +741,7 @@ EOF

bazel build "${package}:x" \
--aspects="//${package}:lib.bzl%actions_test_aspect" \
--output_groups=out --experimental_action_args
--output_groups=out

cat "bazel-bin/${package}/aspect_out" | \
grep "\(gcc\|clang\|clanc-cl.exe\|cl.exe\)" \
Expand Down Expand Up @@ -898,14 +898,14 @@ EOF
# Test that actions are reconstructible under default configuration
bazel build "${package}:a" \
--aspects="//${package}:lib.bzl%actions_test_aspect" \
--output_groups=out --experimental_action_args || \
--output_groups=out || \
fail "bazel build should've passed"

# Test that compile actions are reconstructible when using param files
bazel build "${package}:a" \
# Test that compile actions are reconstructible when using param files
bazel build "${package}:a" \
--features=compiler_param_file \
--aspects="//${package}:lib.bzl%actions_test_aspect" \
--output_groups=out --experimental_action_args || \
--output_groups=out || \
fail "bazel build should've passed with --features=compiler_param_file"
}

Expand Down
10 changes: 4 additions & 6 deletions src/test/shell/integration/action_aspect_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,13 @@ load(":lib.bzl", "tree_art_rule")
tree_art_rule(name = "x")
EOF

bazel build package:x --experimental_action_args \
|| fail "Unexpected build failure"
bazel build package:x || fail "Unexpected build failure"

cat "${PRODUCT_NAME}-bin/package/digest" | grep "a.txt.*b.txt.*c.txt" \
|| fail "rule digest does not contain tree artifact args"

bazel build package:x --aspects=//package:lib.bzl%actions_test_aspect \
--output_groups=out --experimental_action_args
--output_groups=out

cat "${PRODUCT_NAME}-bin/package/aspect_out" | grep "a.txt.*b.txt.*c.txt" \
|| fail "aspect Args do not contain tree artifact args"
Expand Down Expand Up @@ -218,14 +217,13 @@ load(":lib.bzl", "tree_art_rule")
tree_art_rule(name = "x")
EOF

bazel build package:x --experimental_action_args \
|| fail "Unexpected build failure"
bazel build package:x || fail "Unexpected build failure"

cat "${PRODUCT_NAME}-bin/package/digest" | grep "a.txt.*b.txt.*c.txt" \
|| fail "rule digest does not contain tree artifact args"

bazel build package:x --aspects=//package:lib.bzl%actions_test_aspect \
--output_groups=out --experimental_action_args
--output_groups=out

cat "${PRODUCT_NAME}-bin/package/raw_args_out" | grep ".params" \
|| fail "aspect Args does not contain a params file"
Expand Down
2 changes: 1 addition & 1 deletion src/test/shell/integration/java_integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,7 @@ EOF

bazel build "${package}:x" \
--aspects="//${package}:lib.bzl%actions_test_aspect" \
--output_groups=out --experimental_action_args
--output_groups=out

cat "${PRODUCT_NAME}-bin/${package}/aspect_out" | grep "0.params .*1.params" \
|| fail "aspect Args do not contain both params files"
Expand Down

0 comments on commit 39bc97e

Please sign in to comment.