From e0bbe757b3626fbbadbab798e6752ca086e9ff46 Mon Sep 17 00:00:00 2001 From: gregce Date: Tue, 12 Sep 2017 23:58:34 +0200 Subject: Remove outdated references to static vs. dynamic configurations. PiperOrigin-RevId: 168452997 --- .../devtools/build/lib/analysis/Dependency.java | 96 +++++++++++----------- .../build/lib/skyframe/AspectFunction.java | 4 +- .../devtools/build/lib/skyframe/AspectValue.java | 6 +- .../lib/skyframe/ConfiguredTargetFunction.java | 6 +- .../build/lib/skyframe/SkyframeBuildView.java | 4 +- .../build/lib/skyframe/SkyframeExecutor.java | 6 +- 6 files changed, 59 insertions(+), 63 deletions(-) (limited to 'src/main/java/com/google') diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java b/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java index 9b67fe8583..f548b1860a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/Dependency.java @@ -26,11 +26,13 @@ import javax.annotation.Nullable; /** * A dependency of a configured target through a label. * - *

For static configurations: includes the target and the configuration of the dependency + *

The dep's configuration can be specified in one of two ways: + * + *

Explicit configurations: includes the target and the configuration of the dependency * configured target and any aspects that may be required, as well as the configurations for * these aspects. * - *

For dynamic configurations: includes the target and the desired configuration transitions + *

Configuration transitions: includes the target and the desired configuration transitions * that should be applied to produce the dependency's configuration. It's the caller's * responsibility to construct an actual configuration out of that. A set of aspects is also * included; the caller must also construct configurations for each of these. @@ -45,30 +47,29 @@ public abstract class Dependency { /** * Creates a new {@link Dependency} with a null configuration, suitable for edges with no - * configuration in static configuration builds. + * configuration. */ public static Dependency withNullConfiguration(Label label) { return new NullConfigurationDependency(label); } /** - * Creates a new {@link Dependency} with the given configuration, suitable for static - * configuration builds. + * Creates a new {@link Dependency} with the given explicit configuration. * *

The configuration must not be {@code null}. * *

A {@link Dependency} created this way will have no associated aspects. */ public static Dependency withConfiguration(Label label, BuildConfiguration configuration) { - return new StaticConfigurationDependency( + return new ExplicitConfigurationDependency( label, configuration, AspectCollection.EMPTY, ImmutableMap.of()); } /** - * Creates a new {@link Dependency} with the given configuration and aspects, suitable for - * static configuration builds. The configuration is also applied to all aspects. + * Creates a new {@link Dependency} with the given configuration and aspects. The configuration + * is also applied to all aspects. * *

The configuration and aspects must not be {@code null}. */ @@ -79,12 +80,13 @@ public abstract class Dependency { for (AspectDescriptor aspect : aspects.getAllAspects()) { aspectBuilder.put(aspect, configuration); } - return new StaticConfigurationDependency(label, configuration, aspects, aspectBuilder.build()); + return new ExplicitConfigurationDependency(label, configuration, aspects, + aspectBuilder.build()); } /** * Creates a new {@link Dependency} with the given configuration and aspects, suitable for - * storing the output of a dynamic configuration trimming step. The aspects each have their own + * storing the output of a configuration trimming step. The aspects each have their own * configuration. * *

The aspects and configurations must not be {@code null}. @@ -93,17 +95,16 @@ public abstract class Dependency { Label label, BuildConfiguration configuration, AspectCollection aspects, Map aspectConfigurations) { - return new StaticConfigurationDependency( + return new ExplicitConfigurationDependency( label, configuration, aspects, ImmutableMap.copyOf(aspectConfigurations)); } /** - * Creates a new {@link Dependency} with the given transition and aspects, suitable for dynamic - * configuration builds. + * Creates a new {@link Dependency} with the given transition and aspects. */ public static Dependency withTransitionAndAspects( Label label, Attribute.Transition transition, AspectCollection aspects) { - return new DynamicConfigurationDependency(label, transition, aspects); + return new ConfigurationTransitionDependency(label, transition, aspects); } protected final Label label; @@ -122,23 +123,23 @@ public abstract class Dependency { } /** - * Returns true if this dependency specifies a static configuration, false if it specifies - * a dynamic transition. + * Returns true if this dependency specifies an explicit configuration, false if it specifies + * a configuration transition. */ - public abstract boolean hasStaticConfiguration(); + public abstract boolean hasExplicitConfiguration(); /** - * Returns the static configuration for the target this dependency points to. + * Returns the explicit configuration intended for this dependency. * - * @throws IllegalStateException if {@link #hasStaticConfiguration} returns false. + * @throws IllegalStateException if {@link #hasExplicitConfiguration} returns false. */ @Nullable public abstract BuildConfiguration getConfiguration(); /** - * Returns the dynamic transition to be applied to reach the target this dependency points to. + * Returns the configuration transition to apply to reach the target this dependency points to. * - * @throws IllegalStateException if {@link #hasStaticConfiguration} returns true. + * @throws IllegalStateException if {@link #hasExplicitConfiguration} returns true. */ public abstract Attribute.Transition getTransition(); @@ -151,15 +152,15 @@ public abstract class Dependency { public abstract AspectCollection getAspects(); /** - * Returns the the static configuration an aspect should be evaluated with + * Returns the configuration an aspect should be evaluated with ** - * @throws IllegalStateException if {@link #hasStaticConfiguration()} returns false. + * @throws IllegalStateException if {@link #hasExplicitConfiguration()} returns false. */ public abstract BuildConfiguration getAspectConfiguration(AspectDescriptor aspect); /** - * Implementation of a dependency with no configuration, suitable for static configuration - * builds of edges to source files or e.g. for visibility. + * Implementation of a dependency with no configuration, suitable for, e.g., source files or + * visibility. */ private static final class NullConfigurationDependency extends Dependency { public NullConfigurationDependency(Label label) { @@ -167,7 +168,7 @@ public abstract class Dependency { } @Override - public boolean hasStaticConfiguration() { + public boolean hasExplicitConfiguration() { return true; } @@ -180,7 +181,7 @@ public abstract class Dependency { @Override public Attribute.Transition getTransition() { throw new IllegalStateException( - "A dependency with a static configuration does not have a transition."); + "This dependency has an explicit configuration, not a transition."); } @Override @@ -214,15 +215,14 @@ public abstract class Dependency { } /** - * Implementation of a dependency with static configurations, suitable for static configuration - * builds. + * Implementation of a dependency with an explicitly set configuration. */ - private static final class StaticConfigurationDependency extends Dependency { + private static final class ExplicitConfigurationDependency extends Dependency { private final BuildConfiguration configuration; private final AspectCollection aspects; private final ImmutableMap aspectConfigurations; - public StaticConfigurationDependency( + public ExplicitConfigurationDependency( Label label, BuildConfiguration configuration, AspectCollection aspects, ImmutableMap aspectConfigurations) { @@ -233,7 +233,7 @@ public abstract class Dependency { } @Override - public boolean hasStaticConfiguration() { + public boolean hasExplicitConfiguration() { return true; } @@ -245,7 +245,7 @@ public abstract class Dependency { @Override public Attribute.Transition getTransition() { throw new IllegalStateException( - "A dependency with a static configuration does not have a transition."); + "This dependency has an explicit configuration, not a transition."); } @Override @@ -265,10 +265,10 @@ public abstract class Dependency { @Override public boolean equals(Object other) { - if (!(other instanceof StaticConfigurationDependency)) { + if (!(other instanceof ExplicitConfigurationDependency)) { return false; } - StaticConfigurationDependency otherDep = (StaticConfigurationDependency) other; + ExplicitConfigurationDependency otherDep = (ExplicitConfigurationDependency) other; return label.equals(otherDep.label) && configuration.equals(otherDep.configuration) && aspects.equals(otherDep.aspects) @@ -278,20 +278,19 @@ public abstract class Dependency { @Override public String toString() { return String.format( - "StaticConfigurationDependency{label=%s, configuration=%s, aspectConfigurations=%s}", - label, configuration, aspectConfigurations); + "%s{label=%s, configuration=%s, aspectConfigurations=%s}", + getClass().getSimpleName(), label, configuration, aspectConfigurations); } } /** - * Implementation of a dependency with a given configuration transition, suitable for dynamic - * configuration builds. + * Implementation of a dependency with a given configuration transition. */ - private static final class DynamicConfigurationDependency extends Dependency { + private static final class ConfigurationTransitionDependency extends Dependency { private final Attribute.Transition transition; private final AspectCollection aspects; - public DynamicConfigurationDependency( + public ConfigurationTransitionDependency( Label label, Attribute.Transition transition, AspectCollection aspects) { super(label); this.transition = Preconditions.checkNotNull(transition); @@ -299,14 +298,14 @@ public abstract class Dependency { } @Override - public boolean hasStaticConfiguration() { + public boolean hasExplicitConfiguration() { return false; } @Override public BuildConfiguration getConfiguration() { throw new IllegalStateException( - "A dependency with a dynamic configuration transition does not have a configuration."); + "This dependency has a transition, not an explicit configuration."); } @Override @@ -322,8 +321,7 @@ public abstract class Dependency { @Override public BuildConfiguration getAspectConfiguration(AspectDescriptor aspect) { throw new IllegalStateException( - "A dependency with a dynamic configuration transition does not have aspect " - + "configurations."); + "This dependency has a transition, not an explicit aspect configuration."); } @Override @@ -333,10 +331,10 @@ public abstract class Dependency { @Override public boolean equals(Object other) { - if (!(other instanceof DynamicConfigurationDependency)) { + if (!(other instanceof ConfigurationTransitionDependency)) { return false; } - DynamicConfigurationDependency otherDep = (DynamicConfigurationDependency) other; + ConfigurationTransitionDependency otherDep = (ConfigurationTransitionDependency) other; return label.equals(otherDep.label) && transition.equals(otherDep.transition) && aspects.equals(otherDep.aspects); @@ -345,8 +343,8 @@ public abstract class Dependency { @Override public String toString() { return String.format( - "DynamicConfigurationDependency{label=%s, transition=%s, aspects=%s}", - label, transition, aspects); + "%s{label=%s, transition=%s, aspects=%s}", + getClass().getSimpleName(), label, transition, aspects); } } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java index ce8405bafb..7b3f7253a2 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java @@ -255,8 +255,8 @@ public final class AspectFunction implements SkyFunction { // When getting the dependencies of this hybrid aspect+base target, use the aspect's // configuration. The configuration of the aspect will always be a superset of the target's - // (dynamic configuration mode: target is part of the aspect's config fragment requirements; - // static configuration mode: target is the same configuration as the aspect), so the fragments + // (trimmed configuration mode: target is part of the aspect's config fragment requirements; + // untrimmed mode: target is the same configuration as the aspect), so the fragments // required by all dependencies (both those of the aspect and those of the base target) // will be present this way. TargetAndConfiguration originalTargetAndAspectConfiguration = diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java index 0656d45dfb..ee23308d07 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java @@ -112,7 +112,7 @@ public final class AspectValue extends ActionLookupValue { /** * Returns the configuration to be used for the evaluation of the aspect itself. * - *

In dynamic configuration mode, the aspect may require more fragments than the target on + *

In trimmed configuration mode, the aspect may require more fragments than the target on * which it is being evaluated; in addition to configuration fragments required by the target * and its dependencies, an aspect has configuration fragment requirements of its own, as well * as dependencies of its own with their own configuration fragment requirements. @@ -122,7 +122,7 @@ public final class AspectValue extends ActionLookupValue { * configurations trimmed from this one as normal. * *

Because of these properties, this configuration is always a superset of that returned by - * {@link #getBaseConfiguration()}. In static configuration mode, this configuration will be + * {@link #getBaseConfiguration()}. In untrimmed configuration mode, this configuration will be * equivalent to that returned by {@link #getBaseConfiguration()}. * * @see #getBaseConfiguration() @@ -134,7 +134,7 @@ public final class AspectValue extends ActionLookupValue { /** * Returns the configuration to be used for the base target. * - *

In dynamic configuration mode, the configured target this aspect is attached to may have + *

In trimmed configuration mode, the configured target this aspect is attached to may have * a different configuration than the aspect itself (see the documentation for * {@link #getAspectConfiguration()} for an explanation why). The base configuration is the one * used to construct a key to look up the base configured target. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java index 2b18dc8b01..ce60f42cfe 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java @@ -531,7 +531,7 @@ public final class ConfiguredTargetFunction implements SkyFunction { * Creates a dynamic configuration for each dep that's custom-fitted specifically for that dep. * *

More specifically: given a set of {@link Dependency} instances holding dynamic config - * transition requests (e.g. {@link Dependency#hasStaticConfiguration()} == false}), returns + * transition requests (e.g. {@link Dependency#hasExplicitConfiguration()} == false}), returns * equivalent dependencies containing dynamically created configurations applying those * transitions. If {@link BuildConfiguration.Options#trimConfigurations()} is true, these * configurations only contain the fragments needed by the dep and its transitive closure. Else @@ -618,7 +618,7 @@ public final class ConfiguredTargetFunction implements SkyFunction { // // A *lot* of targets have null deps, so this produces real savings. Profiling tests over a // simple cc_binary show this saves ~1% of total analysis phase time. - if (dep.hasStaticConfiguration()) { + if (dep.hasExplicitConfiguration()) { continue; } @@ -869,7 +869,7 @@ public final class ConfiguredTargetFunction implements SkyFunction { OrderedSetMultimap result = OrderedSetMultimap.create(); for (Map.Entry depsEntry : originalDeps.entries()) { AttributeAndLabel attrAndLabel = iterator.next(); - if (depsEntry.getValue().hasStaticConfiguration()) { + if (depsEntry.getValue().hasExplicitConfiguration()) { result.put(attrAndLabel.attribute, depsEntry.getValue()); } else { Collection dynamicAttrDeps = dynamicDeps.get(attrAndLabel); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java index 6a472b7839..fb18457312 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java @@ -505,8 +505,6 @@ public final class SkyframeBuildView { * Returns the host configuration trimmed to the same fragments as the input configuration. If * the input is null, returns the top-level host configuration. * - *

For static configurations, this unconditionally returns the (sole) top-level configuration. - * *

This may only be called after {@link #setTopLevelHostConfiguration} has set the * correct host configuration at the top-level. */ @@ -531,7 +529,7 @@ public final class SkyframeBuildView { Set> fragmentClasses = config.trimConfigurations() ? config.fragmentClasses() - : ((ConfiguredRuleClassProvider) ruleClassProvider).getAllFragments(); + : ruleClassProvider.getAllFragments(); BuildConfiguration hostConfig = hostConfigurationCache.get(fragmentClasses); if (hostConfig != null) { return hostConfig; 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 8ddecaf657..fafe911823 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 @@ -1435,7 +1435,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { Map>> fragmentsMap = new HashMap<>(); Set