From 4e093d8e08454c9c2c04e94fe2e97ef4592198b6 Mon Sep 17 00:00:00 2001 From: gregce Date: Thu, 28 Dec 2017 11:58:15 -0800 Subject: Replace Attribute.ConfigurationTransition with config.transitions.ConfigurationTransitionProxy. The "proxy" part is to dissuade people from writing: void myfunc(ConfigurationTransition transition) signatures casually. Maybe that's actually a better name than "Transition". But I'd rather rename Transition to ConfigurationTransition in its own change if we want to do that. PiperOrigin-RevId: 180285321 --- .../build/lib/analysis/BaseRuleClasses.java | 11 +++--- .../devtools/build/lib/analysis/BuildView.java | 14 +++---- .../build/lib/analysis/DependencyResolver.java | 3 +- .../devtools/build/lib/analysis/RuleContext.java | 18 +++++---- .../analysis/config/ComposingSplitTransition.java | 4 +- .../lib/analysis/config/ConfigurationResolver.java | 5 ++- .../analysis/config/DynamicTransitionMapper.java | 4 +- .../lib/analysis/config/TransitionResolver.java | 32 ++++++++-------- .../transitions/ConfigurationTransition.java | 43 --------------------- .../transitions/ConfigurationTransitionProxy.java | 44 ++++++++++++++++++++++ .../build/lib/analysis/skylark/SkylarkAttr.java | 4 +- .../skylark/SkylarkRuleClassFunctions.java | 2 +- .../lib/analysis/skylark/SkylarkRuleContext.java | 4 +- .../lib/bazel/rules/common/BazelFilegroupRule.java | 2 +- .../devtools/build/lib/packages/Attribute.java | 39 ++++--------------- .../lib/packages/ConfigurationFragmentPolicy.java | 10 ++--- .../devtools/build/lib/rules/cpp/CcImportRule.java | 2 +- .../build/lib/rules/cpp/CppRuleClasses.java | 4 +- .../build/lib/skyframe/SkyframeExecutor.java | 10 ++--- 19 files changed, 118 insertions(+), 137 deletions(-) delete mode 100644 src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ConfigurationTransition.java create mode 100644 src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ConfigurationTransitionProxy.java (limited to 'src/main/java/com/google/devtools') 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 35ed29b55b..aec52e1233 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 @@ -14,7 +14,7 @@ package com.google.devtools.build.lib.analysis; -import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.DATA; +import static com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy.DATA; import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.DISTRIBUTIONS; import static com.google.devtools.build.lib.packages.BuildType.LABEL; @@ -35,6 +35,7 @@ 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.PatchTransition; 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.Transition; import com.google.devtools.build.lib.analysis.constraints.EnvironmentRule; import com.google.devtools.build.lib.analysis.test.TestConfiguration; @@ -393,7 +394,7 @@ public class BaseRuleClasses { } /** - * Declares the implementations for {@link Attribute.ConfigurationTransition} enums. + * Declares the implementations for {@link ConfigurationTransitionProxy} enums. * *

We can't put this in {@link Attribute} because that's in the {@code lib.packages} package, * which has no access to configuration classes. @@ -402,9 +403,9 @@ public class BaseRuleClasses { */ public static final ImmutableMap DYNAMIC_TRANSITIONS_MAP = ImmutableMap.of( - Attribute.ConfigurationTransition.NONE, DynamicTransitionMapper.SELF, - Attribute.ConfigurationTransition.NULL, DynamicTransitionMapper.SELF - // Attribute.ConfigurationTransition.DATA is skipped because it's C++-specific. + ConfigurationTransitionProxy.NONE, DynamicTransitionMapper.SELF, + ConfigurationTransitionProxy.NULL, 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 222a250b4e..8beac6213c 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,6 +45,7 @@ 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.PatchTransition; +import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy; import com.google.devtools.build.lib.analysis.config.transitions.Transition; import com.google.devtools.build.lib.analysis.constraints.TopLevelConstraintSemantics; import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory; @@ -62,7 +63,6 @@ import com.google.devtools.build.lib.packages.AspectClass; import com.google.devtools.build.lib.packages.AspectDescriptor; import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Attribute; -import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.NativeAspectClass; import com.google.devtools.build.lib.packages.NoSuchPackageException; @@ -941,7 +941,7 @@ public class BuildView { Iterable buildOptions) { Preconditions.checkArgument(ct.getConfiguration().fragmentClasses().equals(fragments)); Dependency asDep = Dependency.withTransitionAndAspects(ct.getLabel(), - Attribute.ConfigurationTransition.NONE, AspectCollection.EMPTY); + ConfigurationTransitionProxy.NONE, AspectCollection.EMPTY); ImmutableList.Builder builder = ImmutableList.builder(); for (BuildOptions options : buildOptions) { builder.add(Iterables.getOnlyElement( @@ -1020,27 +1020,27 @@ public class BuildView { .getTarget(handler, label) .getAssociatedRule(); } catch (NoSuchPackageException | NoSuchTargetException e) { - return ConfigurationTransition.NONE; + return ConfigurationTransitionProxy.NONE; } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new AssertionError("Configuration of " + label + " interrupted"); } if (rule == null) { - return ConfigurationTransition.NONE; + return ConfigurationTransitionProxy.NONE; } RuleTransitionFactory factory = rule .getRuleClassObject() .getTransitionFactory(); if (factory == null) { - return ConfigurationTransition.NONE; + return ConfigurationTransitionProxy.NONE; } - // dynamicTransitionMapper is only needed because of Attribute.ConfigurationTransition.DATA: + // dynamicTransitionMapper is only needed because of ConfigurationTransitionProxy.DATA: // this is C++-specific but non-C++ rules declare it. So they can't directly provide the // C++-specific patch transition that implements it. PatchTransition transition = (PatchTransition) ruleClassProvider.getDynamicTransitionMapper().map(factory.buildTransitionFor(rule)); - return (transition == null) ? ConfigurationTransition.NONE : transition; + return (transition == null) ? ConfigurationTransitionProxy.NONE : transition; } /** 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 d268e7d2b3..87145f86c6 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 @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.PatchTransition; import com.google.devtools.build.lib.analysis.config.TransitionResolver; +import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy; 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.cmdline.Label; @@ -727,7 +728,7 @@ public abstract class DependencyResolver { ruleConfig, rule, attributeAndOwner.attribute, toTarget, attributeMap); outgoingEdges.put( attributeAndOwner.attribute, - transition == Attribute.ConfigurationTransition.NULL + transition == ConfigurationTransitionProxy.NULL ? Dependency.withNullConfiguration(depLabel) : Dependency.withTransitionAndAspects(depLabel, transition, requiredAspects(attributeAndOwner, toTarget))); 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 ce2569a1a6..25a0cf3381 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.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.config.transitions.ConfigurationTransitionProxy; 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.analysis.configuredtargets.RuleConfiguredTarget.Mode; @@ -65,7 +66,6 @@ import com.google.devtools.build.lib.packages.AbstractRuleErrorConsumer; import com.google.devtools.build.lib.packages.Aspect; import com.google.devtools.build.lib.packages.AspectDescriptor; import com.google.devtools.build.lib.packages.Attribute; -import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy; @@ -410,7 +410,7 @@ public final class RuleContext extends TargetContext @Nullable public T getFragment(Class fragment) { // NONE means target configuration. - return getFragment(fragment, ConfigurationTransition.NONE); + return getFragment(fragment, ConfigurationTransitionProxy.NONE); } @Nullable @@ -441,7 +441,7 @@ public final class RuleContext extends TargetContext public boolean isLegalFragment(Class fragment) { // NONE means target configuration. - return isLegalFragment(fragment, ConfigurationTransition.NONE); + return isLegalFragment(fragment, ConfigurationTransitionProxy.NONE); } protected BuildConfiguration getConfiguration(Transition transition) { @@ -1063,13 +1063,15 @@ public final class RuleContext extends TargetContext + " is not configured for the host configuration"); } } else if (mode == Mode.TARGET) { - if (!(transition instanceof PatchTransition) && transition != ConfigurationTransition.NONE) { + if (!(transition instanceof PatchTransition) + && transition != ConfigurationTransitionProxy.NONE) { throw new IllegalStateException(getRule().getLocation() + ": " + getRuleClassNameForLogging() + " attribute " + attributeName + " is not configured for the target configuration"); } } else if (mode == Mode.DATA) { - if (!(transition instanceof PatchTransition) && transition != ConfigurationTransition.DATA) { + if (!(transition instanceof PatchTransition) + && transition != ConfigurationTransitionProxy.DATA) { throw new IllegalStateException(getRule().getLocation() + ": " + getRuleClassNameForLogging() + " attribute " + attributeName + " is not configured for the data configuration"); @@ -1099,9 +1101,11 @@ public final class RuleContext extends TargetContext } if (attributeDefinition.getConfigurationTransition().isHostTransition()) { return Mode.HOST; - } else if (attributeDefinition.getConfigurationTransition() == ConfigurationTransition.NONE) { + } else if (attributeDefinition.getConfigurationTransition() + == ConfigurationTransitionProxy.NONE) { return Mode.TARGET; - } else if (attributeDefinition.getConfigurationTransition() == ConfigurationTransition.DATA) { + } else if (attributeDefinition.getConfigurationTransition() + == ConfigurationTransitionProxy.DATA) { return Mode.DATA; } else if (attributeDefinition.hasSplitConfigurationTransition()) { return Mode.SPLIT; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingSplitTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingSplitTransition.java index 41cb44d3c0..e0f40f88ec 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingSplitTransition.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ComposingSplitTransition.java @@ -16,9 +16,9 @@ package com.google.devtools.build.lib.analysis.config; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy; 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.ConfigurationTransition; import java.util.List; /** @@ -72,7 +72,7 @@ public class ComposingSplitTransition implements SplitTransition { */ // TODO(gregce): move this somewhere more general. This isn't intrinsic to composed splits. static List apply(BuildOptions fromOptions, Transition transition) { - if (transition == ConfigurationTransition.NONE) { + if (transition == ConfigurationTransitionProxy.NONE) { return ImmutableList.of(fromOptions); } else if (transition instanceof PatchTransition) { return ImmutableList.of(((PatchTransition) transition).apply(fromOptions)); 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 d09219eaee..c10e265399 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,6 +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.SplitTransition; import com.google.devtools.build.lib.analysis.config.transitions.Transition; import com.google.devtools.build.lib.cmdline.Label; @@ -184,7 +185,7 @@ public final class ConfigurationResolver { Transition transition = dep.getTransition(); if (sameFragments) { - if (transition == Attribute.ConfigurationTransition.NONE) { + if (transition == ConfigurationTransitionProxy.NONE) { // The dep uses the same exact configuration. putOnlyEntry( resolvedDeps, @@ -413,7 +414,7 @@ public final class ConfigurationResolver { Iterable> requiredFragments, RuleClassProvider ruleClassProvider, boolean trimResults) { List result; - if (transition == Attribute.ConfigurationTransition.NONE) { + if (transition == ConfigurationTransitionProxy.NONE) { result = ImmutableList.of(fromOptions); } else if (transition instanceof PatchTransition) { // TODO(bazel-team): safety-check that this never mutates fromOptions. 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 4c492017c7..96316ebce5 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 @@ -24,7 +24,7 @@ import com.google.devtools.build.lib.packages.RuleClass; * *

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 Attribute.ConfigurationTransition}, which defines its transitions as + * 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. * @@ -36,7 +36,7 @@ import com.google.devtools.build.lib.packages.RuleClass; * {@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 Attribute.ConfigurationTransition#DATA} provides the most complicated challenge. This is + * {@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 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 27c3548a2c..fe1a0f8f10 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 @@ -17,10 +17,10 @@ package com.google.devtools.build.lib.analysis.config; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; 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.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.Attribute.ConfigurationTransition; import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper; import com.google.devtools.build.lib.packages.InputFile; import com.google.devtools.build.lib.packages.PackageGroup; @@ -61,20 +61,20 @@ public final class TransitionResolver { * * @return the child's configuration, expressed as a diff from the parent's configuration. This * is usually a {@PatchTransition} but exceptions apply (e.g. - * {@link Attribute.ConfigurationTransition}). + * {@link ConfigurationTransitionProxy}). */ public Transition evaluateTransition(BuildConfiguration fromConfig, final Rule fromRule, final Attribute attribute, final Target toTarget, ConfiguredAttributeMapper attributeMap) { // I. Input files and package groups have no configurations. We don't want to duplicate them. if (usesNullConfiguration(toTarget)) { - return Attribute.ConfigurationTransition.NULL; + return ConfigurationTransitionProxy.NULL; } // II. Host configurations never switch to another. All prerequisites of host targets have the // same host configuration. if (fromConfig.isHostConfiguration()) { - return Attribute.ConfigurationTransition.NONE; + return ConfigurationTransitionProxy.NONE; } // Make sure config_setting dependencies are resolved in the referencing rule's configuration, @@ -91,13 +91,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 Attribute.ConfigurationTransition.NONE; + return ConfigurationTransitionProxy.NONE; } // 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 = Attribute.ConfigurationTransition.NONE; + Transition currentTransition = ConfigurationTransitionProxy.NONE; // Apply the parent rule's outgoing transition if it has one. RuleTransitionFactory transitionFactory = @@ -137,7 +137,7 @@ public final class TransitionResolver { // Top-level transitions (chosen by configuration fragments): Transition topLevelTransition = fromConfig.topLevelConfigurationHook(target); if (topLevelTransition == null) { - topLevelTransition = ConfigurationTransition.NONE; + topLevelTransition = ConfigurationTransitionProxy.NONE; } // Rule class transitions (chosen by rule class definitions): @@ -162,11 +162,11 @@ public final class TransitionResolver { public Transition composeTransitions(Transition transition1, Transition transition2) { if (isFinal(transition1)) { return transition1; - } else if (transition2 == Attribute.ConfigurationTransition.NONE) { + } else if (transition2 == ConfigurationTransitionProxy.NONE) { return transition1; - } else if (transition2 == Attribute.ConfigurationTransition.NULL) { + } else if (transition2 == ConfigurationTransitionProxy.NULL) { // A NULL transition can just replace earlier transitions: no need to compose them. - return Attribute.ConfigurationTransition.NULL; + return ConfigurationTransitionProxy.NULL; } else if (transition2.isHostTransition()) { // A HOST transition can just replace earlier transitions: no need to compose them. // But it also improves performance: host transitions are common, and @@ -177,7 +177,7 @@ public final class TransitionResolver { // TODO(gregce): remove the below conversion when all transitions are patch transitions. Transition dynamicTransition = transitionMapper.map(transition2); - return transition1 == Attribute.ConfigurationTransition.NONE + return transition1 == ConfigurationTransitionProxy.NONE ? dynamicTransition : new ComposingSplitTransition(transition1, dynamicTransition); } @@ -187,7 +187,7 @@ public final class TransitionResolver { * be composed after it. */ private static boolean isFinal(Transition transition) { - return (transition == Attribute.ConfigurationTransition.NULL + return (transition == ConfigurationTransitionProxy.NULL || transition == HostTransition.INSTANCE); } @@ -195,11 +195,11 @@ public final class TransitionResolver { * Applies the given split and composes it after an existing transition. */ private static Transition split(Transition currentTransition, SplitTransition split) { - Preconditions.checkState(currentTransition != Attribute.ConfigurationTransition.NULL, + Preconditions.checkState(currentTransition != ConfigurationTransitionProxy.NULL, "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 == Attribute.ConfigurationTransition.NONE + return currentTransition == ConfigurationTransitionProxy.NONE ? split : new ComposingSplitTransition(currentTransition, split); } @@ -207,7 +207,7 @@ public final class TransitionResolver { /** * @param currentTransition a pre-existing transition to be composed with * @param toTarget rule to examine for transitions - * @param transitionMapper only needed because of Attribute.ConfigurationTransition.DATA: this is + * @param transitionMapper only needed because of ConfigurationTransitionProxy.DATA: this is * C++-specific but non-C++ rules declare it. So they can't directly provide the C++-specific * patch transition that implements it. */ @@ -223,7 +223,7 @@ public final class TransitionResolver { PatchTransition ruleClassTransition = (PatchTransition) transitionMapper.map(transitionFactory.buildTransitionFor(associatedRule)); if (ruleClassTransition != null) { - if (currentTransition == ConfigurationTransition.NONE) { + if (currentTransition == ConfigurationTransitionProxy.NONE) { return ruleClassTransition; } else { return new ComposingSplitTransition(currentTransition, ruleClassTransition); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ConfigurationTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ConfigurationTransition.java deleted file mode 100644 index 6cee8d4509..0000000000 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ConfigurationTransition.java +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright 2017 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; - -/** - * Declaration how the configuration should change when following a label or label list attribute. - * - *

Do not add to this. This is a legacy interface from when Blaze had limited support for - * transitions. Use {@link PatchTransition} or {@link SplitTransition} instead. - */ -@Deprecated -public enum ConfigurationTransition implements Transition { - /** No transition, i.e., the same configuration as the current. */ - NONE, - - /** Transition to a null configuration (applies to, e.g., input files). */ - NULL, - - /** Transition from the target configuration to the data configuration. */ - // TODO(bazel-team): Move this elsewhere. - DATA, - - /** - * Transition to one or more configurations. To obtain the actual child configurations, - * invoke {@link com.google.devtools.build.lib.packages.Attribute#getSplitTransition}). - * com.google.devtools.build.lib.packages.AttributeMap)}. - * - *

See {@link SplitTransition}. - **/ - SPLIT -} 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 new file mode 100644 index 0000000000..1bf0f403ef --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ConfigurationTransitionProxy.java @@ -0,0 +1,44 @@ +// Copyright 2017 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; + +/** + * Declaration how the configuration should change when following a label or label list attribute. + * + *

Do not add to this. This is a legacy interface from when Blaze had limited support for + * transitions. Use {@link com.google.devtools.build.lib.analysis.config.PatchTransition} or + * {@link SplitTransition} instead. + */ +@Deprecated +public enum ConfigurationTransitionProxy implements Transition { + /** No transition, i.e., the same configuration as the current. */ + NONE, + + /** Transition to a null configuration (applies to, e.g., input files). */ + NULL, + + /** Transition from the target configuration to the data configuration. */ + // TODO(bazel-team): Move this elsewhere. + DATA, + + /** + * Transition to one or more configurations. To obtain the actual child configurations, + * invoke {@link com.google.devtools.build.lib.packages.Attribute#getSplitTransition}). + * com.google.devtools.build.lib.packages.AttributeMap)}. + * + *

See {@link SplitTransition}. + **/ + SPLIT +} diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java index 78e393fb51..9abc869ad9 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java @@ -20,12 +20,12 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; 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.SplitTransition; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet; -import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; import com.google.devtools.build.lib.packages.Attribute.SkylarkComputedDefaultTemplate; import com.google.devtools.build.lib.packages.Attribute.SplitTransitionProvider; import com.google.devtools.build.lib.packages.AttributeValueSource; @@ -295,7 +295,7 @@ public final class SkylarkAttr implements SkylarkValue { if (containsNonNoneKey(arguments, CONFIGURATION_ARG)) { Object trans = arguments.get(CONFIGURATION_ARG); if (trans.equals("data")) { - builder.cfg(ConfigurationTransition.DATA); + builder.cfg(ConfigurationTransitionProxy.DATA); } else if (trans.equals("host")) { builder.cfg(HostTransition.INSTANCE); } else if (trans instanceof SplitTransition) { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java index e27d47963e..32a78c9ad2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java @@ -15,7 +15,7 @@ package com.google.devtools.build.lib.analysis.skylark; import static com.google.devtools.build.lib.analysis.BaseRuleClasses.RUN_UNDER; -import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.DATA; +import static com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy.DATA; import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL; import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; 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 3f8a54a71b..8ce345fc7b 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,6 +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.configuredtargets.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.stringtemplate.ExpansionException; import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector; @@ -46,7 +47,6 @@ import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.packages.Aspect; import com.google.devtools.build.lib.packages.AspectDescriptor; import com.google.devtools.build.lib.packages.Attribute; -import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SkylarkImplicitOutputsFunction; @@ -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, ConfigurationTransition.NONE); + this.fragments = new FragmentCollection(ruleContext, ConfigurationTransitionProxy.NONE); this.hostFragments = new FragmentCollection(ruleContext, HostTransition.INSTANCE); this.aspectDescriptor = aspectDescriptor; this.skylarkSemantics = skylarkSemantics; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelFilegroupRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelFilegroupRule.java index 0548b00581..68844815ba 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelFilegroupRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelFilegroupRule.java @@ -13,7 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.bazel.rules.common; -import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.DATA; +import static com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy.DATA; import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; import static com.google.devtools.build.lib.packages.BuildType.LICENSE; 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 38fdda7317..0a368e1fc8 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 @@ -29,6 +29,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Ordering; import com.google.common.collect.Sets; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; +import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy; 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.events.EventHandler; @@ -147,32 +148,6 @@ public final class Attribute implements Comparable { } } - /** - * Declaration how the configuration should change when following a label or label list attribute. - * - *

Do not add to this. Use {@link - * com.google.devtools.build.lib.analysis.config.PatchTransition} or {@link SplitTransition} - * instead. - */ - public enum ConfigurationTransition implements Transition { - /** No transition, i.e., the same configuration as the current. */ - NONE, - - /** Transition to a null configuration (applies to, e.g., input files). */ - NULL, - - /** Transition from the target configuration to the data configuration. */ - // TODO(bazel-team): Move this elsewhere. - DATA, - - /** - * Transition to one or more configurations. To obtain the actual child configurations, - * invoke {@link Attribute#getSplitTransition(AttributeMap)}. - * See {@link SplitTransition}. - **/ - SPLIT - } - private enum PropertyFlag { MANDATORY, EXECUTABLE, @@ -384,7 +359,7 @@ public final class Attribute implements Comparable { public static class Builder { private final String name; private final Type type; - private Transition configTransition = ConfigurationTransition.NONE; + private Transition configTransition = ConfigurationTransitionProxy.NONE; private RuleClassNamePredicate allowedRuleClassesForLabels = ANY_RULE; private RuleClassNamePredicate allowedRuleClassesForLabelsWarning = NO_RULE; private SplitTransitionProvider splitTransitionProvider; @@ -504,11 +479,11 @@ public final class Attribute implements Comparable { * Defines the configuration transition for this attribute. */ public Builder cfg(SplitTransitionProvider splitTransitionProvider) { - Preconditions.checkState(this.configTransition == ConfigurationTransition.NONE, + Preconditions.checkState(this.configTransition == ConfigurationTransitionProxy.NONE, "the configuration transition is already set"); this.splitTransitionProvider = Preconditions.checkNotNull(splitTransitionProvider); - this.configTransition = ConfigurationTransition.SPLIT; + this.configTransition = ConfigurationTransitionProxy.SPLIT; return this; } @@ -525,9 +500,9 @@ public final class Attribute implements Comparable { * {@code NONE}. */ public Builder cfg(Transition configTransition) { - Preconditions.checkState(this.configTransition == ConfigurationTransition.NONE, + Preconditions.checkState(this.configTransition == ConfigurationTransitionProxy.NONE, "the configuration transition is already set"); - Preconditions.checkArgument(configTransition != ConfigurationTransition.SPLIT, + Preconditions.checkArgument(configTransition != ConfigurationTransitionProxy.SPLIT, "split transitions must be defined using the SplitTransition object"); if (configTransition instanceof SplitTransition) { return cfg((SplitTransition) configTransition); @@ -1829,7 +1804,7 @@ public final class Attribute implements Comparable { ImmutableList> aspects) { Preconditions.checkNotNull(configTransition); Preconditions.checkArgument( - (configTransition == ConfigurationTransition.NONE) + (configTransition == ConfigurationTransitionProxy.NONE) || type.getLabelClass() == LabelClass.DEPENDENCY || type.getLabelClass() == LabelClass.NONDEP_REFERENCE, "Configuration transitions can only be specified for label or label list attributes"); diff --git a/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java b/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java index fdc6061a99..6679f248b3 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java +++ b/src/main/java/com/google/devtools/build/lib/packages/ConfigurationFragmentPolicy.java @@ -18,8 +18,8 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.LinkedHashMultimap; import com.google.common.collect.SetMultimap; +import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy; import com.google.devtools.build.lib.analysis.config.transitions.Transition; -import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import java.util.Collection; import java.util.Set; @@ -82,7 +82,7 @@ public final class ConfigurationFragmentPolicy { *

The value is inherited by subclasses. */ public Builder requiresConfigurationFragments(Collection> configurationFragments) { - requiresConfigurationFragments(ConfigurationTransition.NONE, configurationFragments); + requiresConfigurationFragments(ConfigurationTransitionProxy.NONE, configurationFragments); return this; } @@ -98,7 +98,7 @@ public final class ConfigurationFragmentPolicy { // We can relax this assumption if needed. But it's already sketchy to let a rule see more // than its own configuration. So we don't want to casually proliferate this pattern. Preconditions.checkArgument( - transition == Attribute.ConfigurationTransition.NONE || transition.isHostTransition()); + transition == ConfigurationTransitionProxy.NONE || transition.isHostTransition()); requiredConfigurationFragments.putAll(transition, configurationFragments); return this; } @@ -117,7 +117,7 @@ public final class ConfigurationFragmentPolicy { Collection configurationFragmentNames) { requiresConfigurationFragmentsBySkylarkModuleName( - Attribute.ConfigurationTransition.NONE, configurationFragmentNames); + ConfigurationTransitionProxy.NONE, configurationFragmentNames); return this; } @@ -136,7 +136,7 @@ public final class ConfigurationFragmentPolicy { // We can relax this assumption if needed. But it's already sketchy to let a rule see more // than its own configuration. So we don't want to casually proliferate this pattern. Preconditions.checkArgument( - transition == ConfigurationTransition.NONE || transition.isHostTransition()); + transition == ConfigurationTransitionProxy.NONE || transition.isHostTransition()); requiredConfigurationFragmentNames.putAll(transition, configurationFragmentNames); return this; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImportRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImportRule.java index 6529e5c4fb..9339a5b919 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImportRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcImportRule.java @@ -14,7 +14,7 @@ package com.google.devtools.build.lib.rules.cpp; -import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.DATA; +import static com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy.DATA; import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL; import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java index 90cca62ef2..c46c30e00a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java @@ -34,10 +34,10 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.LanguageDependentFragment.LibraryLanguage; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.analysis.config.PatchTransition; +import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy; import com.google.devtools.build.lib.analysis.config.transitions.Transition; import com.google.devtools.build.lib.analysis.test.InstrumentedFilesCollector.InstrumentationSpec; import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction; import com.google.devtools.build.lib.packages.RuleTransitionFactory; @@ -71,7 +71,7 @@ public class CppRuleClasses { */ public static final ImmutableMap DYNAMIC_TRANSITIONS_MAP = ImmutableMap.of( - Attribute.ConfigurationTransition.DATA, DisableLipoTransition.INSTANCE + ConfigurationTransitionProxy.DATA, DisableLipoTransition.INSTANCE ); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index c2295c4b60..5504107d95 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -73,6 +73,7 @@ import com.google.devtools.build.lib.analysis.config.ConfigurationResolver; import com.google.devtools.build.lib.analysis.config.HostTransition; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.PatchTransition; +import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransitionProxy; import com.google.devtools.build.lib.analysis.config.transitions.Transition; import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget; import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget.DuplicateException; @@ -88,14 +89,11 @@ import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.packages.AspectDescriptor; import com.google.devtools.build.lib.packages.AstParseResult; -import com.google.devtools.build.lib.packages.Attribute; -import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; import com.google.devtools.build.lib.packages.BuildFileName; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.NoSuchThingException; import com.google.devtools.build.lib.packages.Package; -import com.google.devtools.build.lib.packages.Package.Builder; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.RuleVisibility; @@ -1124,8 +1122,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { Transition dataTransition = ((ConfiguredRuleClassProvider) ruleClassProvider) .getDynamicTransitionMapper() - .map(Attribute.ConfigurationTransition.DATA); - BuildOptions dataOptions = dataTransition != Attribute.ConfigurationTransition.NONE + .map(ConfigurationTransitionProxy.DATA); + BuildOptions dataOptions = dataTransition != ConfigurationTransitionProxy.NONE ? ((PatchTransition) dataTransition).apply(firstTargetConfig.getOptions()) : firstTargetConfig.getOptions(); @@ -1593,7 +1591,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { public ConfiguredTarget getConfiguredTargetForTesting( ExtendedEventHandler eventHandler, Label label, BuildConfiguration configuration) { return getConfiguredTargetForTesting( - eventHandler, label, configuration, ConfigurationTransition.NONE); + eventHandler, label, configuration, ConfigurationTransitionProxy.NONE); } /** -- cgit v1.2.3