Ignore base target toolchain dependencies if aspect is applied on output file
when aspect is applied to an output file it does not propagate to its generating rule attribute dependencies and it should not propagate to its toolchain dependencies. The type of the target was mistakenly checked using `getAssociatedRule` which is not null if the target is `OutputFile`.
PiperOrigin-RevId: 684923928
Change-Id: Idb0ba4dec8b646f2f8aa09002ea75817eb4d6f81
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index 867f63f..f117698 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -613,8 +613,7 @@
*/
private static boolean canAspectsPropagateToToolchains(
ImmutableList<Aspect> topologicalAspectPath, Target baseTarget) {
- Rule rule = baseTarget.getAssociatedRule();
- if (rule == null) {
+ if (!baseTarget.isRule()) {
return false;
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
index 50c8d83..382921d 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
@@ -484,6 +484,62 @@
}
}
+ /**
+ * An aspect that applies to output files and propagates to toolchain dependencies and attribute
+ * dependencies.
+ */
+ public static class DepsVisitingFileAspect extends BaseAspect {
+
+ /** Test provider which includes the base target label. */
+ @SerializationConstant
+ public static final StarlarkProvider PROVIDER =
+ StarlarkProvider.builder(Location.BUILTIN)
+ .buildExported(new StarlarkProvider.Key(keyForBuild(FAKE_LABEL), "AspectProvider"));
+
+ private String depAttr;
+ private Label toolChainType;
+
+ public DepsVisitingFileAspect(String depAttr, String toolChainType) {
+ this.depAttr = depAttr;
+ this.toolChainType = Label.parseCanonicalUnchecked(toolChainType);
+ }
+
+ @Override
+ public ConfiguredAspect create(
+ Label targetLabel,
+ ConfiguredTarget ct,
+ RuleContext ruleContext,
+ AspectParameters parameters,
+ RepositoryName toolsRepository)
+ throws ActionConflictException, InterruptedException {
+ try {
+ return ConfiguredAspect.builder(ruleContext)
+ .addStarlarkDeclaredProvider(
+ StarlarkInfo.create(
+ PROVIDER, ImmutableMap.of("val", ct.getLabel().getCanonicalForm()), null))
+ .build();
+ } catch (EvalException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
+ @Override
+ public String getName() {
+ return String.format("%s_%s_%s", super.getName(), depAttr, toolChainType.getCanonicalForm());
+ }
+
+ @Override
+ public AspectDefinition getDefinition(AspectParameters aspectParameters) {
+ AspectDefinition.Builder aspectDefinition =
+ new AspectDefinition.Builder(this)
+ .applyToFiles(true)
+ .propagateAlongAttribute(depAttr)
+ .propagateToToolchainsTypes(ImmutableSet.of(toolChainType));
+
+ return aspectDefinition.build();
+ }
+ }
+
/** An aspect that defines its own implicit attribute, requiring PackageSpecificationProvider. */
public static class PackageGroupAttributeAspect extends BaseAspect {
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkAspectsToolchainPropagationTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkAspectsToolchainPropagationTest.java
index c6d9dee..61591dd 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkAspectsToolchainPropagationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkAspectsToolchainPropagationTest.java
@@ -24,6 +24,7 @@
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.analysis.ConfiguredAspect;
import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
+import com.google.devtools.build.lib.analysis.util.TestAspects.DepsVisitingFileAspect;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.StarlarkInfo;
import com.google.devtools.build.lib.packages.StarlarkProvider;
@@ -1457,7 +1458,7 @@
""");
useConfiguration("--extra_toolchains=//toolchain:foo_toolchain_with_dep");
- var analysisResult = update(ImmutableList.of("//test:defs.bzl%toolchain_aspect"), "//test:t1");
+ var analysisResult = update("//test:t1");
var configuredTarget = Iterables.getOnlyElement(analysisResult.getTargetsToBuild());
@@ -1473,6 +1474,181 @@
"toolchain_aspect on @@//toolchain:toolchain_dep");
}
+ @Test
+ public void toolchainAspectOnOutputFile_notPropagatedToDeps() throws Exception {
+ scratch.file(
+ "test/defs.bzl",
+ """
+ AspectProvider = provider()
+ def _impl(target, ctx):
+ return [AspectProvider(val="hi")]
+
+ toolchain_aspect = aspect(
+ implementation = _impl,
+ toolchains_aspects = ['//rule:toolchain_type_1'],
+ attr_aspects = ['dep'],
+ )
+
+ def _rule_1_impl(ctx):
+ if ctx.outputs.out:
+ ctx.actions.write(ctx.outputs.out, 'hi')
+ return []
+
+ r1 = rule(
+ implementation = _rule_1_impl,
+ attrs = {
+ "out": attr.output(),
+ "dep": attr.label(),
+ },
+ toolchains = ['//rule:toolchain_type_1'],
+ )
+ """);
+ scratch.file(
+ "test/BUILD",
+ """
+ load('//test:defs.bzl', 'r1')
+ r1(name = 't1', out = 'my_out.txt', dep = ':t2')
+ r1(name = 't2')
+ """);
+ useConfiguration("--extra_toolchains=//toolchain:foo_toolchain");
+
+ var unused = update(ImmutableList.of("//test:defs.bzl%toolchain_aspect"), "//test:my_out.txt");
+
+ // {@link AspectKey} is created for toolchain_aspect on the output file //test:my_out.txt but
+ // the aspect is not applied (no returned providers) because the aspect cannot be applied to
+ // output files. The aspect does not propagate to any of the generating rule dependencies.
+ var nodes =
+ skyframeExecutor.getEvaluator().getDoneValues().entrySet().stream()
+ .filter(
+ entry ->
+ entry.getKey() instanceof AspectKey
+ && ((AspectKey) entry.getKey())
+ .getAspectClass()
+ .getName()
+ .equals("//test:defs.bzl%toolchain_aspect"))
+ .collect(toImmutableList());
+ assertThat(nodes).hasSize(1);
+
+ AspectKey aspectKey = (AspectKey) Iterables.getOnlyElement(nodes).getKey();
+ assertThat(aspectKey.getLabel().toString()).isEqualTo("//test:my_out.txt");
+
+ ConfiguredAspect aspectValue = (ConfiguredAspect) Iterables.getOnlyElement(nodes).getValue();
+ assertThat(aspectValue.getProviders().getProviderCount()).isEqualTo(0);
+ }
+
+ @Test
+ public void toolchainAspectApplyToGeneratingRule_propagateToDeps() throws Exception {
+ scratch.file(
+ "test/defs.bzl",
+ """
+ def _impl(target, ctx):
+ return []
+
+ toolchain_aspect = aspect(
+ implementation = _impl,
+ toolchains_aspects = ['//rule:toolchain_type_1'],
+ attr_aspects = ['dep'],
+ apply_to_generating_rules = True,
+ )
+
+ def _rule_1_impl(ctx):
+ if ctx.outputs.out:
+ ctx.actions.write(ctx.outputs.out, 'hi')
+ return []
+
+ r1 = rule(
+ implementation = _rule_1_impl,
+ attrs = {
+ "out": attr.output(),
+ "dep": attr.label(),
+ },
+ toolchains = ['//rule:toolchain_type_1'],
+ )
+ """);
+ scratch.file(
+ "test/BUILD",
+ """
+ load('//test:defs.bzl', 'r1')
+ r1(name = 't1', out = 'my_out.txt', dep = ':t2')
+ r1(name = 't2')
+ """);
+ useConfiguration("--extra_toolchains=//toolchain:foo_toolchain");
+
+ var unused = update(ImmutableList.of("//test:defs.bzl%toolchain_aspect"), "//test:my_out.txt");
+
+ var visitedTargets =
+ skyframeExecutor.getEvaluator().getDoneValues().entrySet().stream()
+ .filter(
+ entry ->
+ entry.getKey() instanceof AspectKey
+ && ((AspectKey) entry.getKey())
+ .getAspectClass()
+ .getName()
+ .equals("//test:defs.bzl%toolchain_aspect"))
+ .map(e -> ((AspectKey) e.getKey()).getLabel().toString())
+ .collect(toImmutableList());
+
+ // toolchain_aspect is applied to the generating rule of the output file and propagated to its
+ // attribute dependency and toolchain dependency.
+ assertThat(visitedTargets)
+ .containsExactly("//test:my_out.txt", "//test:t1", "//test:t2", "//toolchain:foo");
+ }
+
+ @Test
+ public void toolchainAspectApplyToFiles_notPropagatedToDeps() throws Exception {
+ DepsVisitingFileAspect aspect = new DepsVisitingFileAspect("dep", "//rule:toolchain_type_1");
+ setRulesAndAspectsAvailableInTests(ImmutableList.of(aspect), ImmutableList.of());
+ scratch.file(
+ "test/defs.bzl",
+ """
+ def _rule_1_impl(ctx):
+ if ctx.outputs.out:
+ ctx.actions.write(ctx.outputs.out, 'hi')
+ return []
+
+ r1 = rule(
+ implementation = _rule_1_impl,
+ attrs = {
+ "out": attr.output(),
+ "dep": attr.label(),
+ },
+ toolchains = ['//rule:toolchain_type_1'],
+ )
+ """);
+ scratch.file(
+ "test/BUILD",
+ """
+ load('//test:defs.bzl', 'r1')
+ r1(name = 't1', out = 'my_out.txt', dep = ':t2')
+ r1(name = 't2')
+ """);
+ useConfiguration("--extra_toolchains=//toolchain:foo_toolchain");
+
+ var unused = update(ImmutableList.of(aspect.getName()), "//test:my_out.txt");
+
+ // {@link DepsVisitingFileAspect} is only applied to //test:my_out.txt file therefore it does
+ // not propagate to the dependencies of its generating rule.
+ var nodes =
+ skyframeExecutor.getEvaluator().getDoneValues().entrySet().stream()
+ .filter(
+ entry ->
+ entry.getKey() instanceof AspectKey
+ && ((AspectKey) entry.getKey())
+ .getAspectClass()
+ .getName()
+ .equals(aspect.getName()))
+ .collect(toImmutableList());
+ assertThat(nodes).hasSize(1);
+
+ AspectKey aspectKey = (AspectKey) Iterables.getOnlyElement(nodes).getKey();
+ assertThat(aspectKey.getLabel().toString()).isEqualTo("//test:my_out.txt");
+
+ ConfiguredAspect aspectValue = (ConfiguredAspect) Iterables.getOnlyElement(nodes).getValue();
+ StarlarkInfo provider =
+ (StarlarkInfo) aspectValue.get(DepsVisitingFileAspect.PROVIDER.getKey());
+ assertThat(provider.getValue("val")).isEqualTo("//test:my_out.txt");
+ }
+
private ImmutableList<AspectKey> getAspectKeys(String targetLabel, String aspectLabel) {
return skyframeExecutor.getEvaluator().getDoneValues().entrySet().stream()
.filter(
