Skip to content

Commit

Permalink
Fix missing print statements in output functions
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 177708823
  • Loading branch information
vladmos authored and Copybara-Service committed Dec 2, 2017
1 parent ff1b77e commit 076977e
Show file tree
Hide file tree
Showing 12 changed files with 104 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1211,7 +1211,9 @@ public Artifact getImplicitOutputArtifact(ImplicitOutputsFunction function)
throws InterruptedException {
Iterable<String> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,10 @@ public SkylarkRuleContext(RuleContext ruleContext,
SkylarkImplicitOutputsFunction func =
(SkylarkImplicitOutputsFunction) implicitOutputsFunction;
for (Map.Entry<String, String> 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()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, Object> attrValues = new HashMap<>();
for (String attrName : rule.getAttributeNames()) {
Attribute attr = rule.getAttributeDefinition(attrName);
Expand All @@ -1456,15 +1457,15 @@ private Object computeValue(AttributeMap rule) throws EvalException, Interrupted
}
}
}
return invokeCallback(attrValues);
return invokeCallback(eventHandler, attrValues);
}

private Object invokeCallback(Map<String, Object> attrValues)
private Object invokeCallback(EventHandler eventHandler, Map<String, Object> 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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -64,13 +65,13 @@ public abstract class ImplicitOutputsFunction {
*/
public abstract static class SkylarkImplicitOutputsFunction extends ImplicitOutputsFunction {

public abstract ImmutableMap<String, String> calculateOutputs(AttributeMap map)
throws EvalException, InterruptedException;
public abstract ImmutableMap<String, String> calculateOutputs(
EventHandler eventHandler, AttributeMap map) throws EvalException, InterruptedException;

@Override
public Iterable<String> getImplicitOutputs(AttributeMap map)
public Iterable<String> getImplicitOutputs(EventHandler eventHandler, AttributeMap map)
throws EvalException, InterruptedException {
return calculateOutputs(map).values();
return calculateOutputs(eventHandler, map).values();
}
}

Expand All @@ -90,8 +91,8 @@ public SkylarkImplicitOutputsFunctionWithCallback(
}

@Override
public ImmutableMap<String, String> calculateOutputs(AttributeMap map)
throws EvalException, InterruptedException {
public ImmutableMap<String, String> calculateOutputs(
EventHandler eventHandler, AttributeMap map) throws EvalException, InterruptedException {
Map<String, Object> attrValues = new HashMap<>();
for (String attrName : map.getAttributeNames()) {
Type<?> attrType = map.getAttributeType(attrName);
Expand All @@ -109,11 +110,17 @@ public ImmutableMap<String, String> calculateOutputs(AttributeMap map)
+ "or uses a select() (i.e. could have multiple values)");
try {
ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
for (Map.Entry<String, String> entry : castMap(callback.call(attrs),
String.class, String.class, "implicit outputs function return value").entrySet()) {
for (Map.Entry<String, String> 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<String> substitutions = fromTemplates(entry.getValue()).getImplicitOutputs(map);
Iterable<String> substitutions =
fromTemplates(entry.getValue()).getImplicitOutputs(eventHandler, map);
if (Iterables.isEmpty(substitutions)) {
throw new EvalException(
loc,
Expand Down Expand Up @@ -144,15 +151,15 @@ public SkylarkImplicitOutputsFunctionWithMap(ImmutableMap<String, String> output
}

@Override
public ImmutableMap<String, String> calculateOutputs(AttributeMap map)
throws EvalException, InterruptedException {
public ImmutableMap<String, String> calculateOutputs(
EventHandler eventHandler, AttributeMap map) throws EvalException, InterruptedException {

ImmutableMap.Builder<String, String> builder = ImmutableMap.builder();
for (Map.Entry<String, String> entry : outputMap.entrySet()) {
// Empty iff invalid placeholders present.
ImplicitOutputsFunction outputsFunction =
fromUnsafeTemplates(ImmutableList.of(entry.getValue()));
Iterable<String> substitutions = outputsFunction.getImplicitOutputs(map);
Iterable<String> substitutions = outputsFunction.getImplicitOutputs(eventHandler, map);
if (Iterables.isEmpty(substitutions)) {
throw new EvalException(
null,
Expand All @@ -173,7 +180,8 @@ public ImmutableMap<String, String> calculateOutputs(AttributeMap map)
*/
public abstract static class SafeImplicitOutputsFunction extends ImplicitOutputsFunction {
@Override
public abstract Iterable<String> getImplicitOutputs(AttributeMap map);
public abstract Iterable<String> getImplicitOutputs(
EventHandler eventHandler, AttributeMap map);
}

/**
Expand Down Expand Up @@ -203,20 +211,20 @@ public Set<String> 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<String> getImplicitOutputs(AttributeMap rule)
public abstract Iterable<String> 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<String> getImplicitOutputs(AttributeMap rule) {
return Collections.emptyList();
}
};
/** The implicit output function that returns no files. */
public static final SafeImplicitOutputsFunction NONE =
new SafeImplicitOutputsFunction() {
@Override
public Iterable<String> getImplicitOutputs(EventHandler eventHandler, AttributeMap rule) {
return Collections.emptyList();
}
};

/**
* A convenience wrapper for {@link #fromTemplates(Iterable)}.
Expand All @@ -240,7 +248,7 @@ public static SafeImplicitOutputsFunction fromTemplates(final Iterable<String> t
return new SafeImplicitOutputsFunction() {
// TODO(bazel-team): parse the templates already here
@Override
public Iterable<String> getImplicitOutputs(AttributeMap rule) {
public Iterable<String> getImplicitOutputs(EventHandler eventHandler, AttributeMap rule) {
ImmutableSet.Builder<String> result = new ImmutableSet.Builder<>();
for (String template : templates) {
List<String> substitutions = substitutePlaceholderIntoTemplate(template, rule);
Expand Down Expand Up @@ -277,7 +285,8 @@ public static ImplicitOutputsFunction fromUnsafeTemplates(Iterable<String> templ
return new ImplicitOutputsFunction() {
// TODO(bazel-team): parse the templates already here
@Override
public Iterable<String> getImplicitOutputs(AttributeMap rule) throws EvalException {
public Iterable<String> getImplicitOutputs(EventHandler eventHandler, AttributeMap rule)
throws EvalException {
ImmutableSet.Builder<String> result = new ImmutableSet.Builder<>();
for (String template : templates) {
List<String> substitutions =
Expand Down Expand Up @@ -319,13 +328,14 @@ public static SafeImplicitOutputsFunction fromFunctions(
final Iterable<SafeImplicitOutputsFunction> functions) {
return new SafeImplicitOutputsFunction() {
@Override
public Iterable<String> getImplicitOutputs(AttributeMap rule) {
public Iterable<String> getImplicitOutputs(EventHandler eventHandler, AttributeMap rule) {
Collection<String> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -339,7 +340,7 @@ public static boolean hasProguardSpecs(AttributeMap rule) {
new SafeImplicitOutputsFunction() {

@Override
public Iterable<String> getImplicitOutputs(AttributeMap rule) {
public Iterable<String> getImplicitOutputs(EventHandler eventHandler, AttributeMap rule) {
List<SafeImplicitOutputsFunction> functions = Lists.newArrayList();
functions.add(AndroidRuleClasses.ANDROID_BINARY_APK);
functions.add(AndroidRuleClasses.ANDROID_BINARY_UNSIGNED_APK);
Expand All @@ -352,14 +353,15 @@ public Iterable<String> 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<String> getImplicitOutputs(AttributeMap attributes) {
public Iterable<String> getImplicitOutputs(
EventHandler eventHandler, AttributeMap attributes) {

ImmutableList.Builder<SafeImplicitOutputsFunction> implicitOutputs =
ImmutableList.builder();
Expand All @@ -376,7 +378,8 @@ public Iterable<String> getImplicitOutputs(AttributeMap attributes) {
AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR);
}

return fromFunctions(implicitOutputs.build()).getImplicitOutputs(attributes);
return fromFunctions(implicitOutputs.build())
.getImplicitOutputs(eventHandler, attributes);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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<String> 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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 076977e

Please sign in to comment.