From 6acfe4ea6f586be211b46622cda485ea83eca7a9 Mon Sep 17 00:00:00 2001 From: gregce Date: Fri, 25 May 2018 08:38:39 -0700 Subject: Provide ability to inject custom ConstraintSemantics behavior. PiperOrigin-RevId: 198053509 --- .../devtools/build/lib/analysis/BuildView.java | 1 + .../lib/analysis/ConfiguredRuleClassProvider.java | 28 +++++++++++--- .../lib/analysis/ConfiguredTargetFactory.java | 2 + .../lib/analysis/RuleConfiguredTargetBuilder.java | 5 ++- .../devtools/build/lib/analysis/RuleContext.java | 20 +++++++++- .../analysis/constraints/ConstraintSemantics.java | 45 ++++++++++++++-------- 6 files changed, 77 insertions(+), 24 deletions(-) (limited to 'src/main/java/com/google/devtools/build') diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java index 458f56cd2f..ae3a500bd5 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java @@ -1263,6 +1263,7 @@ public class BuildView { .setConfigConditions(ImmutableMap.of()) .setUniversalFragments(ruleClassProvider.getUniversalFragments()) .setToolchainContext(toolchainContext) + .setConstraintSemantics(ruleClassProvider.getConstraintSemantics()) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java index 2fd2617367..a0321ec5aa 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java @@ -23,7 +23,6 @@ import com.google.common.base.Preconditions; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; -import com.google.common.collect.ImmutableBiMap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -37,6 +36,7 @@ import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactor import com.google.devtools.build.lib.analysis.config.DefaultsPackage; import com.google.devtools.build.lib.analysis.config.FragmentOptions; import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; +import com.google.devtools.build.lib.analysis.constraints.ConstraintSemantics; import com.google.devtools.build.lib.analysis.skylark.SkylarkModules; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; @@ -245,8 +245,7 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { private Set reservedActionMnemonics = new TreeSet<>(); private BuildConfiguration.ActionEnvironmentProvider actionEnvironmentProvider = (BuildOptions options) -> ActionEnvironment.EMPTY; - private ImmutableBiMap.Builder> - registeredSkylarkProviders = ImmutableBiMap.builder(); + private ConstraintSemantics constraintSemantics = new ConstraintSemantics(); // TODO(pcloudy): Remove this field after Bazel rule definitions are not used internally. private String nativeLauncherLabel; @@ -380,6 +379,16 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { return this; } + /** + * Sets the logic that lets rules declare which environments they support and validates rules + * don't depend on rules that aren't compatible with the same environments. Defaults to + * {@ConstraintSemantics}. See {@ConstraintSemantics} for more details. + */ + public Builder setConstraintSemantics(ConstraintSemantics constraintSemantics) { + this.constraintSemantics = constraintSemantics; + return this; + } + /** * Sets the C++ LIPO data transition, as defined in {@link * com.google.devtools.build.lib.rules.cpp.transitions.DisableLipoTransition}. @@ -507,7 +516,8 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { skylarkAccessibleTopLevels.build(), skylarkModules.build(), ImmutableSet.copyOf(reservedActionMnemonics), - actionEnvironmentProvider); + actionEnvironmentProvider, + constraintSemantics); } @Override @@ -622,6 +632,8 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { private final ImmutableMap> configurationFragmentMap; + private final ConstraintSemantics constraintSemantics; + private ConfiguredRuleClassProvider( Label preludeLabel, String runfilesPrefix, @@ -641,7 +653,8 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { ImmutableMap skylarkAccessibleJavaClasses, ImmutableList> skylarkModules, ImmutableSet reservedActionMnemonics, - BuildConfiguration.ActionEnvironmentProvider actionEnvironmentProvider) { + BuildConfiguration.ActionEnvironmentProvider actionEnvironmentProvider, + ConstraintSemantics constraintSemantics) { this.preludeLabel = preludeLabel; this.runfilesPrefix = runfilesPrefix; this.toolsRepository = toolsRepository; @@ -661,6 +674,7 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { this.reservedActionMnemonics = reservedActionMnemonics; this.actionEnvironmentProvider = actionEnvironmentProvider; this.configurationFragmentMap = createFragmentMap(configurationFragmentFactories); + this.constraintSemantics = constraintSemantics; } public PrerequisiteValidator getPrerequisiteValidator() { @@ -860,6 +874,10 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider { return configurationFragmentMap; } + public ConstraintSemantics getConstraintSemantics() { + return constraintSemantics; + } + /** Returns all skylark objects in global scope for this RuleClassProvider. */ public Map getTransitiveGlobalBindings() { return globals.getTransitiveBindings(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java index 3cda14c0f8..40ed396d07 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java @@ -340,6 +340,7 @@ public final class ConfiguredTargetFactory { .setConfigConditions(configConditions) .setUniversalFragments(ruleClassProvider.getUniversalFragments()) .setToolchainContext(toolchainContext) + .setConstraintSemantics(ruleClassProvider.getConstraintSemantics()) .build(); if (ruleContext.hasErrors()) { return null; @@ -464,6 +465,7 @@ public final class ConfiguredTargetFactory { .setConfigConditions(configConditions) .setUniversalFragments(ruleClassProvider.getUniversalFragments()) .setToolchainContext(toolchainContext) + .setConstraintSemantics(ruleClassProvider.getConstraintSemantics()) .build(); if (ruleContext.hasErrors()) { return null; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java index e3f587eb8f..786dba9aeb 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java @@ -173,12 +173,13 @@ public final class RuleConfiguredTargetBuilder { if (!ruleContext.getRule().getRuleClassObject().supportsConstraintChecking()) { return; } + ConstraintSemantics constraintSemantics = ruleContext.getConstraintSemantics(); EnvironmentCollection supportedEnvironments = - ConstraintSemantics.getSupportedEnvironments(ruleContext); + constraintSemantics.getSupportedEnvironments(ruleContext); if (supportedEnvironments != null) { EnvironmentCollection.Builder refinedEnvironments = new EnvironmentCollection.Builder(); Map removedEnvironmentCulprits = new LinkedHashMap<>(); - ConstraintSemantics.checkConstraints(ruleContext, supportedEnvironments, refinedEnvironments, + constraintSemantics.checkConstraints(ruleContext, supportedEnvironments, refinedEnvironments, removedEnvironmentCulprits); add(SupportedEnvironmentsProvider.class, new SupportedEnvironments(supportedEnvironments, refinedEnvironments.build(), 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 93387fa6cc..4381373bea 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 @@ -54,6 +54,7 @@ import com.google.devtools.build.lib.analysis.config.transitions.NoTransition; import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; +import com.google.devtools.build.lib.analysis.constraints.ConstraintSemantics; import com.google.devtools.build.lib.analysis.fileset.FilesetProvider; import com.google.devtools.build.lib.analysis.platform.PlatformInfo; import com.google.devtools.build.lib.analysis.stringtemplate.TemplateContext; @@ -185,6 +186,7 @@ public final class RuleContext extends TargetContext private final ImmutableList> universalFragments; private final ErrorReporter reporter; @Nullable private final ToolchainContext toolchainContext; + private final ConstraintSemantics constraintSemantics; private ActionOwner actionOwner; @@ -200,7 +202,8 @@ public final class RuleContext extends TargetContext ImmutableList> universalFragments, String ruleClassNameForLogging, ImmutableMap aspectAttributes, - @Nullable ToolchainContext toolchainContext) { + @Nullable ToolchainContext toolchainContext, + ConstraintSemantics constraintSemantics) { super(builder.env, builder.rule, builder.configuration, builder.prerequisiteMap.get(null), builder.visibility); this.rule = builder.rule; @@ -227,6 +230,7 @@ public final class RuleContext extends TargetContext this.disableLipoTransition = builder.disableLipoTransition; reporter = builder.reporter; this.toolchainContext = toolchainContext; + this.constraintSemantics = constraintSemantics; } private void getAllFeatures(Set allEnabledFeatures, Set allDisabledFeatures) { @@ -1067,6 +1071,10 @@ public final class RuleContext extends TargetContext return toolchainContext; } + public ConstraintSemantics getConstraintSemantics() { + return constraintSemantics; + } + @Override @Nullable public PlatformInfo getExecutionPlatform() { @@ -1403,6 +1411,7 @@ public final class RuleContext extends TargetContext private ImmutableMap aspectAttributes; private ImmutableList aspects; private ToolchainContext toolchainContext; + private ConstraintSemantics constraintSemantics; Builder( AnalysisEnvironment env, @@ -1428,6 +1437,7 @@ public final class RuleContext extends TargetContext Preconditions.checkNotNull(prerequisiteMap); Preconditions.checkNotNull(configConditions); Preconditions.checkNotNull(visibility); + Preconditions.checkNotNull(constraintSemantics); AttributeMap attributes = ConfiguredAttributeMapper.of(rule, configConditions); validateAttributes(attributes); ListMultimap targetMap = createTargetMap(); @@ -1442,7 +1452,8 @@ public final class RuleContext extends TargetContext universalFragments, getRuleClassNameForLogging(), aspectAttributes != null ? aspectAttributes : ImmutableMap.of(), - toolchainContext); + toolchainContext, + constraintSemantics); } private void validateAttributes(AttributeMap attributes) { @@ -1500,6 +1511,11 @@ public final class RuleContext extends TargetContext return this; } + Builder setConstraintSemantics(ConstraintSemantics constraintSemantics) { + this.constraintSemantics = constraintSemantics; + return this; + } + private boolean validateFilesetEntry(FilesetEntry filesetEntry, ConfiguredTargetAndData src) { if (src.getConfiguredTarget().getProvider(FilesetProvider.class) != null) { return true; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java index 3d16a02d4e..ee74b3fcff 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java @@ -143,7 +143,22 @@ import javax.annotation.Nullable; * . */ public class ConstraintSemantics { - private ConstraintSemantics() { + public ConstraintSemantics() { + } + + /** + * Logs an error message that the current rule violates constraints. + */ + public void ruleError(RuleContext ruleContext, String message) { + ruleContext.ruleError(message); + } + + /** + * Logs an error message that an attribute on the current rule doesn't properly declare + * constraints. + */ + public void attributeError(RuleContext ruleContext, String attribute, String message) { + ruleContext.attributeError(attribute, message); } /** @@ -194,7 +209,7 @@ public class ConstraintSemantics { * the given attributes. Only includes environments from "known" groups, i.e. the groups * owning the environments explicitly referenced from these attributes. */ - private static class EnvironmentCollector { + private class EnvironmentCollector { private final RuleContext ruleContext; private final String restrictionAttr; private final String compatibilityAttr; @@ -258,10 +273,10 @@ public class ConstraintSemantics { Label restrictionEnv = restrictionEnvironments.getEnvironments(group).iterator().next(); if (compatibilityEnv.equals(restrictionEnv)) { - ruleContext.attributeError(compatibilityAttr, compatibilityEnv + attributeError(ruleContext, compatibilityAttr, compatibilityEnv + " cannot appear both here and in " + restrictionAttr); } else { - ruleContext.attributeError(compatibilityAttr, compatibilityEnv + " and " + attributeError(ruleContext, compatibilityAttr, compatibilityEnv + " and " + restrictionEnv + " belong to the same environment group. They should be declared " + "together either here or in " + restrictionAttr); } @@ -322,7 +337,7 @@ public class ConstraintSemantics { * environment: itself. Extract that from its more generic provider interface and sanity * check that that's in fact what we see. */ - private static EnvironmentWithGroup resolveEnvironment(TransitiveInfoCollection envRule) { + private EnvironmentWithGroup resolveEnvironment(TransitiveInfoCollection envRule) { SupportedEnvironmentsProvider prereq = Preconditions.checkNotNull(envRule.getProvider(SupportedEnvironmentsProvider.class)); return Iterables.getOnlyElement(prereq.getStaticEnvironments().getGroupedEnvironments()); @@ -378,7 +393,7 @@ public class ConstraintSemantics { * as described above. Returns null if any errors are encountered. */ @Nullable - public static EnvironmentCollection getSupportedEnvironments(RuleContext ruleContext) { + public EnvironmentCollection getSupportedEnvironments(RuleContext ruleContext) { if (!validateAttributes(ruleContext)) { return null; } @@ -420,7 +435,7 @@ public class ConstraintSemantics { * no such defaults. */ @Nullable - private static EnvironmentCollector maybeGetRuleClassDefaults(RuleContext ruleContext) { + private EnvironmentCollector maybeGetRuleClassDefaults(RuleContext ruleContext) { Rule rule = ruleContext.getRule(); String restrictionAttr = RuleClass.DEFAULT_RESTRICTED_ENVIRONMENT_ATTR; String compatibilityAttr = RuleClass.DEFAULT_COMPATIBLE_ENVIRONMENT_ATTR; @@ -459,7 +474,7 @@ public class ConstraintSemantics { * Validity-checks this rule's constraint-related attributes. Returns true if all is good, * returns false and reports appropriate errors if there are any problems. */ - private static boolean validateAttributes(RuleContext ruleContext) { + private boolean validateAttributes(RuleContext ruleContext) { AttributeMap attributes = ruleContext.attributes(); // Report an error if "restricted to" is explicitly set to nothing. Even if this made @@ -469,7 +484,7 @@ public class ConstraintSemantics { .getPrerequisites(restrictionAttr, RuleConfiguredTarget.Mode.DONT_CHECK); if (restrictionEnvironments.isEmpty() && attributes.isAttributeValueExplicitlySpecified(restrictionAttr)) { - ruleContext.attributeError(restrictionAttr, "attribute cannot be empty"); + attributeError(ruleContext, restrictionAttr, "attribute cannot be empty"); return false; } @@ -518,7 +533,7 @@ public class ConstraintSemantics { * pruning away environments through refinement. If multiple dependencies qualify (e.g. two * direct deps under the current rule), one is arbitrarily chosen. */ - public static void checkConstraints( + public void checkConstraints( RuleContext ruleContext, EnvironmentCollection staticEnvironments, EnvironmentCollection.Builder refinedEnvironments, @@ -561,15 +576,15 @@ public class ConstraintSemantics { * @param staticEnvironments the static environments of the rule being analyzed * @param dep the dep to check */ - private static void checkStaticConstraints(RuleContext ruleContext, + private void checkStaticConstraints(RuleContext ruleContext, EnvironmentCollection staticEnvironments, TransitiveInfoCollection dep) { SupportedEnvironmentsProvider depEnvironments = dep.getProvider(SupportedEnvironmentsProvider.class); Collection