diff options
Diffstat (limited to 'src')
5 files changed, 151 insertions, 25 deletions
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 3527c31949..707b688387 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 @@ -19,6 +19,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Verify; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.Iterables; +import com.google.common.collect.Sets; import com.google.devtools.build.lib.analysis.OutputFileConfiguredTarget; import com.google.devtools.build.lib.analysis.RuleConfiguredTarget; import com.google.devtools.build.lib.analysis.RuleContext; @@ -458,28 +459,44 @@ public class ConstraintSemantics { */ public static Collection<Label> getUnsupportedEnvironments( EnvironmentCollection actualEnvironments, EnvironmentCollection expectedEnvironments) { - Set<Label> missingEnvironments = new LinkedHashSet<>(); + Collection<Label> actualEnvironmentLabels = actualEnvironments.getEnvironments(); - // For each expected environment, it must either be a supported environment OR a default - // for a group the supported environment set doesn't know about. + // Check if each explicitly expected environment is satisfied. for (EnvironmentWithGroup expectedEnv : expectedEnvironments.getGroupedEnvironments()) { EnvironmentGroup group = expectedEnv.group(); Label environment = expectedEnv.environment(); - if (!actualEnvironments.getEnvironments().contains(environment) - && (actualEnvironments.getGroups().contains(group) || !group.isDefault(environment))) { + boolean isSatisfied = false; + if (actualEnvironments.getGroups().contains(group)) { + // If the actual environments include members from the expected environment's group, we + // need to either find the environment itself or another one that transitively fulfills it. + if (actualEnvironmentLabels.contains(environment) + || intersect(actualEnvironmentLabels, group.getFulfillers(environment))) { + isSatisfied = true; + } + } else { + // If the actual environments don't reference the expected environment's group at all, + // the group's defaults are implicitly included. So we need to check those defaults for + // either the expected environment or another environment that transitively fulfills it. + if (group.isDefault(environment) + || intersect(group.getFulfillers(environment), group.getDefaults())) { + isSatisfied = true; + } + } + if (!isSatisfied) { missingEnvironments.add(environment); } } // For any environment group not referenced by the expected environments, its defaults are - // implicitly applied. We can ignore it if it's also missing from the supported environments - // (since in that case the same defaults apply), otherwise have to check. + // implicitly expected. We can ignore this if the actual environments also don't reference the + // group (since in that case the same defaults apply), otherwise we have to check. for (EnvironmentGroup group : actualEnvironments.getGroups()) { if (!expectedEnvironments.getGroups().contains(group)) { - for (Label defaultEnv : group.getDefaults()) { - if (!actualEnvironments.getEnvironments().contains(defaultEnv)) { - missingEnvironments.add(defaultEnv); + for (Label expectedDefault : group.getDefaults()) { + if (!actualEnvironmentLabels.contains(expectedDefault) + && !intersect(actualEnvironmentLabels, group.getFulfillers(expectedDefault))) { + missingEnvironments.add(expectedDefault); } } } @@ -488,6 +505,10 @@ public class ConstraintSemantics { return missingEnvironments; } + private static boolean intersect(Iterable<Label> labels1, Iterable<Label> labels2) { + return !Sets.intersection(Sets.newHashSet(labels1), Sets.newHashSet(labels2)).isEmpty(); + } + /** * Returns all dependencies that should be constraint-checked against the current rule. */ diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/Environment.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/Environment.java index ab91f454ea..c769cfb830 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/Environment.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/Environment.java @@ -38,8 +38,6 @@ public class Environment implements RuleConfiguredTargetFactory { // The main analysis work to do here is to simply fill in SupportedEnvironmentsProvider to // pass the environment itself to depending rules. - // - // This will likely expand when we add support for environments fulfilling other environments. Label label = ruleContext.getLabel(); EnvironmentGroup group; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/EnvironmentRule.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/EnvironmentRule.java index ea0644ef82..58116e8267 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/EnvironmentRule.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/EnvironmentRule.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.Type; +import com.google.devtools.build.lib.util.FileTypeSet; /** * Rule definition for environment rules (for Bazel's constraint enforcement system). @@ -33,13 +34,33 @@ import com.google.devtools.build.lib.packages.Type; public final class EnvironmentRule implements RuleDefinition { public static final String RULE_NAME = "environment"; + public static final String FULFILLS_ATTRIBUTE = "fulfills"; + @Override public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) { return builder .override(attr("tags", Type.STRING_LIST) - // No need to show up in ":all", etc. target patterns. + // No need to show up in ":all", etc. target patterns. .value(ImmutableList.of("manual")) .nonconfigurable("low-level attribute, used in TargetUtils without configurations")) + /* <!-- #BLAZE_RULE(environment).ATTRIBUTE(fulfills) --> + The set of environments this one is considered a valid "standin" for. + ${SYNOPSIS} + <p> + If rule A depends on rule B, A declares compatibility with environment <code>:foo</code>, + and B declares compatibility with environment <code>:bar</code>, this is normally not + considered a valid dependency. But if <code>:bar</code> fulfills <code>:foo</code>, the + dependency is considered valid. B's own dependencies are subsequently expected to support + <code>:bar</code> (the original expectation for <code>:foo</code> is dropped). + </p> + <p> + Environments may only fulfill other environments in the same environment group. + </p> + <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ + .add(attr(FULFILLS_ATTRIBUTE, Type.LABEL_LIST) + .allowedRuleClasses(EnvironmentRule.RULE_NAME) + .allowedFileTypes(FileTypeSet.NO_FILE) + .nonconfigurable("used for defining constraint models - this shouldn't be configured")) .removeAttribute(RuleClass.COMPATIBLE_ENVIRONMENT_ATTR) .removeAttribute(RuleClass.RESTRICTED_ENVIRONMENT_ATTR) .exemptFromConstraintChecking("this rule *defines* a constraint") diff --git a/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java b/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java index 07da227daf..ff9edaefd0 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java +++ b/src/main/java/com/google/devtools/build/lib/packages/EnvironmentGroup.java @@ -15,9 +15,15 @@ package com.google.devtools.build.lib.packages; import com.google.common.base.Predicate; +import com.google.common.base.Verify; +import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.common.collect.Multimap; import com.google.common.collect.Sets; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Location; @@ -25,6 +31,7 @@ import com.google.devtools.build.lib.syntax.Label; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -67,6 +74,13 @@ public class EnvironmentGroup implements Target { private final Set<Label> defaults; /** + * Maps a member environment to the set of environments that directly fulfill it. Note that + * we can't populate this map until all Target instances for member environments have been + * initialized, which may occur after group instantiation (this makes the class mutable). + */ + private final Map<Label, NestedSet<Label>> fulfillersMap = new HashMap<>(); + + /** * Predicate that matches labels from a different package than the initialized package. */ private static final class DifferentPackage implements Predicate<Label> { @@ -107,7 +121,7 @@ public class EnvironmentGroup implements Target { * and checks that all defaults are legitimate members of the group. * * <p>Does <b>not</b> check that the referenced environments exist (see - * {@link #checkEnvironmentsExist). + * {@link #processMemberEnvironments}). * * @return a list of validation errors that occurred */ @@ -131,26 +145,85 @@ public class EnvironmentGroup implements Target { } /** - * Given the set of targets in this group's package, checks that all of the group's declared - * environments are part of that set (i.e. the group doesn't reference non-existant labels). + * Checks that the group's declared environments are legitimate same-package environment + * rules and prepares the "fulfills" relationships between these environments to support + * {@link #getFulfillers}. * * @param pkgTargets mapping from label name to target instance for this group's package * @return a list of validation errors that occurred */ - List<Event> checkEnvironmentsExist(Map<String, Target> pkgTargets) { + List<Event> processMemberEnvironments(Map<String, Target> pkgTargets) { List<Event> events = new ArrayList<>(); + // Maps an environment to the environments that directly fulfill it. + Multimap<Label, Label> directFulfillers = HashMultimap.create(); + for (Label envName : environments) { - Target env = pkgTargets.get(envName.getName()); - if (env == null) { - events.add(Event.error(location, "environment " + envName + " does not exist")); - } else if (!env.getTargetKind().equals("environment rule")) { - events.add(Event.error(location, env.getLabel() + " is not a valid environment")); + Target env = pkgTargets.get(envName.getName()); + if (isValidEnvironment(env, envName, "", events)) { + AttributeMap attr = NonconfigurableAttributeMapper.of((Rule) env); + for (Label fulfilledEnv : attr.get("fulfills", Type.LABEL_LIST)) { + if (isValidEnvironment(pkgTargets.get(fulfilledEnv.getName()), fulfilledEnv, + "in \"fulfills\" attribute of " + envName + ": ", events)) { + directFulfillers.put(fulfilledEnv, envName); + } + } } } + + // Now that we know which environments directly fulfill each other, compute which environments + // transitively fulfill each other. We could alternatively compute this on-demand, but since + // we don't expect these chains to be very large we opt toward computing them once at package + // load time. + Verify.verify(fulfillersMap.isEmpty()); + for (Label envName : environments) { + setTransitiveFulfillers(envName, directFulfillers, fulfillersMap); + } + return events; } /** + * Given an environment and set of environments that directly fulfill it, computes a nested + * set of environments that <i>transitively</i> fulfill it, places it into transitiveFulfillers, + * and returns that set. + */ + private static NestedSet<Label> setTransitiveFulfillers(Label env, + Multimap<Label, Label> directFulfillers, Map<Label, NestedSet<Label>> transitiveFulfillers) { + if (transitiveFulfillers.containsKey(env)) { + return transitiveFulfillers.get(env); + } else if (!directFulfillers.containsKey(env)) { + // Nobody fulfills this environment. + NestedSet<Label> emptySet = NestedSetBuilder.emptySet(Order.STABLE_ORDER); + transitiveFulfillers.put(env, emptySet); + return emptySet; + } else { + NestedSetBuilder<Label> set = NestedSetBuilder.stableOrder(); + for (Label fulfillingEnv : directFulfillers.get(env)) { + set.add(fulfillingEnv); + set.addTransitive( + setTransitiveFulfillers(fulfillingEnv, directFulfillers, transitiveFulfillers)); + } + NestedSet<Label> builtSet = set.build(); + transitiveFulfillers.put(env, builtSet); + return builtSet; + } + } + + private boolean isValidEnvironment(Target env, Label envName, String prefix, List<Event> events) { + if (env == null) { + events.add(Event.error(location, prefix + "environment " + envName + " does not exist")); + return false; + } else if (!env.getTargetKind().equals("environment rule")) { + events.add(Event.error(location, prefix + env.getLabel() + " is not a valid environment")); + return false; + } else if (!environments.contains(env.getLabel())) { + events.add(Event.error(location, prefix + env.getLabel() + " is not a member of this group")); + return false; + } + return true; + } + + /** * Returns the environments that belong to this group. */ public Set<Label> getEnvironments() { @@ -173,6 +246,20 @@ public class EnvironmentGroup implements Target { return defaults.contains(environment); } + /** + * Returns the set of environments that transitively fulfill the specified environment. + * The environment must be a valid member of this group. + * + * <p>>For example, if the input is <code>":foo"</code> and <code>":bar"</code> fulfills + * <code>":foo"</code> and <code>":baz"</code> fulfills <code>":bar"</code>, this returns + * <code>[":foo", ":bar", ":baz"]</code>. + * + * <p>If no environments fulfill the input, returns an empty set. + */ + public Iterable<Label> getFulfillers(Label environment) { + return Verify.verifyNotNull(fulfillersMap.get(environment)); + } + @Override public Label getLabel() { return label; diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java index 5d2bafdc2c..b909dd8604 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Package.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java @@ -1385,10 +1385,9 @@ public class Package implements Serializable { defaultDistributionSet = Collections.unmodifiableSet(defaultDistributionSet); - // Now all targets have been loaded, so we can check all declared environments in an - // environment group exist. + // Now all targets have been loaded, so we validate the group's member environments. for (EnvironmentGroup envGroup : ImmutableSet.copyOf(environmentGroups.values())) { - Collection<Event> errors = envGroup.checkEnvironmentsExist(targets); + Collection<Event> errors = envGroup.processMemberEnvironments(targets); if (!errors.isEmpty()) { addEvents(errors); setContainsErrors(); |