From a3f5f576cd35798140ba3e81d03d919dd4ecb847 Mon Sep 17 00:00:00 2001 From: Dmitry Lomov Date: Fri, 11 Nov 2016 12:17:25 +0000 Subject: output_group is not a real Skylark provider for aspects, as well as for rules. -- MOS_MIGRATED_REVID=138863855 --- .../rules/SkylarkRuleConfiguredTargetBuilder.java | 66 ++++++++++-------- .../build/lib/skyframe/SkylarkAspectFactory.java | 18 ++--- .../build/lib/skylark/SkylarkAspectsTest.java | 79 +++++++++++++++++++++- 3 files changed, 123 insertions(+), 40 deletions(-) (limited to 'src') diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java index 94c507787d..84f6a5f359 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java @@ -186,41 +186,47 @@ public final class SkylarkRuleConfiguredTargetBuilder { for (String outputGroup : outputGroups.keySet()) { SkylarkValue objects = outputGroups.get(outputGroup); - NestedSet artifacts; + NestedSet artifacts = convertToOutputGroupValue(loc, outputGroup, objects); + builder.addOutputGroup(outputGroup, artifacts); + } + } - String typeErrorMessage = - "Output group '%s' is of unexpected type. " - + "Should be list or set of Files, but got '%s' instead."; + public static NestedSet convertToOutputGroupValue(Location loc, String outputGroup, + SkylarkValue objects) throws EvalException { + NestedSet artifacts; - if (objects instanceof SkylarkList) { - NestedSetBuilder nestedSetBuilder = NestedSetBuilder.stableOrder(); - for (Object o : (SkylarkList) objects) { - if (o instanceof Artifact) { - nestedSetBuilder.add((Artifact) o); - } else { - throw new EvalException( - loc, - String.format( - typeErrorMessage, - outputGroup, - "list with an element of " + EvalUtils.getDataTypeNameFromClass(o.getClass()))); - } + String typeErrorMessage = + "Output group '%s' is of unexpected type. " + + "Should be list or set of Files, but got '%s' instead."; + + if (objects instanceof SkylarkList) { + NestedSetBuilder nestedSetBuilder = NestedSetBuilder.stableOrder(); + for (Object o : (SkylarkList) objects) { + if (o instanceof Artifact) { + nestedSetBuilder.add((Artifact) o); + } else { + throw new EvalException( + loc, + String.format( + typeErrorMessage, + outputGroup, + "list with an element of " + EvalUtils.getDataTypeNameFromClass(o.getClass()))); } - artifacts = nestedSetBuilder.build(); - } else { - artifacts = - SkylarkType.cast( - objects, - SkylarkNestedSet.class, - Artifact.class, - loc, - typeErrorMessage, - outputGroup, - EvalUtils.getDataTypeName(objects, true)) - .getSet(Artifact.class); } - builder.addOutputGroup(outputGroup, artifacts); + artifacts = nestedSetBuilder.build(); + } else { + artifacts = + SkylarkType.cast( + objects, + SkylarkNestedSet.class, + Artifact.class, + loc, + typeErrorMessage, + outputGroup, + EvalUtils.getDataTypeName(objects, true)) + .getSet(Artifact.class); } + return artifacts; } private static ConfiguredTarget addStructFieldsAndBuild( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java index b5b8087726..eb3922ad1a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredAspect; import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory; import com.google.devtools.build.lib.analysis.ConfiguredTarget; @@ -25,12 +24,13 @@ import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.SkylarkAspect; import com.google.devtools.build.lib.packages.SkylarkClassObject; +import com.google.devtools.build.lib.rules.SkylarkRuleConfiguredTargetBuilder; import com.google.devtools.build.lib.rules.SkylarkRuleContext; +import com.google.devtools.build.lib.skylarkinterface.SkylarkValue; import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.EvalExceptionWithStackTrace; import com.google.devtools.build.lib.syntax.Mutability; -import com.google.devtools.build.lib.syntax.SkylarkNestedSet; import com.google.devtools.build.lib.syntax.SkylarkType; import java.util.Map; @@ -90,8 +90,9 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory { for (String key : struct.getKeys()) { if (key.equals("output_groups")) { addOutputGroups(struct.getValue(key), loc, builder); + } else { + builder.addSkylarkTransitiveInfo(key, struct.getValue(key), loc); } - builder.addSkylarkTransitiveInfo(key, struct.getValue(key), loc); } ConfiguredAspect configuredAspect = builder.build(); SkylarkProviderValidationUtil.checkOrphanArtifacts(ruleContext); @@ -101,21 +102,20 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory { ruleContext.ruleError("\n" + e.print()); return null; } - } } private static void addOutputGroups(Object value, Location loc, ConfiguredAspect.Builder builder) throws EvalException { - Map outputGroups = SkylarkType - .castMap(value, String.class, SkylarkNestedSet.class, "output_groups"); + Map outputGroups = + SkylarkType.castMap(value, String.class, SkylarkValue.class, "output_groups"); for (String outputGroup : outputGroups.keySet()) { - SkylarkNestedSet objects = outputGroups.get(outputGroup); + SkylarkValue objects = outputGroups.get(outputGroup); + builder.addOutputGroup(outputGroup, - SkylarkType.cast(objects, SkylarkNestedSet.class, Artifact.class, loc, - "Output group '%s'", outputGroup).getSet(Artifact.class)); + SkylarkRuleConfiguredTargetBuilder.convertToOutputGroupValue(loc, outputGroup, objects)); } } diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java index 0188b1e4c7..cebf63dccc 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java @@ -258,7 +258,6 @@ public class SkylarkAspectsTest extends AnalysisTestCase { "", "MyAspect = aspect(", " implementation=_impl,", - " attr_aspects=['deps'],", ")"); scratch.file( "test/BUILD", @@ -292,6 +291,50 @@ public class SkylarkAspectsTest extends AnalysisTestCase { assertThat(names).containsExactlyElementsIn(expectedSet); } + @Test + public void testAspectWithOutputGroupsAsList() throws Exception { + scratch.file( + "test/aspect.bzl", + "def _impl(target, ctx):", + " g = target.output_group('_hidden_top_level" + INTERNAL_SUFFIX + "')", + " return struct(output_groups = { 'my_result' : [ f for f in g] })", + "", + "MyAspect = aspect(", + " implementation=_impl,", + ")"); + scratch.file( + "test/BUILD", + "java_library(", + " name = 'xxx',", + " srcs = ['A.java'],", + ")"); + + AnalysisResult analysisResult = + update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:xxx"); + assertThat( + transform( + analysisResult.getTargetsToBuild(), + new Function() { + @Nullable + @Override + public String apply(ConfiguredTarget configuredTarget) { + return configuredTarget.getLabel().toString(); + } + })) + .containsExactly("//test:xxx"); + AspectValue aspectValue = analysisResult.getAspects().iterator().next(); + OutputGroupProvider outputGroupProvider = + aspectValue.getConfiguredAspect().getProvider(OutputGroupProvider.class); + assertThat(outputGroupProvider).isNotNull(); + NestedSet names = outputGroupProvider.getOutputGroup("my_result"); + assertThat(names).isNotEmpty(); + NestedSet expectedSet = getConfiguredTarget("//test:xxx") + .getProvider(OutputGroupProvider.class) + .getOutputGroup(OutputGroupProvider.HIDDEN_TOP_LEVEL); + assertThat(names).containsExactlyElementsIn(expectedSet); + } + + @Test public void testAspectsFromSkylarkRules() throws Exception { scratch.file( @@ -598,6 +641,40 @@ public class SkylarkAspectsTest extends AnalysisTestCase { assertContainsEvent("ERROR /workspace/test/BUILD:3:1: Output group duplicate provided twice"); } + @Test + public void duplicateGroupsFormAspects() throws Exception { + scratch.file( + "test/aspect.bzl", + "def _a1_impl(target, ctx):", + " f = ctx.new_file('f.txt')", + " ctx.file_action(f, 'f')", + " return struct(output_groups = { 'a1_group' : set([f]) })", + "", + "a1 = aspect(implementation=_a1_impl, attr_aspects = ['dep'])", + "def _rule_impl(ctx):", + " pass", + "my_rule1 = rule(_rule_impl, attrs = { 'dep' : attr.label(aspects = [a1]) })", + "def _a2_impl(target, ctx):", + " f = ctx.new_file('f.txt')", + " ctx.file_action(f, 'f')", + " return struct(output_groups = { 'a2_group' : set([f]) })", + "", + "a2 = aspect(implementation=_a2_impl, attr_aspects = ['dep'])", + "my_rule2 = rule(_rule_impl, attrs = { 'dep' : attr.label(aspects = [a2]) })" + ); + scratch.file( + "test/BUILD", + "load(':aspect.bzl', 'my_rule1', 'my_rule2')", + "my_rule1(name = 'base')", + "my_rule1(name = 'xxx', dep = ':base')", + "my_rule2(name = 'yyy', dep = ':xxx')" + ); + + // no error. + update("//test:yyy"); + } + + @Test public void duplicateSkylarkProviders() throws Exception { scratch.file( -- cgit v1.2.3