diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/analysis')
10 files changed, 66 insertions, 113 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java index 8fff84a91d..269e2451ab 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java @@ -28,15 +28,10 @@ import static com.google.devtools.build.lib.syntax.Type.STRING_LIST; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; -import com.google.devtools.build.lib.analysis.config.DynamicTransitionMapper; import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.analysis.config.RunUnder; -import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy; -import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; -import com.google.devtools.build.lib.analysis.config.transitions.Transition; import com.google.devtools.build.lib.analysis.constraints.EnvironmentRule; import com.google.devtools.build.lib.analysis.test.TestConfiguration; import com.google.devtools.build.lib.cmdline.Label; @@ -392,19 +387,4 @@ public class BaseRuleClasses { .build(); } } - - /** - * Declares the implementations for {@link ConfigurationTransitionProxy} enums. - * - * <p>We can't put this in {@link Attribute} because that's in the {@code lib.packages} package, - * which has no access to configuration classes. - * - * <p>New transitions should extend {@link PatchTransition}, which avoids the need for this map. - */ - public static final ImmutableMap<Transition, Transition> DYNAMIC_TRANSITIONS_MAP = - ImmutableMap.of( - ConfigurationTransitionProxy.NONE, DynamicTransitionMapper.SELF - // ConfigurationTransitionProxy.DATA is skipped because it's C++-specific. - // The C++ rule definitions handle its mapping. - ); } 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 95ea7030d8..5097f42160 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 @@ -45,7 +45,7 @@ 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.ConfigurationResolver; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; -import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy; +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.Transition; import com.google.devtools.build.lib.analysis.constraints.TopLevelConstraintSemantics; @@ -957,7 +957,7 @@ public class BuildView { Iterable<BuildOptions> buildOptions) { Preconditions.checkArgument(ct.getConfiguration().fragmentClasses().equals(fragments)); Dependency asDep = Dependency.withTransitionAndAspects(ct.getLabel(), - ConfigurationTransitionProxy.NONE, AspectCollection.EMPTY); + NoTransition.INSTANCE, AspectCollection.EMPTY); ImmutableList.Builder<BuildConfiguration> builder = ImmutableList.builder(); for (BuildOptions options : buildOptions) { builder.add(Iterables.getOnlyElement( @@ -1036,19 +1036,19 @@ public class BuildView { .getTarget(handler, label) .getAssociatedRule(); } catch (NoSuchPackageException | NoSuchTargetException e) { - return ConfigurationTransitionProxy.NONE; + return NoTransition.INSTANCE; } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new AssertionError("Configuration of " + label + " interrupted"); } if (rule == null) { - return ConfigurationTransitionProxy.NONE; + return NoTransition.INSTANCE; } RuleTransitionFactory factory = rule .getRuleClassObject() .getTransitionFactory(); if (factory == null) { - return ConfigurationTransitionProxy.NONE; + return NoTransition.INSTANCE; } // dynamicTransitionMapper is only needed because of ConfigurationTransitionProxy.DATA: @@ -1056,7 +1056,7 @@ public class BuildView { // C++-specific patch transition that implements it. PatchTransition transition = (PatchTransition) ruleClassProvider.getDynamicTransitionMapper().map(factory.buildTransitionFor(rule)); - return (transition == null) ? ConfigurationTransitionProxy.NONE : transition; + return (transition == null) ? NoTransition.INSTANCE : transition; } /** 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 a02d4105fd..c8cee3cf98 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 @@ -48,6 +48,7 @@ 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.transitions.ConfigurationTransitionProxy; +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.config.transitions.Transition; @@ -416,8 +417,8 @@ public final class RuleContext extends TargetContext @Nullable public <T extends Fragment> T getFragment(Class<T> fragment) { - // NONE means target configuration. - return getFragment(fragment, ConfigurationTransitionProxy.NONE); + // No transition means target configuration. + return getFragment(fragment, NoTransition.INSTANCE); } @Nullable @@ -447,8 +448,8 @@ public final class RuleContext extends TargetContext } public <T extends Fragment> boolean isLegalFragment(Class<T> fragment) { - // NONE means target configuration. - return isLegalFragment(fragment, ConfigurationTransitionProxy.NONE); + // No transition means target configuration. + return isLegalFragment(fragment, NoTransition.INSTANCE); } protected BuildConfiguration getConfiguration(Transition transition) { @@ -1072,8 +1073,7 @@ public final class RuleContext extends TargetContext + " is not configured for the host configuration"); } } else if (mode == Mode.TARGET) { - if (!(transition instanceof PatchTransition) - && transition != ConfigurationTransitionProxy.NONE) { + if (!(transition instanceof PatchTransition) && transition != NoTransition.INSTANCE) { throw new IllegalStateException(getRule().getLocation() + ": " + getRuleClassNameForLogging() + " attribute " + attributeName + " is not configured for the target configuration"); @@ -1095,35 +1095,6 @@ public final class RuleContext extends TargetContext } /** - * Returns the Mode for which the attribute is configured. - * This is intended for Skylark, where the Mode is implicitly chosen. - */ - public Mode getAttributeMode(String attributeName) { - Attribute attributeDefinition = attributes().getAttributeDefinition(attributeName); - if (attributeDefinition == null) { - throw new IllegalStateException(getRule().getLocation() + ": " + getRuleClassNameForLogging() - + " attribute " + attributeName + " is not defined"); - } - if (attributeDefinition.getType().getLabelClass() != LabelClass.DEPENDENCY) { - throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName - + " is not a label type attribute"); - } - if (attributeDefinition.getConfigurationTransition().isHostTransition()) { - return Mode.HOST; - } else if (attributeDefinition.getConfigurationTransition() - == ConfigurationTransitionProxy.NONE) { - return Mode.TARGET; - } else if (attributeDefinition.getConfigurationTransition() - == ConfigurationTransitionProxy.DATA) { - return Mode.DATA; - } else if (attributeDefinition.hasSplitConfigurationTransition()) { - return Mode.SPLIT; - } - throw new IllegalStateException(getRule().getLocation() + ": " - + getRuleClassNameForLogging() + " attribute " + attributeName + " is not configured"); - } - - /** * For the specified attribute "attributeName" (which must be of type * list(label)), resolve all the labels into ConfiguredTargets (for the * configuration appropriate to the attribute) and return their build 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 3bce74d771..c60f6a5aa7 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 @@ -26,7 +26,7 @@ import com.google.common.collect.Multimap; import com.google.common.collect.Sets; import com.google.devtools.build.lib.analysis.Dependency; import com.google.devtools.build.lib.analysis.TargetAndConfiguration; -import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy; +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.config.transitions.Transition; @@ -186,7 +186,7 @@ public final class ConfigurationResolver { Transition transition = dep.getTransition(); if (sameFragments) { - if (transition == ConfigurationTransitionProxy.NONE) { + if (transition == NoTransition.INSTANCE) { // The dep uses the same exact configuration. putOnlyEntry( resolvedDeps, @@ -415,9 +415,7 @@ public final class ConfigurationResolver { Iterable<Class<? extends BuildConfiguration.Fragment>> requiredFragments, RuleClassProvider ruleClassProvider, boolean trimResults) { List<BuildOptions> result; - if (transition == ConfigurationTransitionProxy.NONE) { - result = ImmutableList.<BuildOptions>of(fromOptions); - } else if (transition instanceof PatchTransition) { + if (transition instanceof PatchTransition) { // TODO(bazel-team): safety-check that this never mutates fromOptions. result = ImmutableList.of(((PatchTransition) transition).apply(fromOptions)); } else if (transition instanceof SplitTransition) { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/DynamicTransitionMapper.java b/src/main/java/com/google/devtools/build/lib/analysis/config/DynamicTransitionMapper.java index e481fb1c45..d08a0dbc7d 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/DynamicTransitionMapper.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/DynamicTransitionMapper.java @@ -14,43 +14,23 @@ package com.google.devtools.build.lib.analysis.config; import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy; 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.config.transitions.Transition; -import com.google.devtools.build.lib.packages.Attribute; -import com.google.devtools.build.lib.packages.RuleClass; /** * Maps non-{@link PatchTransition} declarations to their implementable equivalents. * * <p>Blaze applies configuration transitions by executing {@link PatchTransition} instances. But - * for legacy reasons, not every transition declaration is a {@link PatchTransition}. The most - * prominent example is {@link ConfigurationTransitionProxy}, which defines its transitions as - * enums. These transitions are used all over the place. So we need a way to continue to support - * them. + * for legacy reasons, {@link ConfigurationTransitionProxy#DATA} (which is used for C++/LIPO logic) + * is an implementation-free enum. * - * <p>Hence this class. - * - * <p>Going forward, we should eliminate the need for this class by eliminating - * non-{@link PatchTransition} transitions. This is conceptually straightforward: replace - * declarations of the form {@link RuleClass.Builder#cfg(Transition)} with - * {@link RuleClass.Builder#cfg(PatchTransition)}. That way, transition declarations "just work", - * with no extra fuss. But this is a migration that will take some time to complete. - * - * {@link ConfigurationTransitionProxy#DATA} provides the most complicated challenge. This is - * C++/LIPO logic, and the implementation is in C++ rule code - * ({@link com.google.devtools.build.lib.rules.cpp.transitions.DisableLipoTransition}). But the enum - * is defined in {@link Attribute}, which is in {@code lib.packages}, which has access to neither - * rule-specific nor configuration-specific code. Furthermore, many non-C++ rules declare this - * transition. We ultimately need a cleaner way to inject this rules-specific logic into general - * Blaze code. + * <p>We should ultimately restrict that logic just to the C++ rule definitions and remove this + * interface. But {@link ConfigurationTransitionProxy#DATA} is used everywhere, including in + * non-C++ rules and in {@code lib.packages} code, which lacks acccess to C++ configuration logic. */ public final class DynamicTransitionMapper { - /** - * Use this to declare a no-op transition that keeps the input configuration. - */ - public static final Transition SELF = new Transition() {}; - private final ImmutableMap<Transition, Transition> map; /** @@ -79,11 +59,9 @@ public final class DynamicTransitionMapper { return fromTransition; } Transition toTransition = map.get(fromTransition); - if (toTransition == SELF) { - return fromTransition; - } else if (toTransition != null) { - return toTransition; + if (toTransition == null) { + throw new IllegalArgumentException("No dynamic mapping for " + fromTransition.toString()); } - throw new IllegalArgumentException("No dynamic mapping for " + fromTransition.toString()); + return toTransition; } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/TransitionResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/config/TransitionResolver.java index 44530445c5..9fc95970fd 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/TransitionResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/TransitionResolver.java @@ -19,6 +19,7 @@ import com.google.common.base.Preconditions; import com.google.devtools.build.lib.analysis.TargetAndConfiguration; import com.google.devtools.build.lib.analysis.config.transitions.ComposingSplitTransition; import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy; +import com.google.devtools.build.lib.analysis.config.transitions.NoTransition; import com.google.devtools.build.lib.analysis.config.transitions.NullTransition; import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition; @@ -77,7 +78,7 @@ public final class TransitionResolver { // II. Host configurations never switch to another. All prerequisites of host targets have the // same host configuration. if (fromConfig.isHostConfiguration()) { - return ConfigurationTransitionProxy.NONE; + return NoTransition.INSTANCE; } // Make sure config_setting dependencies are resolved in the referencing rule's configuration, @@ -94,13 +95,13 @@ public final class TransitionResolver { // TODO(bazel-team): don't require special casing here. This is far too hackish. if (toTarget instanceof Rule && ((Rule) toTarget).getRuleClassObject().isConfigMatcher()) { // TODO(gregce): see if this actually gets called - return ConfigurationTransitionProxy.NONE; + return NoTransition.INSTANCE; } // The current transition to apply. When multiple transitions are requested, this is a // ComposingSplitTransition, which encapsulates them into a single object so calling code // doesn't need special logic for combinations. - Transition currentTransition = ConfigurationTransitionProxy.NONE; + Transition currentTransition = NoTransition.INSTANCE; // Apply the parent rule's outgoing transition if it has one. RuleTransitionFactory transitionFactory = @@ -140,7 +141,7 @@ public final class TransitionResolver { // Top-level transitions (chosen by configuration fragments): Transition topLevelTransition = fromConfig.topLevelConfigurationHook(target); if (topLevelTransition == null) { - topLevelTransition = ConfigurationTransitionProxy.NONE; + topLevelTransition = NoTransition.INSTANCE; } // Rule class transitions (chosen by rule class definitions): @@ -165,7 +166,7 @@ public final class TransitionResolver { public Transition composeTransitions(Transition transition1, Transition transition2) { if (isFinal(transition1)) { return transition1; - } else if (transition2 == ConfigurationTransitionProxy.NONE) { + } else if (transition2 == NoTransition.INSTANCE) { return transition1; } else if (transition2 == NullTransition.INSTANCE) { // A NULL transition can just replace earlier transitions: no need to compose them. @@ -180,7 +181,7 @@ public final class TransitionResolver { // TODO(gregce): remove the below conversion when all transitions are patch transitions. Transition dynamicTransition = transitionMapper.map(transition2); - return transition1 == ConfigurationTransitionProxy.NONE + return transition1 == NoTransition.INSTANCE ? dynamicTransition : new ComposingSplitTransition(transition1, dynamicTransition); } @@ -202,7 +203,7 @@ public final class TransitionResolver { "cannot apply splits after null transitions (null transitions are expected to be final)"); Preconditions.checkState(currentTransition != HostTransition.INSTANCE, "cannot apply splits after host transitions (host transitions are expected to be final)"); - return currentTransition == ConfigurationTransitionProxy.NONE + return currentTransition == NoTransition.INSTANCE ? split : new ComposingSplitTransition(currentTransition, split); } @@ -226,7 +227,7 @@ public final class TransitionResolver { PatchTransition ruleClassTransition = (PatchTransition) transitionMapper.map(transitionFactory.buildTransitionFor(associatedRule)); if (ruleClassTransition != null) { - if (currentTransition == ConfigurationTransitionProxy.NONE) { + if (currentTransition == NoTransition.INSTANCE) { return ruleClassTransition; } else { return new ComposingSplitTransition(currentTransition, ruleClassTransition); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingSplitTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingSplitTransition.java index d58bafd917..d36968c21b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingSplitTransition.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingSplitTransition.java @@ -70,9 +70,7 @@ public class ComposingSplitTransition implements SplitTransition { */ // TODO(gregce): move this somewhere more general. This isn't intrinsic to composed splits. static List<BuildOptions> apply(BuildOptions fromOptions, Transition transition) { - if (transition == ConfigurationTransitionProxy.NONE) { - return ImmutableList.<BuildOptions>of(fromOptions); - } else if (transition instanceof PatchTransition) { + if (transition instanceof PatchTransition) { return ImmutableList.<BuildOptions>of(((PatchTransition) transition).apply(fromOptions)); } else if (transition instanceof SplitTransition) { SplitTransition split = (SplitTransition) transition; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ConfigurationTransitionProxy.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ConfigurationTransitionProxy.java index 6a19b36fbb..81baf16503 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ConfigurationTransitionProxy.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ConfigurationTransitionProxy.java @@ -22,9 +22,6 @@ package com.google.devtools.build.lib.analysis.config.transitions; */ @Deprecated public enum ConfigurationTransitionProxy implements Transition { - /** No transition, i.e., the same configuration as the current. */ - NONE, - /** Transition from the target configuration to the data configuration. */ // TODO(bazel-team): Move this elsewhere. DATA, diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoTransition.java new file mode 100644 index 0000000000..d5581e5469 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoTransition.java @@ -0,0 +1,30 @@ +// Copyright 2018 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.analysis.config.transitions; + +import com.google.devtools.build.lib.analysis.config.BuildOptions; + +/** + * No-op configuration transition. + */ +public final class NoTransition implements PatchTransition { + public static final NoTransition INSTANCE = new NoTransition(); + + private NoTransition() {} + + @Override + public BuildOptions apply(BuildOptions options) { + return options; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java index 506ff64543..ce082a3aea 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java @@ -37,7 +37,7 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.FragmentCollection; import com.google.devtools.build.lib.analysis.config.HostTransition; -import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy; +import com.google.devtools.build.lib.analysis.config.transitions.NoTransition; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.stringtemplate.ExpansionException; import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector; @@ -225,7 +225,7 @@ public final class SkylarkRuleContext implements SkylarkValue { this.actionFactory = new SkylarkActionFactory(this, skylarkSemantics, ruleContext); this.ruleContext = Preconditions.checkNotNull(ruleContext); this.ruleLabelCanonicalName = ruleContext.getLabel().getCanonicalForm(); - this.fragments = new FragmentCollection(ruleContext, ConfigurationTransitionProxy.NONE); + this.fragments = new FragmentCollection(ruleContext, NoTransition.INSTANCE); this.hostFragments = new FragmentCollection(ruleContext, HostTransition.INSTANCE); this.aspectDescriptor = aspectDescriptor; this.skylarkSemantics = skylarkSemantics; |