From 9a765c8b498a72e20af6c391bef01e794913c317 Mon Sep 17 00:00:00 2001 From: messa Date: Wed, 22 Sep 2021 06:28:52 -0700 Subject: [PATCH] Flip --incompatible_top_level_aspects_dependency flag Aspect-on-aspect will be enabled by default for command line aspects. PiperOrigin-RevId: 398224654 --- .../semantics/BuildLanguageOptions.java | 4 +- .../build/lib/analysis/AspectTest.java | 2 +- .../packages/semantics/ConsistencyTest.java | 2 + .../starlark/StarlarkDefinedAspectsTest.java | 46 +++---------------- 4 files changed, 12 insertions(+), 42 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index b1e555c7712003..a92fd0f9fb5d97 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -560,7 +560,7 @@ public class BuildLanguageOptions extends OptionsBase implements Serializable { @Option( name = "incompatible_top_level_aspects_dependency", - defaultValue = "false", + defaultValue = "true", documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, @@ -718,7 +718,7 @@ public StarlarkSemantics toStarlarkSemantics() { public static final String INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS = "-incompatible_top_level_aspects_require_providers"; public static final String INCOMPATIBLE_TOP_LEVEL_ASPECTS_DEPENDENCY = - "-incompatible_top_level_aspects_dependency"; + "+incompatible_top_level_aspects_dependency"; // non-booleans public static final StarlarkSemantics.Key EXPERIMENTAL_BUILTINS_BZL_PATH = diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java index 6277ec87937540..618b6ae8db4cb9 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java @@ -885,6 +885,7 @@ public void duplicateTopLevelAspects_duplicateAspectsIgnored() throws Exception AspectApplyingToFiles aspectApplyingToFiles = new AspectApplyingToFiles(); setRulesAndAspectsAvailableInTests(ImmutableList.of(aspectApplyingToFiles), ImmutableList.of()); pkg("a", "java_binary(name = 'x', main_class = 'x.FooBar', srcs = ['x.java'])"); + useConfiguration("--noincompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update( @@ -904,7 +905,6 @@ public void duplicateTopLevelAspects_duplicateAspectsNotAllowed() throws Excepti AspectApplyingToFiles aspectApplyingToFiles = new AspectApplyingToFiles(); setRulesAndAspectsAvailableInTests(ImmutableList.of(aspectApplyingToFiles), ImmutableList.of()); pkg("a", "java_binary(name = 'x', main_class = 'x.FooBar', srcs = ['x.java'])"); - useConfiguration("--incompatible_top_level_aspects_dependency"); reporter.removeHandler(failFastHandler); assertThrows( diff --git a/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java index 16cfe4dfff18ce..c93ab8a14dfc02 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java @@ -147,6 +147,7 @@ private static BuildLanguageOptions buildRandomOptions(Random rand) throws Excep "--incompatible_no_rule_outputs_param=" + rand.nextBoolean(), "--incompatible_run_shell_command_string=" + rand.nextBoolean(), "--incompatible_struct_has_no_methods=" + rand.nextBoolean(), + "--incompatible_top_level_aspects_dependency=" + rand.nextBoolean(), "--incompatible_visibility_private_attributes_at_definition=" + rand.nextBoolean(), "--incompatible_require_linker_input_cc_api=" + rand.nextBoolean(), "--incompatible_restrict_string_escapes=" + rand.nextBoolean(), @@ -197,6 +198,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) { .setBool(BuildLanguageOptions.INCOMPATIBLE_NO_RULE_OUTPUTS_PARAM, rand.nextBoolean()) .setBool(BuildLanguageOptions.INCOMPATIBLE_RUN_SHELL_COMMAND_STRING, rand.nextBoolean()) .setBool(BuildLanguageOptions.INCOMPATIBLE_STRUCT_HAS_NO_METHODS, rand.nextBoolean()) + .setBool(BuildLanguageOptions.INCOMPATIBLE_TOP_LEVEL_ASPECTS_DEPENDENCY, rand.nextBoolean()) .setBool( BuildLanguageOptions.INCOMPATIBLE_VISIBILITY_PRIVATE_ATTRIBUTES_AT_DEFINITION, rand.nextBoolean()) diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java index f6504c9fcb72bb..cd217a87ad6c67 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java @@ -3429,7 +3429,6 @@ public void testAspectRequiredProviders_listOfRequiredProvidersLists() throws Ex @Test public void testAspectRequiredByMultipleAspects_inheritsAttrAspects() throws Exception { - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "prov_a = provider()", @@ -3533,7 +3532,6 @@ public void testAspectRequiredByMultipleAspects_inheritsAttrAspects() throws Exc @Test public void testAspectRequiredByMultipleAspects_inheritsRequiredProviders() throws Exception { - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "aspect_prov_a = provider()", @@ -3686,7 +3684,6 @@ public void testAspectRequiredByMultipleAspects_inheritsRequiredProviders() thro @Test public void testAspectRequiredByMultipleAspects_withDifferentParametersValues() throws Exception { - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "prov_a = provider()", @@ -3820,7 +3817,6 @@ public void testAspectRequiredByMultipleAspects_withDifferentParametersValues() @Test public void testAspectRequiresAspect_requireNativeAspect() throws Exception { exposeNativeAspectToStarlark(); - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "prov_a = provider()", @@ -4664,7 +4660,6 @@ public void testTopLevelAspectOnAspect_stackOfAspects() throws Exception { "simple_rule(", " name = 'dep_target',", ")"); - useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update( @@ -4769,7 +4764,6 @@ public void testTopLevelAspectOnAspect_requiredProviderProvidedTwiceFailed() thr "simple_rule(", " name = 'dep_target',", ")"); - useConfiguration("--incompatible_top_level_aspects_dependency"); reporter.removeHandler(failFastHandler); // The call to `update` does not throw an exception when "--keep_going" is passed in the @@ -4854,7 +4848,6 @@ public void testTopLevelAspectOnAspect_requiredProviderProvidedTwicePassed() thr "simple_rule(", " name = 'dep_target',", ")"); - useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update( @@ -4926,7 +4919,6 @@ public void testTopLevelAspectOnAspect_requiredProviderNotProvided() throws Exce "simple_rule(", " name = 'dep_target',", ")"); - useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update(ImmutableList.of("test/defs.bzl%a2", "test/defs.bzl%a1"), "//test:main"); @@ -4999,7 +4991,6 @@ public void testTopLevelAspectOnAspect_requiredProviderProvidedAfterTheAspect() "simple_rule(", " name = 'dep_target',", ")"); - useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update(ImmutableList.of("test/defs.bzl%a1", "test/defs.bzl%a2"), "//test:main"); @@ -5078,7 +5069,6 @@ public void testTopLevelAspectOnAspect_differentAttrAspects() throws Exception { "simple_rule(", " name = 'extra_dep_target',", ")"); - useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update(ImmutableList.of("test/defs.bzl%a2", "test/defs.bzl%a1"), "//test:main"); @@ -5174,7 +5164,6 @@ public void testTopLevelAspectOnAspect_differentRequiredRuleProviders() throws E "rule_with_prov_b(", " name = 'target_with_prov_b',", ")"); - useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update(ImmutableList.of("test/defs.bzl%a2", "test/defs.bzl%a1"), "//test:main"); @@ -5267,7 +5256,6 @@ public void testTopLevelAspectOnAspect_providerRequiredByMultipleAspects() throw "simple_rule(", " name = 'dep_target',", ")"); - useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update( @@ -5401,7 +5389,6 @@ public void testTopLevelAspectOnAspect_diamondCase() throws Exception { "r0(", " name = 't0',", ")"); - useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update( @@ -5427,8 +5414,8 @@ public void testTopLevelAspectOnAspect_diamondCase() throws Exception { /** * --aspects = a1, a2, a1: aspect a1 requires a2p, a2 provides a2p. * - *

top level aspects list is deduplicated by default and only the first occurrence of a1 will - * be there so a1 won't get the value of a2p. + *

top level aspects list is deduplicated when --incompatible_top_level_aspects_dependency is + * disabled and only the first occurrence of a1 will be there so a1 won't get the value of a2p. */ @Test public void testTopLevelAspectOnAspect_duplicateAspectsIgnored() throws Exception { @@ -5481,6 +5468,7 @@ public void testTopLevelAspectOnAspect_duplicateAspectsIgnored() throws Exceptio "simple_rule(", " name = 'dep_target',", ")"); + useConfiguration("--noincompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update( @@ -5551,7 +5539,6 @@ public void testTopLevelAspectOnAspect_duplicateAspectsNotAllowed() throws Excep "simple_rule(", " name = 'dep_target',", ")"); - useConfiguration("--incompatible_top_level_aspects_dependency"); reporter.removeHandler(failFastHandler); // The call to `update` does not throw an exception when "--keep_going" is passed in the @@ -5630,7 +5617,6 @@ public void testTopLevelAspectOnAspect_requiredAspectProviderOnlyAvailableOnDep( "simple_rule(", " name = 'dep_target',", ")"); - useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update(ImmutableList.of("test/defs.bzl%a1"), "//test:main"); @@ -5697,7 +5683,6 @@ public void testTopLevelAspectOnAspect_multipleTopLevelTargets() throws Exceptio "simple_rule(", " name = 't2',", ")"); - useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update(ImmutableList.of("test/defs.bzl%a2", "test/defs.bzl%a1"), "//test:t2", "//test:t1"); @@ -5783,7 +5768,6 @@ public void testTopLevelAspectOnAspect_multipleRequiredProviders() throws Except "simple_rule(", " name = 'dep_target',", ")"); - useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update( @@ -5879,7 +5863,6 @@ public void testTopLevelAspectOnAspect_multipleRequiredProviders2() throws Excep "simple_rule(", " name = 'dep_target',", ")"); - useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update( @@ -5914,9 +5897,8 @@ public void testTopLevelAspectOnAspect_multipleRequiredProviders2() throws Excep * aspects = a1, a2; aspect a1 provides a1p provider and aspect a2 requires a1p provider. These * top-level aspects are applied on top-level target `main` whose rule also provides a1p. * - *

If the incompatible_top_level_aspects_dependency flag is used to establish the dependency - * between a1 and a2, the build will fail since a2 will receive provider a1p twice (from a1 - * applied on `main` and from `main` target itself) + *

By default, the dependency between a1 and a2 will be established, the build will fail since + * a2 will receive provider a1p twice (from a1 applied on `main` and from `main` target itself). */ @Test public void testTopLevelAspects_duplicateRuleProviderError() throws Exception { @@ -5951,7 +5933,6 @@ public void testTopLevelAspects_duplicateRuleProviderError() throws Exception { " implementation = _my_rule_impl,", ")"); scratch.file("test/BUILD", "load('//test:defs.bzl', 'my_rule')", "my_rule(name = 'main')"); - useConfiguration("--incompatible_top_level_aspects_dependency"); reporter.removeHandler(failFastHandler); // The call to `update` does not throw an exception when "--keep_going" is passed in the @@ -5972,7 +5953,7 @@ public void testTopLevelAspects_duplicateRuleProviderError() throws Exception { * aspects = a1, a2; aspect a1 provides a1p provider and aspect a2 requires a1p provider. These * top-level aspects are applied on top-level target `main` whose rule also provides a1p. * - *

If the incompatible_top_level_aspects_dependency flag is not used, aspects a1 and a2 will + *

If the incompatible_top_level_aspects_dependency flag is disabled, aspects a1 and a2 will * run independently and the build will succeed. a2 will only see the value of a1p provided by * my_rule. */ @@ -6009,6 +5990,7 @@ public void testTopLevelAspects_duplicateRuleProviderPassed() throws Exception { " implementation = _my_rule_impl,", ")"); scratch.file("test/BUILD", "load('//test:defs.bzl', 'my_rule')", "my_rule(name = 'main')"); + useConfiguration("--noincompatible_top_level_aspects_dependency"); reporter.removeHandler(failFastHandler); AnalysisResult analysisResult = @@ -6026,7 +6008,6 @@ public void testTopLevelAspects_duplicateRuleProviderPassed() throws Exception { @Test public void testTopLevelAspectRequiresAspect_stackOfRequiredAspects() throws Exception { - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -6048,7 +6029,6 @@ public void testTopLevelAspectRequiresAspect_stackOfRequiredAspects() throws Exc @Test public void testTopLevelAspectRequiresAspect_aspectRequiredByMultipleAspects() throws Exception { - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -6072,7 +6052,6 @@ public void testTopLevelAspectRequiresAspect_aspectRequiredByMultipleAspects() t @Test public void testTopLevelAspectRequiresAspect_aspectRequiredByMultipleAspects2() throws Exception { - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -6096,7 +6075,6 @@ public void testTopLevelAspectRequiresAspect_aspectRequiredByMultipleAspects2() @Test public void testTopLevelAspectRequiresAspect_requireExistingAspect_passed() throws Exception { - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -6118,7 +6096,6 @@ public void testTopLevelAspectRequiresAspect_requireExistingAspect_passed() thro @Test public void testTopLevelAspectRequiresAspect_requireExistingAspect_failed() throws Exception { - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -6153,7 +6130,6 @@ public void testTopLevelAspectRequiresAspect_requireExistingAspect_failed() thro public void testTopLevelAspectRequiresAspect_requiredNativeAspect_parametersNotAllowed() throws Exception { exposeNativeAspectToStarlark(); - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -6181,7 +6157,6 @@ public void testTopLevelAspectRequiresAspect_requiredNativeAspect_parametersNotA @Test public void testTopLevelAspectRequiresAspect_requiredStarlarkAspect_parametersNotAllowed() throws Exception { - useConfiguration("--incompatible_top_level_aspects_dependency"); reporter.removeHandler(failFastHandler); scratch.file( "test/defs.bzl", @@ -6211,7 +6186,6 @@ public void testTopLevelAspectRequiresAspect_requiredStarlarkAspect_parametersNo @Test public void testTopLevelAspectRequiresAspect_ruleAttributes() throws Exception { - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "RequiredAspectProv = provider()", @@ -6299,7 +6273,6 @@ public void testTopLevelAspectRequiresAspect_inheritPropagationAttributes() thro // base_aspect propagates over base_dep attribute and requires first_required_aspect which // propagates over first_dep attribute and requires second_required_aspect which propagates over // second_dep attribute - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "BaseAspectProv = provider()", @@ -6403,7 +6376,6 @@ public void testTopLevelAspectRequiresAspect_inheritPropagationAttributes() thro public void testTopLevelAspectRequiresAspect_inheritRequiredProviders() throws Exception { // aspect_a requires provider Prov_A and requires aspect_b which requires // provider Prov_B and requires aspect_c which requires provider Prov_C - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "Prov_A = provider()", @@ -6529,7 +6501,6 @@ public void testTopLevelAspectRequiresAspect_inheritRequiredProviders() throws E @Test public void testTopLevelAspectRequiresAspect_inspectRequiredAspectActions() throws Exception { - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "BaseAspectProvider = provider()", @@ -6586,7 +6557,6 @@ public void testTopLevelAspectRequiresAspect_inspectRequiredAspectActions() thro @Test public void testTopLevelAspectRequiresAspect_inspectRequiredAspectGeneratedFiles() throws Exception { - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "BaseAspectProvider = provider()", @@ -6637,7 +6607,6 @@ public void testTopLevelAspectRequiresAspect_inspectRequiredAspectGeneratedFiles @Test public void testTopLevelAspectRequiresAspect_withRequiredAspectProvidersSatisfied() throws Exception { - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "prov_a = provider()", @@ -6728,7 +6697,6 @@ public void testTopLevelAspectRequiresAspect_withRequiredAspectProvidersSatisfie @Test public void testTopLevelAspectRequiresAspect_withRequiredAspectProvidersNotFound() throws Exception { - useConfiguration("--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "prov_a = provider()",