diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
3 files changed, 103 insertions, 55 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 80b2c4e9ca..5e38598317 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 @@ -2054,7 +2054,7 @@ public final class RuleContext extends TargetContext /** * Because some rules still have to use allowedRuleClasses to do rule dependency validation. - * A dependency is valid if it is from a rule in allowedRuledClasses, OR if all of the providers + * A dependency is valid if it is from a rule in allowedRuleClasses, OR if all of the providers * in mandatoryNativeProviders AND mandatoryProvidersList are provided by the target. */ private void validateRuleDependency(ConfiguredTarget prerequisite, Attribute attribute) { @@ -2077,17 +2077,17 @@ public final class RuleContext extends TargetContext false); } - if (attribute.getAllowedRuleClassesWarningPredicate() != Predicates.<RuleClass>alwaysTrue()) { - Predicate<RuleClass> warningPredicate = attribute.getAllowedRuleClassesWarningPredicate(); - if (warningPredicate.apply(ruleClass)) { - reportBadPrerequisite(attribute, prerequisiteTarget.getTargetKind(), prerequisite, - "expected " + attribute.getAllowedRuleClassesPredicate(), true); - // prerequisite has a rule class allowed with a warning => accept, emitting a warning. - return; - } + if (attribute.getAllowedRuleClassesWarningPredicate().apply(ruleClass)) { + Predicate<RuleClass> allowedRuleClasses = attribute.getAllowedRuleClassesPredicate(); + reportBadPrerequisite(attribute, prerequisiteTarget.getTargetKind(), prerequisite, + allowedRuleClasses == Predicates.<RuleClass>alwaysTrue() + ? null : "expected " + allowedRuleClasses, + true); + // prerequisite has a rule class allowed with a warning => accept, emitting a warning. + return; } - // If we got there, we have no allowed rule class. + // If we get here, we have no allowed rule class. // If mandatory providers are specified, try them. Set<String> unfulfilledRequirements = new LinkedHashSet<>(); if (!attribute.getMandatoryNativeProvidersList().isEmpty() 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 0214a173f0..afcb9c5947 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 @@ -32,6 +32,7 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Location; +import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassNamePredicate; import com.google.devtools.build.lib.syntax.ClassObject; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.EvalUtils; @@ -806,27 +807,39 @@ public final class Attribute implements Comparable<Attribute> { } /** - * If this is a label or label-list attribute, then this sets the allowed - * rule types for the labels occurring in the attribute. If the attribute - * contains Labels of any other rule type, then an error is produced during - * the analysis phase. Defaults to allow any types. + * If this is a label or label-list attribute, then this sets the allowed rule types for the + * labels occurring in the attribute. * - * <p>This only works on a per-target basis, not on a per-file basis; with - * other words, it works for 'deps' attributes, but not 'srcs' attributes. + * <p>If the attribute contains Labels of any other rule type, then if they're in + * {@link #allowedRuleClassesForLabelsWarning}, the build continues with a warning. Else if + * they fulfill {@link #getMandatoryNativeProvidersList()}, the build continues without error. + * Else the build fails during analysis. + * + * <p>If neither this nor {@link #allowedRuleClassesForLabelsWarning} is set, only rules that + * fulfill {@link #getMandatoryNativeProvidersList()} build without error. + * + * <p>This only works on a per-target basis, not on a per-file basis; with other words, it + * works for 'deps' attributes, but not 'srcs' attributes. */ public Builder<TYPE> allowedRuleClasses(Iterable<String> allowedRuleClasses) { return allowedRuleClasses( - new RuleClass.Builder.RuleClassNamePredicate(allowedRuleClasses)); + new RuleClassNamePredicate(allowedRuleClasses)); } /** - * If this is a label or label-list attribute, then this sets the allowed - * rule types for the labels occurring in the attribute. If the attribute - * contains Labels of any other rule type, then an error is produced during - * the analysis phase. Defaults to allow any types. + * If this is a label or label-list attribute, then this sets the allowed rule types for the + * labels occurring in the attribute. * - * <p>This only works on a per-target basis, not on a per-file basis; with - * other words, it works for 'deps' attributes, but not 'srcs' attributes. + * <p>If the attribute contains Labels of any other rule type, then if they're in + * {@link #allowedRuleClassesForLabelsWarning}, the build continues with a warning. Else if + * they fulfill {@link #getMandatoryNativeProvidersList()}, the build continues without error. + * Else the build fails during analysis. + * + * <p>If neither this nor {@link #allowedRuleClassesForLabelsWarning} is set, only rules that + * fulfill {@link #getMandatoryNativeProvidersList()} build without error. + * + * <p>This only works on a per-target basis, not on a per-file basis; with other words, it + * works for 'deps' attributes, but not 'srcs' attributes. */ public Builder<TYPE> allowedRuleClasses(Predicate<RuleClass> allowedRuleClasses) { Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY, @@ -837,13 +850,19 @@ public final class Attribute implements Comparable<Attribute> { } /** - * If this is a label or label-list attribute, then this sets the allowed - * rule types for the labels occurring in the attribute. If the attribute - * contains Labels of any other rule type, then an error is produced during - * the analysis phase. Defaults to allow any types. + * If this is a label or label-list attribute, then this sets the allowed rule types for the + * labels occurring in the attribute. * - * <p>This only works on a per-target basis, not on a per-file basis; with - * other words, it works for 'deps' attributes, but not 'srcs' attributes. + * <p>If the attribute contains Labels of any other rule type, then if they're in + * {@link #allowedRuleClassesForLabelsWarning}, the build continues with a warning. Else if + * they fulfill {@link #getMandatoryNativeProvidersList()}, the build continues without error. + * Else the build fails during analysis. + * + * <p>If neither this nor {@link #allowedRuleClassesForLabelsWarning} is set, only rules that + * fulfill {@link #getMandatoryNativeProvidersList()} build without error. + * + * <p>This only works on a per-target basis, not on a per-file basis; with other words, it + * works for 'deps' attributes, but not 'srcs' attributes. */ public Builder<TYPE> allowedRuleClasses(String... allowedRuleClasses) { return allowedRuleClasses(ImmutableSet.copyOf(allowedRuleClasses)); @@ -889,29 +908,39 @@ public final class Attribute implements Comparable<Attribute> { } /** - * If this is a label or label-list attribute, then this sets the allowed - * rule types with warning for the labels occurring in the attribute. If the attribute - * contains Labels of any other rule type (other than this or those set in - * allowedRuleClasses()), then a warning is produced during - * the analysis phase. Defaults to deny any types. + * If this is a label or label-list attribute, then this sets the allowed rule types with + * warning for the labels occurring in the attribute. This must be a disjoint set from + * {@link #allowedRuleClasses}. * - * <p>This only works on a per-target basis, not on a per-file basis; with - * other words, it works for 'deps' attributes, but not 'srcs' attributes. + * <p>If the attribute contains Labels of any other rule type (other than this or those set in + * allowedRuleClasses()) and they fulfill {@link #getMandatoryNativeProvidersList()}}, the build + * continues without error. Else the build fails during analysis. + * + * <p>If neither this nor {@link #allowedRuleClassesForLabels} is set, only rules that + * fulfill {@link #getMandatoryNativeProvidersList()} build without error. + * + * <p>This only works on a per-target basis, not on a per-file basis; with other words, it + * works for 'deps' attributes, but not 'srcs' attributes. */ public Builder<TYPE> allowedRuleClassesWithWarning(Collection<String> allowedRuleClasses) { return allowedRuleClassesWithWarning( - new RuleClass.Builder.RuleClassNamePredicate(allowedRuleClasses)); + new RuleClassNamePredicate(allowedRuleClasses)); } /** - * If this is a label or label-list attribute, then this sets the allowed - * rule types for the labels occurring in the attribute. If the attribute - * contains Labels of any other rule type (other than this or those set in - * allowedRuleClasses()), then a warning is produced during - * the analysis phase. Defaults to deny any types. + * If this is a label or label-list attribute, then this sets the allowed rule types with + * warning for the labels occurring in the attribute. This must be a disjoint set from + * {@link #allowedRuleClasses}. * - * <p>This only works on a per-target basis, not on a per-file basis; with - * other words, it works for 'deps' attributes, but not 'srcs' attributes. + * <p>If the attribute contains Labels of any other rule type (other than this or those set in + * allowedRuleClasses()) and they fulfill {@link #getMandatoryNativeProvidersList()}}, the build + * continues without error. Else the build fails during analysis. + * + * <p>If neither this nor {@link #allowedRuleClassesForLabels} is set, only rules that + * fulfill {@link #getMandatoryNativeProvidersList()} build without error. + * + * <p>This only works on a per-target basis, not on a per-file basis; with other words, it + * works for 'deps' attributes, but not 'srcs' attributes. */ public Builder<TYPE> allowedRuleClassesWithWarning(Predicate<RuleClass> allowedRuleClasses) { Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY, @@ -922,14 +951,19 @@ public final class Attribute implements Comparable<Attribute> { } /** - * If this is a label or label-list attribute, then this sets the allowed - * rule types for the labels occurring in the attribute. If the attribute - * contains Labels of any other rule type (other than this or those set in - * allowedRuleClasses()), then a warning is produced during - * the analysis phase. Defaults to deny any types. + * If this is a label or label-list attribute, then this sets the allowed rule types with + * warning for the labels occurring in the attribute. This must be a disjoint set from + * {@link #allowedRuleClasses}. * - * <p>This only works on a per-target basis, not on a per-file basis; with - * other words, it works for 'deps' attributes, but not 'srcs' attributes. + * <p>If the attribute contains Labels of any other rule type (other than this or those set in + * allowedRuleClasses()) and they fulfill {@link #getMandatoryNativeProvidersList()}}, the build + * continues without error. Else the build fails during analysis. + * + * <p>If neither this nor {@link #allowedRuleClassesForLabels} is set, only rules that + * fulfill {@link #getMandatoryNativeProvidersList()} build without error. + * + * <p>This only works on a per-target basis, not on a per-file basis; with other words, it + * works for 'deps' attributes, but not 'srcs' attributes. */ public Builder<TYPE> allowedRuleClassesWithWarning(String... allowedRuleClasses) { return allowedRuleClassesWithWarning(ImmutableSet.copyOf(allowedRuleClasses)); @@ -1123,6 +1157,16 @@ public final class Attribute implements Comparable<Attribute> { allowedFileTypesForLabels = FileTypeSet.ANY_FILE; } } + + if (allowedRuleClassesForLabels instanceof RuleClassNamePredicate + && allowedRuleClassesForLabelsWarning instanceof RuleClassNamePredicate) { + Preconditions.checkState( + !((RuleClassNamePredicate) allowedRuleClassesForLabels) + .intersects((RuleClassNamePredicate) allowedRuleClassesForLabelsWarning), + "allowedRuleClasses and allowedRuleClassesWithWarning may not contain " + + "the same rule classes"); + } + return new Attribute( name, type, 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 ae9ec0369e..ae0c358a1e 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 @@ -57,6 +57,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; import java.util.BitSet; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -422,10 +423,6 @@ public class RuleClass { this.ruleClasses = ImmutableSet.copyOf(ruleClasses); } - public RuleClassNamePredicate() { - this(ImmutableSet.<String>of()); - } - @Override public boolean apply(RuleClass ruleClass) { return ruleClasses.contains(ruleClass.getName()); @@ -442,6 +439,13 @@ public class RuleClass { && ruleClasses.equals(((RuleClassNamePredicate) o).ruleClasses); } + /** + * Returns true if this and the other predicate have common rule class entries. + */ + public boolean intersects(RuleClassNamePredicate other) { + return !Collections.disjoint(ruleClasses, other.ruleClasses); + } + @Override public String toString() { return ruleClasses.isEmpty() ? "nothing" : StringUtil.joinEnglishList(ruleClasses); |