Flip --incompatible_top_level_aspects_dependency flag

Aspect-on-aspect will be enabled by default for command line aspects.

PiperOrigin-RevId: 398224654

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<String> EXPERIMENTAL_BUILTINS_BZL_PATH =

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(

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())

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.
    *
-   * <p>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.
+   * <p>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.
    *
-   * <p>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)
+   * <p>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.
    *
-   * <p>If the incompatible_top_level_aspects_dependency flag is not used, aspects a1 and a2 will
+   * <p>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()",