Implement custom executable API. Design is https://github.com/bazelbuild/bazel/issues/3826, specifically https://github.com/bazelbuild/bazel/issues/3826#issuecomment-336220897. Fixes #3826. RELNOTES: ctx.outputs.executable is deprecated. Use DefaultInfo(executable = ...) instead. PiperOrigin-RevId: 173132066
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 2ed3cc3..b47fb3b 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
@@ -222,6 +222,7 @@ "A provider that is provided by every rule, even if it is not returned explicitly. " + "A <code>DefaultInfo</code> accepts the following parameters:" + "<ul>" + + "<li><code>executable</code></li>" + "<li><code>files</code></li>" + "<li><code>runfiles</code></li>" + "<li><code>data_runfiles</code></li>" @@ -515,7 +516,7 @@ .value(true) .nonconfigurable("Called from RunCommand.isExecutable, which takes a Target") .build()); - builder.setOutputsDefaultExecutable(); + builder.setExecutableSkylark(); } if (implicitOutputs != Runtime.NONE) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java index a045018..deefbcb 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java
@@ -108,14 +108,14 @@ ruleContext.ruleError("Expected failure not found: " + expectFailure); return null; } - ConfiguredTarget configuredTarget = createTarget(ruleContext, target); + ConfiguredTarget configuredTarget = createTarget(skylarkRuleContext, target); SkylarkProviderValidationUtil.checkOrphanArtifacts(ruleContext); return configuredTarget; } catch (EvalException e) { addRuleToStackTrace(e, ruleContext.getRule(), ruleImplementation); // If the error was expected, return an empty target. if (!expectFailure.isEmpty() && getMessageWithoutStackTrace(e).matches(expectFailure)) { - return new com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder(ruleContext) + return new RuleConfiguredTargetBuilder(ruleContext) .add(RunfilesProvider.class, RunfilesProvider.EMPTY) .build(); } @@ -151,14 +151,19 @@ return ex.getMessage(); } - private static ConfiguredTarget createTarget(RuleContext ruleContext, Object target) + private static ConfiguredTarget createTarget(SkylarkRuleContext context, Object target) throws EvalException { - RuleConfiguredTargetBuilder builder = new RuleConfiguredTargetBuilder(ruleContext); + RuleConfiguredTargetBuilder builder = new RuleConfiguredTargetBuilder( + context.getRuleContext()); // Set the default files to build. Location loc = - ruleContext.getRule().getRuleClassObject().getConfiguredTargetFunction().getLocation(); - addProviders(ruleContext, builder, target, loc); + context.getRuleContext() + .getRule() + .getRuleClassObject() + .getConfiguredTargetFunction() + .getLocation(); + addProviders(context, builder, target, loc); try { return builder.build(); @@ -260,7 +265,7 @@ } private static void addProviders( - RuleContext ruleContext, RuleConfiguredTargetBuilder builder, Object target, Location loc) + SkylarkRuleContext context, RuleConfiguredTargetBuilder builder, Object target, Location loc) throws EvalException { Info oldStyleProviders = NativeProvider.STRUCT.create(loc); @@ -312,7 +317,7 @@ .getProvider() .getKey() .equals(DefaultInfo.PROVIDER.getKey())) { - parseDefaultProviderKeys(declaredProvider, ruleContext, builder); + parseDefaultProviderKeys(declaredProvider, context, builder); defaultProviderProvidedExplicitly = true; } else { Location creationLoc = declaredProvider.getCreationLocOrNull(); @@ -322,7 +327,7 @@ } if (!defaultProviderProvidedExplicitly) { - parseDefaultProviderKeys(oldStyleProviders, ruleContext, builder); + parseDefaultProviderKeys(oldStyleProviders, context, builder); } for (String key : oldStyleProviders.getKeys()) { @@ -341,7 +346,7 @@ addOutputGroups(oldStyleProviders.getValue(key), loc, builder); } else if (key.equals("instrumented_files")) { Info insStruct = cast("instrumented_files", oldStyleProviders, Info.class, loc); - addInstrumentedFiles(insStruct, ruleContext, builder); + addInstrumentedFiles(insStruct, context.getRuleContext(), builder); } else if (isNativeDeclaredProviderWithLegacySkylarkName(oldStyleProviders.getValue(key))) { builder.addNativeDeclaredProvider((Info) oldStyleProviders.getValue(key)); } else if (!key.equals("providers")) { @@ -363,18 +368,13 @@ * throws an {@link EvalException} if there are unknown keys. */ private static void parseDefaultProviderKeys( - Info provider, RuleContext ruleContext, RuleConfiguredTargetBuilder builder) + Info provider, SkylarkRuleContext context, RuleConfiguredTargetBuilder builder) throws EvalException { SkylarkNestedSet files = null; Runfiles statelessRunfiles = null; Runfiles dataRunfiles = null; Runfiles defaultRunfiles = null; - Artifact executable = - ruleContext.getRule().getRuleClassObject().outputsDefaultExecutable() - // This doesn't actually create a new Artifact just returns the one - // created in SkylarkRuleContext. - ? ruleContext.createOutputArtifact() - : null; + Artifact executable = null; Location loc = provider.getCreationLoc(); @@ -397,9 +397,37 @@ throw new EvalException(loc, "Invalid key for default provider: " + key); } } + + if (executable != null && context.isExecutable() && context.isDefaultExecutableCreated()) { + Artifact defaultExecutable = context.getRuleContext().createOutputArtifact(); + if (!executable.equals(defaultExecutable)) { + throw new EvalException(loc, + String.format( + "The rule '%s' both accesses 'ctx.outputs.executable' and provides " + + "a different executable '%s'. Do not use 'ctx.output.executable'.", + context.getRuleContext().getRule().getRuleClass(), + executable.getRootRelativePathString()) + ); + } + } + + if (executable == null && context.isExecutable()) { + if (context.isDefaultExecutableCreated()) { + // This doesn't actually create a new Artifact just returns the one + // created in SkylarkRuleContext. + executable = context.getRuleContext().createOutputArtifact(); + } else { + throw new EvalException(loc, + String.format("The rule '%s' is executable. " + + "It needs to create an executable File and pass it as the 'executable' " + + "parameter to the DefaultInfo it returns.", + context.getRuleContext().getRule().getRuleClass())); + } + } + addSimpleProviders( builder, - ruleContext, + context.getRuleContext(), loc, executable, files,
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 9c7c5b7..dfccf8d 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
@@ -16,11 +16,13 @@ import com.google.common.base.Function; import com.google.common.base.Optional; +import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap.Builder; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.common.collect.Ordering; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Root; import com.google.devtools.build.lib.analysis.ActionsProvider; @@ -62,6 +64,7 @@ import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter; import com.google.devtools.build.lib.skylarkinterface.SkylarkValue; +import com.google.devtools.build.lib.syntax.ClassObject; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.FuncallExpression.FuncallException; import com.google.devtools.build.lib.syntax.Runtime; @@ -146,10 +149,9 @@ + "attr struct, but their values will be single lists with all the branches of the split " + "merged together."; public static final String OUTPUTS_DOC = - "A <code>struct</code> containing all the output files." - + " The struct is generated the following way:<br>" - + "<ul><li>If the rule is marked as <code>executable=True</code> the struct has an " - + "\"executable\" field with the rules default executable <code>file</code> value." + "A pseudo-struct containing all the pre-declared output files." + + " It is generated the following way:<br>" + + "<ul>" + "" + "<li>For every entry in the rule's <code>outputs</code> dict an attr is generated with " + "the same name and the corresponding <code>file</code> value." + "<li>For every output type attribute a struct attribute is generated with the " @@ -157,7 +159,14 @@ + "if no value is specified in the rule." + "<li>For every output list type attribute a struct attribute is generated with the " + "same name and corresponding <code>list</code> of <code>file</code>s value " - + "(an empty list if no value is specified in the rule).</ul>"; + + "(an empty list if no value is specified in the rule).</li>" + + "<li>DEPRECATED: If the rule is marked as <code>executable=True</code>, a field " + + "\"executable\" can be accessed. That will declare the rule's default executable " + + "<code>File</code> value. The recommended alternative is to declare an executable with " + + "<a href=\"actions.html#declare_file\"><code>ctx.actions.declare_file</code></a> " + + "and return it as the <code>executable</code> field of the rule's " + + "<a href=\"globals.html#DefaultInfo\"><code>DefaultInfo</code></a> provider." + + "</ul>"; public static final Function<Attribute, Object> ATTRIBUTE_VALUE_EXTRACTOR_FOR_ASPECT = new Function<Attribute, Object>() { @Nullable @@ -166,6 +175,7 @@ return attribute.getDefaultValue(null); } }; + public static final String EXECUTABLE_OUTPUT_NAME = "executable"; // This field is a copy of the info from ruleContext, stored separately so it can be accessed // after this object has been nullified. @@ -190,7 +200,7 @@ // TODO(bazel-team): we only need this because of the css_binary rule. private ImmutableMap<Artifact, Label> artifactsLabelMap; - private Info outputsObject; + private Outputs outputsObject; /** * Creates a new SkylarkRuleContext using ruleContext. @@ -213,10 +223,8 @@ if (aspectDescriptor == null) { this.isForAspect = false; Collection<Attribute> attributes = ruleContext.getRule().getAttributes(); - HashMap<String, Object> outputsBuilder = new HashMap<>(); - if (ruleContext.getRule().getRuleClassObject().outputsDefaultExecutable()) { - addOutput(outputsBuilder, "executable", ruleContext.createOutputArtifact()); - } + Outputs outputs = new Outputs(this); + ImplicitOutputsFunction implicitOutputsFunction = ruleContext.getRule().getImplicitOutputsFunction(); @@ -225,8 +233,7 @@ (SkylarkImplicitOutputsFunction) implicitOutputsFunction; for (Map.Entry<String, String> entry : func.calculateOutputs(RawAttributeMapper.of(ruleContext.getRule())).entrySet()) { - addOutput( - outputsBuilder, + outputs.addOutput( entry.getKey(), ruleContext.getImplicitOutputArtifact(entry.getValue())); } @@ -249,12 +256,12 @@ if (type == BuildType.OUTPUT) { if (artifacts.size() == 1) { - addOutput(outputsBuilder, attrName, Iterables.getOnlyElement(artifacts)); + outputs.addOutput(attrName, Iterables.getOnlyElement(artifacts)); } else { - addOutput(outputsBuilder, attrName, Runtime.NONE); + outputs.addOutput(attrName, Runtime.NONE); } } else if (type == BuildType.OUTPUT_LIST) { - addOutput(outputsBuilder, attrName, SkylarkList.createImmutable(artifacts)); + outputs.addOutput(attrName, SkylarkList.createImmutable(artifacts)); } else { throw new IllegalArgumentException( "Type of " + attrName + "(" + type + ") is not output type "); @@ -262,10 +269,7 @@ } this.artifactsLabelMap = artifactLabelMapBuilder.build(); - this.outputsObject = - NativeProvider.STRUCT.create( - outputsBuilder, - "No attribute '%s' in outputs. Make sure you declared a rule output with this name."); + this.outputsObject = outputs; this.attributesCollection = buildAttributesCollection( @@ -295,6 +299,123 @@ } /** + * Represents `ctx.outputs`. + * + * <p>A {@link ClassObject} (struct-like data structure) with "executable" field created + * lazily on-demand. + * + * <p>Note: There is only one {@code Outputs} object per rule context, so default + * (object identity) equals and hashCode suffice. + */ + private static class Outputs implements ClassObject, SkylarkValue { + private final Map<String, Object> outputs; + private final SkylarkRuleContext context; + private boolean executableCreated = false; + + public Outputs(SkylarkRuleContext context) { + this.outputs = new LinkedHashMap<>(); + this.context = context; + } + + private void addOutput(String key, Object value) + throws EvalException { + Preconditions.checkState(!context.isImmutable(), + "Cannot add outputs to immutable Outputs object"); + if (outputs.containsKey(key) + || (context.isExecutable() && EXECUTABLE_OUTPUT_NAME.equals(key))) { + throw new EvalException(null, "Multiple outputs with the same key: " + key); + } + outputs.put(key, value); + } + + + @Override + public boolean isImmutable() { + return context.isImmutable(); + } + + @Override + public ImmutableCollection<String> getKeys() throws EvalException { + checkMutable(); + ImmutableList.Builder<String> result = ImmutableList.builder(); + if (context.isExecutable() && executableCreated) { + result.add(EXECUTABLE_OUTPUT_NAME); + } + result.addAll(outputs.keySet()); + return result.build(); + } + + @Nullable + @Override + public Object getValue(String name) throws EvalException { + checkMutable(); + if (context.isExecutable() && EXECUTABLE_OUTPUT_NAME.equals(name)) { + executableCreated = true; + // createOutputArtifact() will cache the created artifact. + return context.ruleContext.createOutputArtifact(); + } + + return outputs.get(name); + } + + @Nullable + @Override + public String errorMessage(String name) { + return String.format( + "No attribute '%s' in outputs. Make sure you declared a rule output with this name.", + name); + } + + @Override + public void repr(SkylarkPrinter printer) { + if (isImmutable()) { + printer.append("ctx.outputs(for "); + printer.append(context.ruleLabelCanonicalName); + printer.append(")"); + return; + } + boolean first = true; + printer.append("ctx.outputs("); + // Sort by key to ensure deterministic output. + try { + for (String key : Ordering.natural().sortedCopy(getKeys())) { + if (!first) { + printer.append(", "); + } + first = false; + printer.append(key); + printer.append(" = "); + printer.repr(getValue(key)); + } + printer.append(")"); + } catch (EvalException e) { + throw new AssertionError("mutable ctx.outputs should not throw", e); + } + } + + private void checkMutable() throws EvalException { + if (isImmutable()) { + throw new EvalException( + null, + String.format( + "cannot access outputs of rule '%s' outside of its own " + + "rule implementation function", + context.ruleLabelCanonicalName)); + } + } + + } + + public boolean isExecutable() { + return ruleContext.getRule().getRuleClassObject().isExecutableSkylark(); + } + + public boolean isDefaultExecutableCreated() { + return this.outputsObject.executableCreated; + } + + + /** * Nullifies fields of the object when it's not supposed to be used anymore to free unused memory * and to make sure this object is not accessed when it's not supposed to (after the corresponding * rule implementation function has exited). @@ -584,14 +705,6 @@ } } - private void addOutput(HashMap<String, Object> outputsBuilder, String key, Object value) - throws EvalException { - if (outputsBuilder.containsKey(key)) { - throw new EvalException(null, "Multiple outputs with the same key: " + key); - } - outputsBuilder.put(key, value); - } - @Override public boolean isImmutable() { return ruleContext == null; @@ -787,7 +900,7 @@ } @SkylarkCallable(structField = true, doc = OUTPUTS_DOC) - public Info outputs() throws EvalException { + public ClassObject outputs() throws EvalException { checkMutable("outputs"); if (outputsObject == null) { throw new EvalException(Location.BUILTIN, "'outputs' is not defined");
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java index a031f2c..7dd8acb 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -462,7 +462,7 @@ private boolean publicByDefault = false; private boolean binaryOutput = true; private boolean workspaceOnly = false; - private boolean outputsDefaultExecutable = false; + private boolean isExecutableSkylark = false; private boolean isConfigMatcher = false; private ImplicitOutputsFunction implicitOutputsFunction = ImplicitOutputsFunction.NONE; private RuleTransitionFactory transitionFactory; @@ -582,7 +582,7 @@ publicByDefault, binaryOutput, workspaceOnly, - outputsDefaultExecutable, + isExecutableSkylark, implicitOutputsFunction, isConfigMatcher, transitionFactory, @@ -929,8 +929,8 @@ * This rule class outputs a default executable for every rule with the same name as * the rules's. Only works for Skylark. */ - public <TYPE> Builder setOutputsDefaultExecutable() { - this.outputsDefaultExecutable = true; + public <TYPE> Builder setExecutableSkylark() { + this.isExecutableSkylark = true; return this; } @@ -1042,7 +1042,7 @@ private final boolean publicByDefault; private final boolean binaryOutput; private final boolean workspaceOnly; - private final boolean outputsDefaultExecutable; + private final boolean isExecutableSkylark; private final boolean isConfigMatcher; /** @@ -1164,7 +1164,7 @@ boolean publicByDefault, boolean binaryOutput, boolean workspaceOnly, - boolean outputsDefaultExecutable, + boolean isExecutableSkylark, ImplicitOutputsFunction implicitOutputsFunction, boolean isConfigMatcher, RuleTransitionFactory transitionFactory, @@ -1207,7 +1207,7 @@ validateNoClashInPublicNames(attributes); this.attributes = ImmutableList.copyOf(attributes); this.workspaceOnly = workspaceOnly; - this.outputsDefaultExecutable = outputsDefaultExecutable; + this.isExecutableSkylark = isExecutableSkylark; this.configurationFragmentPolicy = configurationFragmentPolicy; this.supportsConstraintChecking = supportsConstraintChecking; this.requiredToolchains = ImmutableSet.copyOf(requiredToolchains); @@ -2021,8 +2021,8 @@ /** * Returns true if this rule class outputs a default executable for every rule. */ - public boolean outputsDefaultExecutable() { - return outputsDefaultExecutable; + public boolean isExecutableSkylark() { + return isExecutableSkylark; } public ImmutableSet<Label> getRequiredToolchains() {