Rollback of commit 0218320f1316b4d25e35abaaa80206237468824d.
*** Reason for rollback ***
Breaks skylark macros that call skylark rules that declare an 'args'
attribute.
--
MOS_MIGRATED_REVID=101118137
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 d051892..67cc935 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
@@ -751,19 +751,9 @@
/**
* Adds or overrides the attribute in the rule class. Meant for Skylark usage.
- *
- * @throws IllegalArgumentException if the attribute overrides an existing attribute (will be
- * legal in the future).
*/
public void addOrOverrideAttribute(Attribute attribute) {
- String name = attribute.getName();
- boolean override = attributes.containsKey(name);
- // Attributes may be overridden in the future, thus we keep the code for overriding right
- // now.
- Preconditions.checkArgument(
- !override, "There is already a built-in attribute '%s' which cannot be overridden", name);
-
- if (override) {
+ if (attributes.containsKey(attribute.getName())) {
overrideAttribute(attribute);
} else {
addAttribute(attribute);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
index fc4c222..2f12950 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
@@ -160,10 +160,8 @@
.build();
/**
- * Translates Skylark attribute names to native names.
- *
- * <p> In native code, private values start with "$".In Skylark, private values start with "_",
- * because of the grammar.
+ * In native code, private values start with $.
+ * In Skylark, private values start with _, because of the grammar.
*/
private static String attributeToNative(String oldName, Location loc, boolean isLateBound)
throws EvalException {
@@ -230,76 +228,68 @@
doc = "List of names of configuration fragments that the rule requires.")},
useAst = true, useEnvironment = true)
private static final BuiltinFunction rule = new BuiltinFunction("rule") {
- @SuppressWarnings({"rawtypes", "unchecked"}) // castMap produces
- // an Attribute.Builder instead of a Attribute.Builder<?> but it's OK.
- public BaseFunction invoke(BaseFunction implementation, Boolean test, Object attrs,
- Object implicitOutputs, Boolean executable, Boolean outputToGenfiles, SkylarkList fragments,
- FuncallExpression ast, Environment funcallEnv) throws EvalException, ConversionException {
- Preconditions.checkState(funcallEnv instanceof SkylarkEnvironment);
- RuleClassType type = test ? RuleClassType.TEST : RuleClassType.NORMAL;
+ @SuppressWarnings({"rawtypes", "unchecked"}) // castMap produces
+ // an Attribute.Builder instead of a Attribute.Builder<?> but it's OK.
+ public BaseFunction invoke(BaseFunction implementation, Boolean test, Object attrs,
+ Object implicitOutputs, Boolean executable, Boolean outputToGenfiles,
+ SkylarkList fragments, FuncallExpression ast, Environment funcallEnv)
+ throws EvalException, ConversionException {
- // We'll set the name later, pass the empty string for now.
- RuleClass.Builder builder =
- test ? new RuleClass.Builder("", type, true, testBaseRule)
- : new RuleClass.Builder("", type, true, baseRule);
+ Preconditions.checkState(funcallEnv instanceof SkylarkEnvironment);
+ RuleClassType type = test ? RuleClassType.TEST : RuleClassType.NORMAL;
- if (attrs != Environment.NONE) {
- for (Map.Entry<String, Attribute.Builder> attr :
- castMap(attrs, String.class, Attribute.Builder.class, "attrs").entrySet()) {
- Attribute.Builder<?> attrBuilder = (Attribute.Builder<?>) attr.getValue();
- String attrName =
- attributeToNative(attr.getKey(), ast.getLocation(), attrBuilder.hasLateBoundValue());
- addAttribute(builder, attrBuilder.build(attrName));
+ // We'll set the name later, pass the empty string for now.
+ RuleClass.Builder builder = test
+ ? new RuleClass.Builder("", type, true, testBaseRule)
+ : new RuleClass.Builder("", type, true, baseRule);
+
+ if (attrs != Environment.NONE) {
+ for (Map.Entry<String, Attribute.Builder> attr : castMap(
+ attrs, String.class, Attribute.Builder.class, "attrs").entrySet()) {
+ Attribute.Builder<?> attrBuilder = (Attribute.Builder<?>) attr.getValue();
+ String attrName = attributeToNative(attr.getKey(), ast.getLocation(),
+ attrBuilder.hasLateBoundValue());
+ builder.addOrOverrideAttribute(attrBuilder.build(attrName));
+ }
}
- }
- if (executable || test) {
- addAttribute(
- builder,
- attr("$is_executable", BOOLEAN)
- .value(true)
- .nonconfigurable("Called from RunCommand.isExecutable, which takes a Target")
- .build());
- builder.setOutputsDefaultExecutable();
- }
-
- if (implicitOutputs != Environment.NONE) {
- if (implicitOutputs instanceof BaseFunction) {
- BaseFunction func = (BaseFunction) implicitOutputs;
- final SkylarkCallbackFunction callback =
- new SkylarkCallbackFunction(func, ast, (SkylarkEnvironment) funcallEnv);
- builder.setImplicitOutputsFunction(
- new SkylarkImplicitOutputsFunctionWithCallback(callback, ast.getLocation()));
- } else {
- builder.setImplicitOutputsFunction(
- new SkylarkImplicitOutputsFunctionWithMap(ImmutableMap.copyOf(castMap(implicitOutputs,
- String.class, String.class, "implicit outputs of the rule class"))));
+ if (executable || test) {
+ builder.addOrOverrideAttribute(
+ attr("$is_executable", BOOLEAN).value(true)
+ .nonconfigurable("Called from RunCommand.isExecutable, which takes a Target")
+ .build());
+ builder.setOutputsDefaultExecutable();
}
- }
- if (outputToGenfiles) {
- builder.setOutputToGenfiles();
- }
+ if (implicitOutputs != Environment.NONE) {
+ if (implicitOutputs instanceof BaseFunction) {
+ BaseFunction func = (BaseFunction) implicitOutputs;
+ final SkylarkCallbackFunction callback =
+ new SkylarkCallbackFunction(func, ast, (SkylarkEnvironment) funcallEnv);
+ builder.setImplicitOutputsFunction(
+ new SkylarkImplicitOutputsFunctionWithCallback(callback, ast.getLocation()));
+ } else {
+ builder.setImplicitOutputsFunction(new SkylarkImplicitOutputsFunctionWithMap(
+ ImmutableMap.copyOf(castMap(implicitOutputs, String.class, String.class,
+ "implicit outputs of the rule class"))));
+ }
+ }
- if (!fragments.isEmpty()) {
- builder.requiresConfigurationFragments(
- new SkylarkModuleNameResolver(),
- Iterables.toArray(castList(fragments, String.class), String.class));
- }
+ if (outputToGenfiles) {
+ builder.setOutputToGenfiles();
+ }
- builder.setConfiguredTargetFunction(implementation);
- builder.setRuleDefinitionEnvironment(
- ((SkylarkEnvironment) funcallEnv).getGlobalEnvironment());
- return new RuleFunction(builder, type);
- }
-
- private void addAttribute(RuleClass.Builder builder, Attribute attribute) throws EvalException {
- try {
- builder.addOrOverrideAttribute(attribute);
- } catch (IllegalArgumentException ex) {
- throw new EvalException(location, ex);
- }
- }
- };
+ if (!fragments.isEmpty()) {
+ builder.requiresConfigurationFragments(
+ new SkylarkModuleNameResolver(),
+ Iterables.toArray(castList(fragments, String.class), String.class));
+ }
+
+ builder.setConfiguredTargetFunction(implementation);
+ builder.setRuleDefinitionEnvironment(
+ ((SkylarkEnvironment) funcallEnv).getGlobalEnvironment());
+ return new RuleFunction(builder, type);
+ }
+ };
// This class is needed for testing
static final class RuleFunction extends BaseFunction {
