From ed251187b078c4262bbbc1da72015ce12f9964f4 Mon Sep 17 00:00:00 2001 From: messa Date: Fri, 13 Aug 2021 05:01:17 -0700 Subject: [PATCH] Add incompatible flag to guard top-level aspects dependencies This CL introduces `incompatible_top_level_aspects_dependency` flag to enable aspect-on-aspect and requiring aspects for command line aspects. This can be needed because the new behavior can break builds where a relation between top-level aspects existed (based on required_aspect_providers and provides) but it was never actually applied as it was not supported. One example of these cases: blaze build //:main --aspects=/tools:my_def.bzl%a1,/tools:my_def.bzl%a2 If aspect a1 provides a1p provider, aspect a2 requires a1p provider and the rule of target `main` also provides a1p. Once top-level aspect-on-aspect is enabled this build will fail because a1p will be provided twice to a2 (from a1 applied on `main` and from `main` target rule). Previously the relation between a1 and a2 was not detected and a2 used to get only the value of a1p from `main`'s rule. PiperOrigin-RevId: 390587290 --- .../build/lib/analysis/AnalysisOptions.java | 15 -- .../build/lib/analysis/BuildView.java | 4 - .../lib/buildtool/BuildRequestOptions.java | 23 +- .../semantics/BuildLanguageOptions.java | 21 ++ .../BuildTopLevelAspectsDetailsFunction.java | 76 +++++- .../build/lib/analysis/AspectTest.java | 2 +- .../starlark/StarlarkDefinedAspectsTest.java | 216 +++++++++++++++--- 7 files changed, 290 insertions(+), 67 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisOptions.java index a39ddb6b2669bf..e8662a2b35b18a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisOptions.java @@ -166,19 +166,4 @@ public class AnalysisOptions extends OptionsBase { + " be used. Example value: \"HOST_CPUS*0.5\".", converter = CpuResourceConverter.class) public int oomSensitiveSkyFunctionsSemaphoreSize; - - @Option( - name = "incompatible_ignore_duplicate_top_level_aspects", - defaultValue = "true", - documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, - metadataTags = { - OptionMetadataTag.INCOMPATIBLE_CHANGE, - OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES - }, - effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, - help = - "If true, remove duplicates from --aspects list by keeping only the first occurrence of" - + " every aspect. Otherwise, throw validation error if duplicate aspects are" - + " encountered.") - public boolean ignoreDuplicateTopLevelAspects; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java index 2e289f3571808d..f835927ef56fa0 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java @@ -275,10 +275,6 @@ public AnalysisResult update( .collect(Collectors.toList()); ImmutableList.Builder aspectClassesBuilder = ImmutableList.builder(); - if (viewOptions.ignoreDuplicateTopLevelAspects) { - // remove duplicates from aspects list - aspects = ImmutableSet.copyOf(aspects).asList(); - } for (String aspect : aspects) { // Syntax: label%aspect int delimiterPosition = aspect.indexOf('%'); diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java index c25a91080a3e95..89d6031fc88e58 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java @@ -306,17 +306,18 @@ public class BuildRequestOptions extends OptionsBase { allowMultiple = true, help = "Comma-separated list of aspects to be applied to top-level targets. All aspects are" - + " applied to all top-level targets. If aspect some_aspect specifies" - + " required aspect providers via required_aspect_providers," - + " some_aspect will run after every aspect that was mentioned before it" - + " in the aspects list and whose advertised providers satisfy" - + " some_aspect required aspect providers. some_aspect will" - + " then have access to the values of those aspects' providers. Aspects that do not" - + " have such dependency will run independently on the top-level targets." - + "" - + " Aspects are specified in the form %, for example" - + " '//tools:my_def.bzl%my_aspect', where 'my_aspect' is a top-level value from a" - + " file tools/my_def.bzl") + + " applied independently to all top-level targets except if" + + " incompatible_top_level_aspects_dependency is used. In this case, if" + + " aspect some_aspect specifies required aspect providers via" + + " required_aspect_providers, some_aspect will run after" + + " every aspect that was mentioned before it in the aspects list whose advertised" + + " providers satisfy some_aspect required aspect providers. Moreover," + + " some_aspect will run after all its required aspects specified by" + + " requires attribute which otherwise will be ignored." + + " some_aspect will then have access to the values of those aspects'" + + " providers." + + " %, for example '//tools:my_def.bzl%my_aspect', where" + + " 'my_aspect' is a top-level value from a file tools/my_def.bzl") public List aspects; public BuildRequestOptions() throws OptionsParsingException {} 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 484bfe4ea994b7..480eac7875afd9 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 @@ -619,6 +619,22 @@ public class BuildLanguageOptions extends OptionsBase implements Serializable { + " providers of the aspect.") public boolean incompatibleTopLevelAspectsRequireProviders; + @Option( + name = "incompatible_top_level_aspects_dependency", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + metadataTags = { + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, + help = + "If set to true, a dependency between the top level aspects will be built based on their" + + " required aspect providers, advertised providers and required aspects. Otherwise," + + " each aspect in the list will run independently and its required aspects will be" + + " ignored.") + public boolean incompatibleTopLevelAspectsDependOnAspects; + @Option( name = "experimental_required_aspects", defaultValue = "false", @@ -698,6 +714,9 @@ public StarlarkSemantics toStarlarkSemantics() { .setBool( INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS, incompatibleTopLevelAspectsRequireProviders) + .setBool( + INCOMPATIBLE_TOP_LEVEL_ASPECTS_DEPENDENCY, + incompatibleTopLevelAspectsDependOnAspects) .setBool(EXPERIMENTAL_REQUIRED_ASPECTS, experimentalRequiredAspects) .build(); return INTERNER.intern(semantics); @@ -770,6 +789,8 @@ public StarlarkSemantics toStarlarkSemantics() { "-incompatible_visibility_private_attributes_at_definition"; 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"; public static final String EXPERIMENTAL_REQUIRED_ASPECTS = "-experimental_required_aspects"; // non-booleans diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildTopLevelAspectsDetailsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildTopLevelAspectsDetailsFunction.java index eb68e55402fc66..78db2c453bfa8c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildTopLevelAspectsDetailsFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildTopLevelAspectsDetailsFunction.java @@ -16,6 +16,7 @@ import com.google.common.base.Objects; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Interner; import com.google.devtools.build.lib.analysis.AspectCollection; import com.google.devtools.build.lib.analysis.AspectCollection.AspectCycleOnPathException; @@ -28,6 +29,7 @@ import com.google.devtools.build.lib.packages.NativeAspectClass; import com.google.devtools.build.lib.packages.StarlarkAspect; import com.google.devtools.build.lib.packages.StarlarkAspectClass; +import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.server.FailureDetails.Analysis; import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; @@ -46,6 +48,7 @@ import java.util.Map; import javax.annotation.Nullable; import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.StarlarkSemantics; /** * SkyFunction to load top level aspects, build the dependency relation between them based on the @@ -68,11 +71,33 @@ public class BuildTopLevelAspectsDetailsFunction implements SkyFunction { @Override public SkyValue compute(SkyKey skyKey, Environment env) throws BuildTopLevelAspectsDetailsFunctionException, InterruptedException { + BuildTopLevelAspectsDetailsKey topLevelAspectsDetailsKey = (BuildTopLevelAspectsDetailsKey) skyKey.argument(); + ImmutableList topLevelAspectsClasses = + topLevelAspectsDetailsKey.getTopLevelAspectsClasses(); + + StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env); + if (starlarkSemantics == null) { + return null; + } + boolean buildTopLevelAspectsDependency = + starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_TOP_LEVEL_ASPECTS_DEPENDENCY); + if (!buildTopLevelAspectsDependency) { + // If building a relation between top-level aspects is not required, then we can remove + // duplicate aspects by keeping the first occurrence of each aspect. + topLevelAspectsClasses = ImmutableSet.copyOf(topLevelAspectsClasses).asList(); + + // Then return a list of indenpendent aspects to be applied on the top-level targets + ImmutableList aspectsDetailsList = + getIndependentTopLevelAspects(env, topLevelAspectsClasses); + if (aspectsDetailsList == null) { + return null; // some aspects are not loaded + } + return new BuildTopLevelAspectsDetailsValue(aspectsDetailsList); + } - ImmutableList topLevelAspects = - getTopLevelAspects(env, topLevelAspectsDetailsKey.getTopLevelAspectsClasses()); + ImmutableList topLevelAspects = getTopLevelAspects(env, topLevelAspectsClasses); if (topLevelAspects == null) { return null; // some aspects are not loaded @@ -98,11 +123,9 @@ public String extractTag(SkyKey skyKey) { } @Nullable - private static ImmutableList getTopLevelAspects( + private static Map loadAspects( Environment env, ImmutableList topLevelAspectsClasses) - throws InterruptedException, BuildTopLevelAspectsDetailsFunctionException { - AspectsListBuilder aspectsList = new AspectsListBuilder(); - + throws InterruptedException { ImmutableList.Builder aspectLoadingKeys = ImmutableList.builder(); for (AspectClass aspectClass : topLevelAspectsClasses) { if (aspectClass instanceof StarlarkAspectClass) { @@ -116,6 +139,47 @@ private static ImmutableList getTopLevelAspects( if (env.valuesMissing()) { return null; } + return loadedAspects; + } + + @Nullable + private static ImmutableList getIndependentTopLevelAspects( + Environment env, ImmutableList topLevelAspectsClasses) + throws InterruptedException { + Map loadedAspects = loadAspects(env, topLevelAspectsClasses); + if (loadedAspects == null) { + return null; + } + ImmutableList.Builder aspectDetailsList = ImmutableList.builder(); + + for (AspectClass aspectClass : topLevelAspectsClasses) { + if (aspectClass instanceof StarlarkAspectClass) { + StarlarkAspectLoadingValue aspectLoadingValue = + (StarlarkAspectLoadingValue) + loadedAspects.get( + LoadStarlarkAspectFunction.createStarlarkAspectLoadingKey( + (StarlarkAspectClass) aspectClass)); + StarlarkAspect starlarkAspect = aspectLoadingValue.getAspect(); + aspectDetailsList.add( + new AspectDetails( + ImmutableList.of(), new AspectDescriptor(starlarkAspect.getAspectClass()))); + } else { + aspectDetailsList.add( + new AspectDetails(ImmutableList.of(), new AspectDescriptor(aspectClass))); + } + } + return aspectDetailsList.build(); + } + + @Nullable + private static ImmutableList getTopLevelAspects( + Environment env, ImmutableList topLevelAspectsClasses) + throws InterruptedException, BuildTopLevelAspectsDetailsFunctionException { + AspectsListBuilder aspectsList = new AspectsListBuilder(); + Map loadedAspects = loadAspects(env, topLevelAspectsClasses); + if (loadedAspects == null) { + return null; + } for (AspectClass aspectClass : topLevelAspectsClasses) { if (aspectClass instanceof StarlarkAspectClass) { 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 f29f0e77ed19e0..6277ec87937540 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 @@ -904,7 +904,7 @@ 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_ignore_duplicate_top_level_aspects=false"); + useConfiguration("--incompatible_top_level_aspects_dependency"); reporter.removeHandler(failFastHandler); assertThrows( 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 f67cb7bfc2215a..8c179868849566 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 @@ -4299,6 +4299,7 @@ public void testTopLevelAspectOnAspect_stackOfAspects() throws Exception { "simple_rule(", " name = 'dep_target',", ")"); + useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update( @@ -4403,6 +4404,7 @@ 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 @@ -4487,6 +4489,7 @@ public void testTopLevelAspectOnAspect_requiredProviderProvidedTwicePassed() thr "simple_rule(", " name = 'dep_target',", ")"); + useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update( @@ -4558,6 +4561,7 @@ 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"); @@ -4630,6 +4634,7 @@ 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"); @@ -4708,6 +4713,7 @@ 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"); @@ -4803,6 +4809,7 @@ 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"); @@ -4895,6 +4902,7 @@ public void testTopLevelAspectOnAspect_providerRequiredByMultipleAspects() throw "simple_rule(", " name = 'dep_target',", ")"); + useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update( @@ -5028,6 +5036,7 @@ public void testTopLevelAspectOnAspect_diamondCase() throws Exception { "r0(", " name = 't0',", ")"); + useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update( @@ -5177,7 +5186,7 @@ public void testTopLevelAspectOnAspect_duplicateAspectsNotAllowed() throws Excep "simple_rule(", " name = 'dep_target',", ")"); - useConfiguration("--incompatible_ignore_duplicate_top_level_aspects=false"); + 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 @@ -5256,6 +5265,7 @@ 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"); @@ -5322,6 +5332,7 @@ 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"); @@ -5407,6 +5418,7 @@ public void testTopLevelAspectOnAspect_multipleRequiredProviders() throws Except "simple_rule(", " name = 'dep_target',", ")"); + useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update( @@ -5502,6 +5514,7 @@ public void testTopLevelAspectOnAspect_multipleRequiredProviders2() throws Excep "simple_rule(", " name = 'dep_target',", ")"); + useConfiguration("--incompatible_top_level_aspects_dependency"); AnalysisResult analysisResult = update( @@ -5532,9 +5545,124 @@ public void testTopLevelAspectOnAspect_multipleRequiredProviders2() throws Excep "aspect a2 on target //test:main sees a3p = a3p_val"); } + /** + * 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) + */ + @Test + public void testTopLevelAspects_duplicateRuleProviderError() throws Exception { + scratch.file( + "test/defs.bzl", + "a1p = provider()", + "a2p = provider()", + "", + "def _a1_impl(target, ctx):", + " return [a1p(value = 'aspect_a1p_val')]", + "a1 = aspect(", + " implementation = _a1_impl,", + " provides = [a1p],", + ")", + "", + "def _a2_impl(target, ctx):", + " result = 'aspect a2 on target {}'.format(target.label)", + " if a1p in target:", + " result += ' sees a1p = {}'.format(target[a1p].value)", + " else:", + " result += ' cannot see a1p'", + " return [a2p(value = result)]", + "a2 = aspect(", + " implementation = _a2_impl,", + " provides = [a2p],", + " required_aspect_providers = [a1p]", + ")", + "", + "def _my_rule_impl(ctx):", + " return [a1p(value = 'rule_a1p_val')]", + "my_rule = rule(", + " 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 + // WithKeepGoing test suite. Otherwise, it throws ViewCreationFailedException. + if (keepGoing()) { + AnalysisResult result = + update(ImmutableList.of("test/defs.bzl%a1", "test/defs.bzl%a2"), "//test:main"); + assertThat(result.hasError()).isTrue(); + } else { + assertThrows( + ViewCreationFailedException.class, + () -> update(ImmutableList.of("test/defs.bzl%a1", "test/defs.bzl%a2"), "//test:main")); + } + assertContainsEvent("Provider a1p provided twice"); + } + + /** + * 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 + * run independently and the build will succeed. a2 will only see the value of a1p provided by + * my_rule. + */ + @Test + public void testTopLevelAspects_duplicateRuleProviderPassed() throws Exception { + scratch.file( + "test/defs.bzl", + "a1p = provider()", + "a2p = provider()", + "", + "def _a1_impl(target, ctx):", + " return [a1p(value = 'aspect_a1p_val')]", + "a1 = aspect(", + " implementation = _a1_impl,", + " provides = [a1p],", + ")", + "", + "def _a2_impl(target, ctx):", + " result = 'aspect a2 on target {}'.format(target.label)", + " if a1p in target:", + " result += ' sees a1p = {}'.format(target[a1p].value)", + " else:", + " result += ' cannot see a1p'", + " return [a2p(value = result)]", + "a2 = aspect(", + " implementation = _a2_impl,", + " provides = [a2p],", + " required_aspect_providers = [a1p]", + ")", + "", + "def _my_rule_impl(ctx):", + " return [a1p(value = 'rule_a1p_val')]", + "my_rule = rule(", + " implementation = _my_rule_impl,", + ")"); + scratch.file("test/BUILD", "load('//test:defs.bzl', 'my_rule')", "my_rule(name = 'main')"); + reporter.removeHandler(failFastHandler); + + AnalysisResult analysisResult = + update(ImmutableList.of("test/defs.bzl%a1", "test/defs.bzl%a2"), "//test:main"); + + Map configuredAspects = analysisResult.getAspectsMap(); + ConfiguredAspect a2 = getConfiguredAspect(configuredAspects, "a2"); + assertThat(a2).isNotNull(); + StarlarkProvider.Key a2Result = + new StarlarkProvider.Key(Label.parseAbsolute("//test:defs.bzl", ImmutableMap.of()), "a2p"); + StructImpl a2ResultProvider = (StructImpl) a2.get(a2Result); + assertThat((String) a2ResultProvider.getValue("value")) + .isEqualTo("aspect a2 on target //test:main sees a1p = rule_a1p_val"); + } + @Test public void testTopLevelAspectRequiresAspect_stackOfRequiredAspects() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -5556,7 +5684,8 @@ public void testTopLevelAspectRequiresAspect_stackOfRequiredAspects() throws Exc @Test public void testTopLevelAspectRequiresAspect_aspectRequiredByMultipleAspects() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -5580,7 +5709,8 @@ public void testTopLevelAspectRequiresAspect_aspectRequiredByMultipleAspects() t @Test public void testTopLevelAspectRequiresAspect_aspectRequiredByMultipleAspects2() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -5604,7 +5734,8 @@ public void testTopLevelAspectRequiresAspect_aspectRequiredByMultipleAspects2() @Test public void testTopLevelAspectRequiresAspect_requireExistingAspect_passed() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -5626,7 +5757,8 @@ public void testTopLevelAspectRequiresAspect_requireExistingAspect_passed() thro @Test public void testTopLevelAspectRequiresAspect_requireExistingAspect_failed() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -5651,7 +5783,8 @@ public void testTopLevelAspectRequiresAspect_requireExistingAspect_failed() thro @Test public void testTopLevelAspectRequiresAspect_inheritDefaultValues() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -5682,7 +5815,8 @@ public void testTopLevelAspectRequiresAspect_inheritDefaultValues() throws Excep @Test public void testTopLevelAspectRequiresAspect_inheritAttrAspectsFromSingleAspect() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -5705,7 +5839,8 @@ public void testTopLevelAspectRequiresAspect_inheritAttrAspectsFromSingleAspect( @Test public void testTopLevelAspectRequiresAspect_inheritRequiredProvidersFromSingleAspect() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "cc = provider()", @@ -5730,7 +5865,8 @@ public void testTopLevelAspectRequiresAspect_inheritRequiredProvidersFromSingleA @Test public void testTopLevelAspectRequiresExistingAspect_inheritAttrAspectsFromSingleAspect() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -5755,7 +5891,8 @@ public void testTopLevelAspectRequiresExistingAspect_inheritAttrAspectsFromSingl @Test public void testTopLevelAspectRequiresExistingAspect_inheritRequiredProvidersFromSingleAspect() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "cc = provider()", @@ -5782,7 +5919,8 @@ public void testTopLevelAspectRequiresExistingAspect_inheritRequiredProvidersFro @Test public void testTopLevelAspectRequiresAspect_inheritAttrAspectsFromMultipleAspects() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -5810,7 +5948,8 @@ public void testTopLevelAspectRequiresAspect_inheritAttrAspectsFromMultipleAspec @Test public void testTopLevelAspectRequiresAspect_inheritRequiredProvidersFromMultipleAspects() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "cc = provider()", @@ -5839,7 +5978,8 @@ public void testTopLevelAspectRequiresAspect_inheritRequiredProvidersFromMultipl @Test public void testTopLevelAspectRequiresAspect_inheritAllAttrAspects() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "cc = provider()", @@ -5868,7 +6008,8 @@ public void testTopLevelAspectRequiresAspect_inheritAllAttrAspects() throws Exce @Test public void testTopLevelAspectRequiresAspect_inheritAllRequiredProviders() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "cc = provider()", @@ -5898,7 +6039,8 @@ public void testTopLevelAspectRequiresAspect_inheritAllRequiredProviders() throw @Test public void testTopLevelAspectRequiresAspect_inheritAttrAspectsFromAspectsStack() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -5924,7 +6066,8 @@ public void testTopLevelAspectRequiresAspect_inheritAttrAspectsFromAspectsStack( @Test public void testTopLevelAspectRequiresAspect_inheritRequiredProvidersFromAspectsStack() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "cc = provider()", @@ -5952,7 +6095,8 @@ public void testTopLevelAspectRequiresAspect_inheritRequiredProvidersFromAspects @Test public void testTopLevelAspectRequiringAspect_inheritAllAttrAspectsFromAspectsStack() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -5979,7 +6123,8 @@ public void testTopLevelAspectRequiringAspect_inheritAllAttrAspectsFromAspectsSt @Test public void testTopLevelAspectRequiresAspect_inheritAllRequiredProvidersFromAspectsStack() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "cc = provider()", @@ -6008,7 +6153,8 @@ public void testTopLevelAspectRequiresAspect_inheritAllRequiredProvidersFromAspe public void testTopLevelAspectRequiresAspect_requiredNativeAspect_inheritsAttrAspects() throws Exception { exposeNativeAspectToStarlark(); - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -6031,7 +6177,8 @@ public void testTopLevelAspectRequiresAspect_requiredNativeAspect_inheritsAttrAs public void testTopLevelAspectRequiresAspect_requiredNativeAspect_inheritsRequiredProviders() throws Exception { exposeNativeAspectToStarlark(); - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "rule_prov = provider()", @@ -6056,7 +6203,8 @@ public void testTopLevelAspectRequiresAspect_requiredNativeAspect_inheritsRequir public void testTopLevelAspectRequiresAspect_requiredNativeAspect_parametersNotAllowed() throws Exception { exposeNativeAspectToStarlark(); - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -6078,7 +6226,8 @@ public void testTopLevelAspectRequiresAspect_requiredNativeAspect_parametersNotA @Test public void testTopLevelAspectRequiresAspect_requiredStarlarkAspect_parametersNotAllowed() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "def _impl(target, ctx):", @@ -6100,7 +6249,8 @@ public void testTopLevelAspectRequiresAspect_requiredStarlarkAspect_parametersNo @Test public void testTopLevelAspectRequiresAspect_ruleAttributes() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "RequiredAspectProv = provider()", @@ -6184,7 +6334,8 @@ 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("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "BaseAspectProv = provider()", @@ -6288,7 +6439,8 @@ 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("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "Prov_A = provider()", @@ -6412,7 +6564,8 @@ public void testTopLevelAspectRequiresAspect_inheritRequiredProviders() throws E @Test public void testTopLevelAspectRequiresAspect_inspectRequiredAspectActions() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "BaseAspectProvider = provider()", @@ -6469,7 +6622,8 @@ public void testTopLevelAspectRequiresAspect_inspectRequiredAspectActions() thro @Test public void testTopLevelAspectRequiresAspect_inspectRequiredAspectGeneratedFiles() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "BaseAspectProvider = provider()", @@ -6520,7 +6674,8 @@ public void testTopLevelAspectRequiresAspect_inspectRequiredAspectGeneratedFiles @Test public void testTopLevelAspectRequiresAspect_withRequiredAspectProvidersSatisfied() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "prov_a = provider()", @@ -6611,7 +6766,8 @@ public void testTopLevelAspectRequiresAspect_withRequiredAspectProvidersSatisfie @Test public void testTopLevelAspectRequiresAspect_withRequiredAspectProvidersNotFound() throws Exception { - useConfiguration("--experimental_required_aspects"); + useConfiguration( + "--experimental_required_aspects", "--incompatible_top_level_aspects_dependency"); scratch.file( "test/defs.bzl", "prov_a = provider()",