From 076977e019ebcb8b822e7727d292636fadcafcca Mon Sep 17 00:00:00 2001 From: vladmos Date: Sat, 2 Dec 2017 14:15:32 -0800 Subject: [PATCH] Fix missing print statements in output functions PiperOrigin-RevId: 177708823 --- .../build/lib/analysis/RuleContext.java | 4 +- .../lib/analysis/skylark/SkylarkAttr.java | 3 +- .../skylark/SkylarkRuleClassFunctions.java | 3 +- .../analysis/skylark/SkylarkRuleContext.java | 5 +- .../build/lib/packages/Attribute.java | 11 ++-- .../lib/packages/ImplicitOutputsFunction.java | 66 +++++++++++-------- .../devtools/build/lib/packages/Rule.java | 2 +- .../lib/rules/android/AndroidRuleClasses.java | 11 ++-- .../lib/syntax/SkylarkCallbackFunction.java | 18 ++--- .../lib/analysis/util/BuildViewTestCase.java | 2 +- .../lib/skylark/SkylarkIntegrationTest.java | 29 ++++++++ .../SkylarkRuleClassFunctionsTest.java | 2 +- 12 files changed, 104 insertions(+), 52 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index 3223ae83939a33..eef1771162e599 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java @@ -1211,7 +1211,9 @@ public Artifact getImplicitOutputArtifact(ImplicitOutputsFunction function) throws InterruptedException { Iterable result; try { - result = function.getImplicitOutputs(RawAttributeMapper.of(rule)); + result = + function.getImplicitOutputs( + getAnalysisEnvironment().getEventHandler(), RawAttributeMapper.of(rule)); } catch (EvalException e) { // It's ok as long as we don't use this method from Skylark. throw new IllegalStateException(e); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java index 7ba8bf61ec5c2b..1dea86cf914d89 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java @@ -189,7 +189,8 @@ private static Attribute.Builder createAttribute( if (defaultValue instanceof UserDefinedFunction) { // Computed attribute. Non label type attributes already caused a type check error. SkylarkCallbackFunction callback = - new SkylarkCallbackFunction((UserDefinedFunction) defaultValue, ast, env); + new SkylarkCallbackFunction( + (UserDefinedFunction) defaultValue, ast, env.getSemantics()); // SkylarkComputedDefaultTemplate needs to know the names of all attributes that it depends // on. However, this method does not know anything about other attributes. // We solve this problem by asking the SkylarkCallbackFunction for the parameter names used diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java index a697721ea9f5cd..baee697583a8a3 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java @@ -522,7 +522,8 @@ public BaseFunction invoke( if (implicitOutputs != Runtime.NONE) { if (implicitOutputs instanceof BaseFunction) { BaseFunction func = (BaseFunction) implicitOutputs; - SkylarkCallbackFunction callback = new SkylarkCallbackFunction(func, ast, funcallEnv); + SkylarkCallbackFunction callback = + new SkylarkCallbackFunction(func, ast, funcallEnv.getSemantics()); builder.setImplicitOutputsFunction( new SkylarkImplicitOutputsFunctionWithCallback(callback, ast.getLocation())); } else { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java index 7ed1211bef448a..a979f2bbd8a4be 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java @@ -232,7 +232,10 @@ public SkylarkRuleContext(RuleContext ruleContext, SkylarkImplicitOutputsFunction func = (SkylarkImplicitOutputsFunction) implicitOutputsFunction; for (Map.Entry entry : - func.calculateOutputs(RawAttributeMapper.of(ruleContext.getRule())).entrySet()) { + func.calculateOutputs( + ruleContext.getAnalysisEnvironment().getEventHandler(), + RawAttributeMapper.of(ruleContext.getRule())) + .entrySet()) { outputs.addOutput( entry.getKey(), ruleContext.getImplicitOutputArtifact(entry.getValue())); diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java index 03f56dc23aa474..6d96a9220bfb84 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java @@ -1410,7 +1410,7 @@ SkylarkComputedDefault computePossibleValues( @Override public Object compute(AttributeMap map) throws InterruptedException { try { - return owner.computeValue(map); + return owner.computeValue(eventHandler, map); } catch (EvalException ex) { caughtEvalExceptionIfAny.compareAndSet(null, ex); return null; @@ -1445,7 +1445,8 @@ public Object compute(AttributeMap map) throws InterruptedException { return new SkylarkComputedDefault(dependencies, dependencyTypesBuilder.build(), lookupTable); } - private Object computeValue(AttributeMap rule) throws EvalException, InterruptedException { + private Object computeValue(EventHandler eventHandler, AttributeMap rule) + throws EvalException, InterruptedException { Map attrValues = new HashMap<>(); for (String attrName : rule.getAttributeNames()) { Attribute attr = rule.getAttributeDefinition(attrName); @@ -1456,15 +1457,15 @@ private Object computeValue(AttributeMap rule) throws EvalException, Interrupted } } } - return invokeCallback(attrValues); + return invokeCallback(eventHandler, attrValues); } - private Object invokeCallback(Map attrValues) + private Object invokeCallback(EventHandler eventHandler, Map attrValues) throws EvalException, InterruptedException { ClassObject attrs = NativeProvider.STRUCT.create( attrValues, "No such regular (non computed) attribute '%s'."); - Object result = callback.call(attrs); + Object result = callback.call(eventHandler, attrs); try { return type.cast((result == Runtime.NONE) ? type.getDefaultValue() : result); } catch (ClassCastException ex) { diff --git a/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java b/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java index e8e91027fa9e8f..5fa3a0cd01bfb2 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java +++ b/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java @@ -27,6 +27,7 @@ import com.google.common.escape.Escaper; import com.google.common.escape.Escapers; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.syntax.ClassObject; import com.google.devtools.build.lib.syntax.EvalException; @@ -64,13 +65,13 @@ public abstract class ImplicitOutputsFunction { */ public abstract static class SkylarkImplicitOutputsFunction extends ImplicitOutputsFunction { - public abstract ImmutableMap calculateOutputs(AttributeMap map) - throws EvalException, InterruptedException; + public abstract ImmutableMap calculateOutputs( + EventHandler eventHandler, AttributeMap map) throws EvalException, InterruptedException; @Override - public Iterable getImplicitOutputs(AttributeMap map) + public Iterable getImplicitOutputs(EventHandler eventHandler, AttributeMap map) throws EvalException, InterruptedException { - return calculateOutputs(map).values(); + return calculateOutputs(eventHandler, map).values(); } } @@ -90,8 +91,8 @@ public SkylarkImplicitOutputsFunctionWithCallback( } @Override - public ImmutableMap calculateOutputs(AttributeMap map) - throws EvalException, InterruptedException { + public ImmutableMap calculateOutputs( + EventHandler eventHandler, AttributeMap map) throws EvalException, InterruptedException { Map attrValues = new HashMap<>(); for (String attrName : map.getAttributeNames()) { Type attrType = map.getAttributeType(attrName); @@ -109,11 +110,17 @@ public ImmutableMap calculateOutputs(AttributeMap map) + "or uses a select() (i.e. could have multiple values)"); try { ImmutableMap.Builder builder = ImmutableMap.builder(); - for (Map.Entry entry : castMap(callback.call(attrs), - String.class, String.class, "implicit outputs function return value").entrySet()) { + for (Map.Entry entry : + castMap( + callback.call(eventHandler, attrs), + String.class, + String.class, + "implicit outputs function return value") + .entrySet()) { // Returns empty string only in case of invalid templates - Iterable substitutions = fromTemplates(entry.getValue()).getImplicitOutputs(map); + Iterable substitutions = + fromTemplates(entry.getValue()).getImplicitOutputs(eventHandler, map); if (Iterables.isEmpty(substitutions)) { throw new EvalException( loc, @@ -144,15 +151,15 @@ public SkylarkImplicitOutputsFunctionWithMap(ImmutableMap output } @Override - public ImmutableMap calculateOutputs(AttributeMap map) - throws EvalException, InterruptedException { + public ImmutableMap calculateOutputs( + EventHandler eventHandler, AttributeMap map) throws EvalException, InterruptedException { ImmutableMap.Builder builder = ImmutableMap.builder(); for (Map.Entry entry : outputMap.entrySet()) { // Empty iff invalid placeholders present. ImplicitOutputsFunction outputsFunction = fromUnsafeTemplates(ImmutableList.of(entry.getValue())); - Iterable substitutions = outputsFunction.getImplicitOutputs(map); + Iterable substitutions = outputsFunction.getImplicitOutputs(eventHandler, map); if (Iterables.isEmpty(substitutions)) { throw new EvalException( null, @@ -173,7 +180,8 @@ public ImmutableMap calculateOutputs(AttributeMap map) */ public abstract static class SafeImplicitOutputsFunction extends ImplicitOutputsFunction { @Override - public abstract Iterable getImplicitOutputs(AttributeMap map); + public abstract Iterable getImplicitOutputs( + EventHandler eventHandler, AttributeMap map); } /** @@ -203,20 +211,20 @@ public Set get(AttributeMap rule, String attr) { private static final Escaper PERCENT_ESCAPER = Escapers.builder().addEscape('%', "%%").build(); /** - * Given a newly-constructed Rule instance (with attributes populated), - * returns the list of output files that this rule produces implicitly. + * Given a newly-constructed Rule instance (with attributes populated), returns the list of output + * files that this rule produces implicitly. */ - public abstract Iterable getImplicitOutputs(AttributeMap rule) + public abstract Iterable getImplicitOutputs(EventHandler eventHandler, AttributeMap rule) throws EvalException, InterruptedException; - /** - * The implicit output function that returns no files. - */ - public static final SafeImplicitOutputsFunction NONE = new SafeImplicitOutputsFunction() { - @Override public Iterable getImplicitOutputs(AttributeMap rule) { - return Collections.emptyList(); - } - }; + /** The implicit output function that returns no files. */ + public static final SafeImplicitOutputsFunction NONE = + new SafeImplicitOutputsFunction() { + @Override + public Iterable getImplicitOutputs(EventHandler eventHandler, AttributeMap rule) { + return Collections.emptyList(); + } + }; /** * A convenience wrapper for {@link #fromTemplates(Iterable)}. @@ -240,7 +248,7 @@ public static SafeImplicitOutputsFunction fromTemplates(final Iterable t return new SafeImplicitOutputsFunction() { // TODO(bazel-team): parse the templates already here @Override - public Iterable getImplicitOutputs(AttributeMap rule) { + public Iterable getImplicitOutputs(EventHandler eventHandler, AttributeMap rule) { ImmutableSet.Builder result = new ImmutableSet.Builder<>(); for (String template : templates) { List substitutions = substitutePlaceholderIntoTemplate(template, rule); @@ -277,7 +285,8 @@ public static ImplicitOutputsFunction fromUnsafeTemplates(Iterable templ return new ImplicitOutputsFunction() { // TODO(bazel-team): parse the templates already here @Override - public Iterable getImplicitOutputs(AttributeMap rule) throws EvalException { + public Iterable getImplicitOutputs(EventHandler eventHandler, AttributeMap rule) + throws EvalException { ImmutableSet.Builder result = new ImmutableSet.Builder<>(); for (String template : templates) { List substitutions = @@ -319,13 +328,14 @@ public static SafeImplicitOutputsFunction fromFunctions( final Iterable functions) { return new SafeImplicitOutputsFunction() { @Override - public Iterable getImplicitOutputs(AttributeMap rule) { + public Iterable getImplicitOutputs(EventHandler eventHandler, AttributeMap rule) { Collection result = new LinkedHashSet<>(); for (SafeImplicitOutputsFunction function : functions) { - Iterables.addAll(result, function.getImplicitOutputs(rule)); + Iterables.addAll(result, function.getImplicitOutputs(eventHandler, rule)); } return result; } + @Override public String toString() { return StringUtil.joinEnglishList(functions); diff --git a/src/main/java/com/google/devtools/build/lib/packages/Rule.java b/src/main/java/com/google/devtools/build/lib/packages/Rule.java index 2dcc89b4440b18..6615fe633fdffe 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Rule.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Rule.java @@ -549,7 +549,7 @@ private void populateImplicitOutputFiles( throws InterruptedException { try { RawAttributeMapper attributeMap = RawAttributeMapper.of(this); - for (String out : implicitOutputsFunction.getImplicitOutputs(attributeMap)) { + for (String out : implicitOutputsFunction.getImplicitOutputs(eventHandler, attributeMap)) { Label label; if (performChecks) { try { diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java index bc28f84e6faaae..5dcef2590ba6c5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet; import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault; @@ -339,7 +340,7 @@ public static boolean hasProguardSpecs(AttributeMap rule) { new SafeImplicitOutputsFunction() { @Override - public Iterable getImplicitOutputs(AttributeMap rule) { + public Iterable getImplicitOutputs(EventHandler eventHandler, AttributeMap rule) { List functions = Lists.newArrayList(); functions.add(AndroidRuleClasses.ANDROID_BINARY_APK); functions.add(AndroidRuleClasses.ANDROID_BINARY_UNSIGNED_APK); @@ -352,14 +353,15 @@ public Iterable getImplicitOutputs(AttributeMap rule) { functions.add(JavaSemantics.JAVA_BINARY_PROGUARD_MAP); } } - return fromFunctions(functions).getImplicitOutputs(rule); + return fromFunctions(functions).getImplicitOutputs(eventHandler, rule); } }; public static final SafeImplicitOutputsFunction ANDROID_LIBRARY_IMPLICIT_OUTPUTS = new SafeImplicitOutputsFunction() { @Override - public Iterable getImplicitOutputs(AttributeMap attributes) { + public Iterable getImplicitOutputs( + EventHandler eventHandler, AttributeMap attributes) { ImmutableList.Builder implicitOutputs = ImmutableList.builder(); @@ -376,7 +378,8 @@ public Iterable getImplicitOutputs(AttributeMap attributes) { AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR); } - return fromFunctions(implicitOutputs.build()).getImplicitOutputs(attributes); + return fromFunctions(implicitOutputs.build()) + .getImplicitOutputs(eventHandler, attributes); } }; diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallbackFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallbackFunction.java index ed79584a05506b..719c94b961130c 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallbackFunction.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallbackFunction.java @@ -15,6 +15,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList.Builder; +import com.google.devtools.build.lib.events.EventHandler; /** * A helper class for calling Skylark functions from Java. @@ -23,26 +24,27 @@ public class SkylarkCallbackFunction { private final BaseFunction callback; private final FuncallExpression ast; - private final Environment funcallEnv; + private final SkylarkSemantics skylarkSemantics; public SkylarkCallbackFunction( - BaseFunction callback, FuncallExpression ast, Environment funcallEnv) { + BaseFunction callback, FuncallExpression ast, SkylarkSemantics skylarkSemantics) { this.callback = callback; this.ast = ast; - this.funcallEnv = funcallEnv; + this.skylarkSemantics = skylarkSemantics; } public ImmutableList getParameterNames() { return callback.signature.getSignature().getNames(); } - public Object call(ClassObject ctx, Object... arguments) + public Object call(EventHandler eventHandler, ClassObject ctx, Object... arguments) throws EvalException, InterruptedException { try (Mutability mutability = Mutability.create("callback %s", callback)) { - Environment env = Environment.builder(mutability) - .setSemantics(funcallEnv.getSemantics()) - .setEventHandler(funcallEnv.getEventHandler()) - .build(); + Environment env = + Environment.builder(mutability) + .setSemantics(skylarkSemantics) + .setEventHandler(eventHandler) + .build(); return callback.call(buildArgumentList(ctx, arguments), null, ast, env); } catch (ClassCastException | IllegalArgumentException e) { throw new EvalException(ast.getLocation(), e.getMessage()); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index 571fe2345a30a5..efdd13dcd70580 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -1917,7 +1917,7 @@ protected Artifact getImplicitOutputArtifact( RawAttributeMapper attr = RawAttributeMapper.of(associatedRule); - String path = Iterables.getOnlyElement(outputFunction.getImplicitOutputs(attr)); + String path = Iterables.getOnlyElement(outputFunction.getImplicitOutputs(eventCollector, attr)); return view.getArtifactFactory() .getDerivedArtifact( diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java index 8b2770a90aa4df..8039563a44d4d0 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java @@ -914,6 +914,35 @@ public void testRuleClassImplicitOutputFunctionAndDefaultValue() throws Exceptio .containsExactly("bar.txt"); } + @Test + public void testRuleClassImplicitOutputFunctionPrints() throws Exception { + scratch.file( + "test/skylark/extension.bzl", + "def custom_rule_impl(ctx):", + " print('implementation', ctx.label)", + " files = [ctx.outputs.o]", + " ctx.actions.run_shell(", + " outputs = files,", + " command = 'echo')", + "", + "def output_func(name):", + " print('output function', name)", + " return {'o': name + '.txt'}", + "", + "custom_rule = rule(implementation = custom_rule_impl,", + " outputs = output_func)"); + + scratch.file( + "test/skylark/BUILD", + "load('/test/skylark/extension', 'custom_rule')", + "", + "custom_rule(name = 'cr')"); + + getConfiguredTarget("//test/skylark:cr"); + assertContainsEvent("output function cr"); + assertContainsEvent("implementation //test/skylark:cr"); + } + @Test public void testRuleClassNonMandatoryEmptyOutputs() throws Exception { scratch.file( diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java index 03713158302041..d9475622067f7a 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java @@ -701,7 +701,7 @@ public void testRuleOutputs() throws Exception { "r1 = rule(impl, outputs = {'a': 'a.txt'})"); RuleClass c = ((SkylarkRuleFunction) lookup("r1")).getRuleClass(); ImplicitOutputsFunction function = c.getDefaultImplicitOutputsFunction(); - assertThat(function.getImplicitOutputs(null)).containsExactly("a.txt"); + assertThat(function.getImplicitOutputs(ev.getEventHandler(), null)).containsExactly("a.txt"); } @Test