diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
4 files changed, 13 insertions, 101 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java index f5829ed2e9..4997a895f8 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java @@ -13,7 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.analysis; -import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -376,16 +375,12 @@ public abstract class DependencyResolver { getSplitOptions(depResolver.rule, attribute, ruleConfig); if (!splitOptions.isEmpty() && !ruleConfig.isHostConfiguration()) { // Late-bound attribute with a split transition: - // Since we want to get the same results as BuildConfiguration.evaluateTransition (but + // Since we want to get the same results as ConfigurationResolver.evaluateTransition (but // skip it since we've already applied the split), we want to make sure this logic - // doesn't do anything differently. evaluateTransition has additional logic - // for host configs and attributes with configurators. To keep the fork as simple as - // possible, we don't support the configurator case. And when we're in the host - // configuration, we fall back to the non-split branch, which calls - // BuildConfiguration.evaluateTransition, which returns its "host mode" result without - // ever looking at the split. - Verify.verify(attribute.getConfigurator() == null); - + // doesn't do anything differently. ConfigurationResolver.evaluateTransition has additional + // logic for host configs. So when we're in the host configuration we fall back to the + // non-split branch, which calls ConfigurationResolver.evaluateTransition, which returns its + // "host mode" result without ever looking at the split. Iterable<BuildConfiguration> splitConfigs = getConfigurations(ruleConfig.fragmentClasses(), splitOptions); if (splitConfigs == null) { 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 3f28614cb7..162b9defe7 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 @@ -47,6 +47,7 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.analysis.config.FragmentCollection; +import com.google.devtools.build.lib.analysis.config.PatchTransition; import com.google.devtools.build.lib.analysis.fileset.FilesetProvider; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.ImmutableSortedKeyListMultimap; @@ -1232,20 +1233,21 @@ public final class RuleContext extends TargetContext throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName + " is not a label type attribute"); } + Attribute.Transition transition = attributeDefinition.getConfigurationTransition(); if (mode == Mode.HOST) { - if (attributeDefinition.getConfigurationTransition() != ConfigurationTransition.HOST) { + if (!(transition instanceof PatchTransition) && transition != ConfigurationTransition.HOST) { throw new IllegalStateException(getRule().getLocation() + ": " + getRuleClassNameForLogging() + " attribute " + attributeName + " is not configured for the host configuration"); } } else if (mode == Mode.TARGET) { - if (attributeDefinition.getConfigurationTransition() != ConfigurationTransition.NONE) { + if (!(transition instanceof PatchTransition) && transition != ConfigurationTransition.NONE) { throw new IllegalStateException(getRule().getLocation() + ": " + getRuleClassNameForLogging() + " attribute " + attributeName + " is not configured for the target configuration"); } } else if (mode == Mode.DATA) { - if (attributeDefinition.getConfigurationTransition() != ConfigurationTransition.DATA) { + if (!(transition instanceof PatchTransition) && transition != ConfigurationTransition.DATA) { throw new IllegalStateException(getRule().getLocation() + ": " + getRuleClassNameForLogging() + " attribute " + attributeName + " is not configured for the data configuration"); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java index 3f6e710430..2da77b0129 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java @@ -15,10 +15,8 @@ package com.google.devtools.build.lib.analysis.config; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.Iterables; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; -import com.google.devtools.build.lib.packages.Attribute.Configurator; import com.google.devtools.build.lib.packages.Attribute.SplitTransition; import com.google.devtools.build.lib.packages.Attribute.Transition; import com.google.devtools.build.lib.packages.InputFile; @@ -109,23 +107,13 @@ public final class ConfigurationResolver { // TODO(gregce): make the below transitions composable (i.e. take away the "else" clauses). // The "else" is a legacy restriction from static configurations. if (attribute.hasSplitConfigurationTransition()) { - Preconditions.checkState(attribute.getConfigurator() == null); currentTransition = split(currentTransition, (SplitTransition<BuildOptions>) attribute.getSplitTransition(fromRule)); } else { // III. Attributes determine configurations. The configuration of a prerequisite is determined // by the attribute. - @SuppressWarnings("unchecked") - Configurator<BuildOptions> configurator = - (Configurator<BuildOptions>) attribute.getConfigurator(); - if (configurator != null) { - // TODO(gregce): attribute configurators are a holdover from static configurations. Remove - // them and remove this branch. - currentTransition = applyAttributeConfigurator(currentTransition, configurator); - } else { - currentTransition = composeTransitions(currentTransition, - attribute.getConfigurationTransition()); - } + currentTransition = composeTransitions(currentTransition, + attribute.getConfigurationTransition()); } return applyConfigurationHook(currentTransition, toTarget); @@ -190,37 +178,6 @@ public final class ConfigurationResolver { } /** - * Applies the given attribute configurator and composes it after an existing transition. - */ - @VisibleForTesting - public Transition applyAttributeConfigurator(Transition currentTransition, - Configurator<BuildOptions> configurator) { - if (isFinal(currentTransition)) { - return currentTransition; - } - return composeTransitions(currentTransition, new AttributeConfiguratorTransition(configurator)); - } - - /** - * A {@link PatchTransition} that applies an attribute configurator over some input options - * to determine which transition to use, then applies that transition over those options - * for the final output. - */ - private static final class AttributeConfiguratorTransition implements PatchTransition { - private final Configurator<BuildOptions> configurator; - - AttributeConfiguratorTransition(Configurator<BuildOptions> configurator) { - this.configurator = configurator; - } - - @Override - public BuildOptions apply(BuildOptions options) { - return Iterables.getOnlyElement( - ComposingSplitTransition.apply(options, configurator.apply(options))); - } - } - - /** * Applies any configuration hooks associated with the dep target, composes their transitions * after an existing transition, and returns the composed result. */ 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 85a0f25b99..5a020be02f 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 @@ -311,22 +311,6 @@ public final class Attribute implements Comparable<Attribute> { }; /** - * Using this callback function, rules can change the configuration of their dependencies during - * the analysis phase. - * - * <p>If dynamic configurations are enabled, the returned transition must be a - * {@link com.google.devtools.build.lib.analysis.config.PatchTransition}. - * - * @deprecated this is only needed for statically configured builds. Dynamically configured builds - * should just use {@link Attribute.Builder#cfg(Transition)}} with a directly provided - * {@link com.google.devtools.build.lib.analysis.config.PatchTransition}. - */ - @Deprecated - public interface Configurator<TBuildOptions> { - Transition apply(TBuildOptions fromOptions); - } - - /** * Provides a {@link SplitTransition} given the originating target {@link Rule}. The split * transition may be constant for all instances of the originating rule, or it may differ * based on attributes of that rule. For instance, a split transition on a rule's deps may differ @@ -424,7 +408,6 @@ public final class Attribute implements Comparable<Attribute> { private Transition configTransition = ConfigurationTransition.NONE; private Predicate<RuleClass> allowedRuleClassesForLabels = Predicates.alwaysTrue(); private Predicate<RuleClass> allowedRuleClassesForLabelsWarning = Predicates.alwaysFalse(); - private Configurator<?> configurator; private SplitTransitionProvider splitTransitionProvider; private FileTypeSet allowedFileTypesForLabels; private ValidityPredicate validityPredicate = ANY_EDGE; @@ -576,17 +559,6 @@ public final class Attribute implements Comparable<Attribute> { } /** - * @deprecated Use {@link #cfg(Transition)}} with a - * {@link com.google.devtools.build.lib.analysis.config.PatchTransition} instead. This method - * only provides legacy support for statically configured builds. - */ - @Deprecated - public Builder<TYPE> cfg(Configurator<?> configurator) { - this.configurator = configurator; - return this; - } - - /** * Requires the attribute target to be executable; only for label or label * list attributes. Defaults to {@code false}. */ @@ -1143,7 +1115,6 @@ public final class Attribute implements Comparable<Attribute> { Sets.immutableEnumSet(propertyFlags), valueSet ? value : type.getDefaultValue(), configTransition, - configurator, splitTransitionProvider, allowedRuleClassesForLabels, allowedRuleClassesForLabelsWarning, @@ -1731,7 +1702,6 @@ public final class Attribute implements Comparable<Attribute> { private final Transition configTransition; - private final Configurator<?> configurator; private final SplitTransitionProvider splitTransitionProvider; /** @@ -1786,7 +1756,6 @@ public final class Attribute implements Comparable<Attribute> { Set<PropertyFlag> propertyFlags, Object defaultValue, Transition configTransition, - Configurator<?> configurator, SplitTransitionProvider splitTransitionProvider, Predicate<RuleClass> allowedRuleClassesForLabels, Predicate<RuleClass> allowedRuleClassesForLabelsWarning, @@ -1798,7 +1767,7 @@ public final class Attribute implements Comparable<Attribute> { ImmutableList<RuleAspect<?>> aspects) { Preconditions.checkNotNull(configTransition); Preconditions.checkArgument( - (configTransition == ConfigurationTransition.NONE && configurator == null) + (configTransition == ConfigurationTransition.NONE) || type.getLabelClass() == LabelClass.DEPENDENCY || type.getLabelClass() == LabelClass.NONDEP_REFERENCE, "Configuration transitions can only be specified for label or label list attributes"); @@ -1808,8 +1777,6 @@ public final class Attribute implements Comparable<Attribute> { name); if (isLateBound(name)) { LateBoundDefault<?> lateBoundDefault = (LateBoundDefault<?>) defaultValue; - Preconditions.checkArgument((configurator == null), - "a late bound attribute cannot specify a configurator"); Preconditions.checkArgument(!lateBoundDefault.useHostConfiguration() || (configTransition == ConfigurationTransition.HOST), "a late bound default value using the host configuration must use the host transition"); @@ -1820,7 +1787,6 @@ public final class Attribute implements Comparable<Attribute> { this.propertyFlags = propertyFlags; this.defaultValue = defaultValue; this.configTransition = configTransition; - this.configurator = configurator; this.splitTransitionProvider = splitTransitionProvider; this.allowedRuleClassesForLabels = allowedRuleClassesForLabels; this.allowedRuleClassesForLabelsWarning = allowedRuleClassesForLabelsWarning; @@ -1920,14 +1886,6 @@ public final class Attribute implements Comparable<Attribute> { } /** - * Returns the configurator instance for this attribute for label or label list attributes. - * For other attributes it will always return {@code null}. - */ - public Configurator<?> getConfigurator() { - return configurator; - } - - /** * Returns the split configuration transition for this attribute. * * @param rule the originating {@link Rule} which owns this attribute |