Factor out BuildConfigurationCollection.Transitions.getDynamicTransition.
This is a legacy dependency on the configuration transition table, which is
only needed for static configurations. Dynamic configurations didn't actually
use anything in that table: this was just a convenience interface that could
have equally been defined somewhere else. So this cl defines it somewhere else.
There's still one last dependency: Transitions.configurationHook. We'll tackle
that in a followup cl.
PiperOrigin-RevId: 161141650
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 8739918..2396c3e 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
@@ -29,14 +29,19 @@
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.PatchTransition;
import com.google.devtools.build.lib.analysis.config.RunUnder;
import com.google.devtools.build.lib.analysis.constraints.EnvironmentRule;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Attribute.LateBoundLabel;
import com.google.devtools.build.lib.packages.Attribute.LateBoundLabelList;
+import com.google.devtools.build.lib.packages.Attribute.Transition;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
@@ -327,4 +332,21 @@
.build();
}
}
+
+ /**
+ * Declares the implementations for {@link Attribute.ConfigurationTransition} 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(
+ Attribute.ConfigurationTransition.NONE, DynamicTransitionMapper.SELF,
+ Attribute.ConfigurationTransition.NULL, DynamicTransitionMapper.SELF,
+ Attribute.ConfigurationTransition.HOST, HostTransition.INSTANCE
+ // Attribute.ConfigurationTransition.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/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index ac5b483..3b7b335 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -31,6 +31,7 @@
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
import com.google.devtools.build.lib.analysis.config.DefaultsPackage;
+import com.google.devtools.build.lib.analysis.config.DynamicTransitionMapper;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
@@ -214,6 +215,8 @@
private final Digraph<Class<? extends RuleDefinition>> dependencyGraph =
new Digraph<>();
private ConfigurationCollectionFactory configurationCollectionFactory;
+ private ImmutableMap.Builder<Attribute.Transition, Attribute.Transition> dynamicTransitionMaps
+ = ImmutableMap.builder();
private Class<? extends BuildConfiguration.Fragment> universalFragment;
private PrerequisiteValidator prerequisiteValidator;
private ImmutableMap.Builder<String, Object> skylarkAccessibleTopLevels =
@@ -320,6 +323,11 @@
return this;
}
+ public Builder addDynamicTransitionMaps(Map<Attribute.Transition, Attribute.Transition> maps) {
+ dynamicTransitionMaps.putAll(maps);
+ return this;
+ }
+
public Builder setUniversalConfigurationFragment(
Class<? extends BuildConfiguration.Fragment> fragment) {
this.universalFragment = fragment;
@@ -429,6 +437,7 @@
ImmutableList.copyOf(configurationOptions),
ImmutableList.copyOf(configurationFragmentFactories),
configurationCollectionFactory,
+ new DynamicTransitionMapper(dynamicTransitionMaps.build()),
universalFragment,
prerequisiteValidator,
skylarkAccessibleTopLevels.build(),
@@ -529,6 +538,11 @@
private final ConfigurationCollectionFactory configurationCollectionFactory;
/**
+ * The dynamic configuration transition mapper.
+ */
+ private final DynamicTransitionMapper dynamicTransitionMapper;
+
+ /**
* A configuration fragment that should be available to all rules even when they don't
* explicitly require it.
*/
@@ -554,6 +568,7 @@
ImmutableList<Class<? extends FragmentOptions>> configurationOptions,
ImmutableList<ConfigurationFragmentFactory> configurationFragments,
ConfigurationCollectionFactory configurationCollectionFactory,
+ DynamicTransitionMapper dynamicTransitionMapper,
Class<? extends BuildConfiguration.Fragment> universalFragment,
PrerequisiteValidator prerequisiteValidator,
ImmutableMap<String, Object> skylarkAccessibleJavaClasses,
@@ -571,6 +586,7 @@
this.configurationOptions = configurationOptions;
this.configurationFragmentFactories = configurationFragments;
this.configurationCollectionFactory = configurationCollectionFactory;
+ this.dynamicTransitionMapper = dynamicTransitionMapper;
this.universalFragment = universalFragment;
this.prerequisiteValidator = prerequisiteValidator;
this.globals = createGlobals(skylarkAccessibleJavaClasses, skylarkModules);
@@ -650,6 +666,13 @@
}
/**
+ * Returns the dynamic configuration transition mapper.
+ */
+ public DynamicTransitionMapper getDynamicTransitionMapper() {
+ return dynamicTransitionMapper;
+ }
+
+ /**
* Returns the configuration fragment that should be available to all rules even when they
* don't explicitly require it.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 8c217af..4e33c0f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -1204,7 +1204,7 @@
private final ImmutableMap<Class<? extends Fragment>, Fragment> fragments;
private final ImmutableMap<String, Class<? extends Fragment>> skylarkVisibleFragments;
private final RepositoryName mainRepositoryName;
-
+ private final DynamicTransitionMapper dynamicTransitionMapper;
/**
* Directories in the output tree.
@@ -1516,11 +1516,15 @@
/**
* Constructs a new BuildConfiguration instance.
+ *
+ * <p>Callers that pass null for {@code dynamicTransitionMapper} should not use dynamic
+ * configurations.
*/
public BuildConfiguration(BlazeDirectories directories,
Map<Class<? extends Fragment>, Fragment> fragmentsMap,
BuildOptions buildOptions,
- String repositoryName) {
+ String repositoryName,
+ @Nullable DynamicTransitionMapper dynamicTransitionMapper) {
this.directories = directories;
this.fragments = ImmutableSortedMap.copyOf(fragmentsMap, lexicalFragmentSorter);
@@ -1530,6 +1534,7 @@
this.actionsEnabled = buildOptions.enableActions();
this.options = buildOptions.get(Options.class);
this.mainRepositoryName = RepositoryName.createFromValidStrippedName(repositoryName);
+ this.dynamicTransitionMapper = dynamicTransitionMapper;
// We can't use an ImmutableMap.Builder here; we need the ability to add entries with keys that
// are already in the map so that the same define can be specified on the command line twice,
@@ -1612,8 +1617,13 @@
}
BuildOptions options = buildOptions.trim(
getOptionsClasses(fragmentsMap.keySet(), ruleClassProvider));
- BuildConfiguration newConfig = new BuildConfiguration(
- directories, fragmentsMap, options, mainRepositoryName.strippedName());
+ BuildConfiguration newConfig =
+ new BuildConfiguration(
+ directories,
+ fragmentsMap,
+ options,
+ mainRepositoryName.strippedName(),
+ dynamicTransitionMapper);
newConfig.setConfigurationTransitions(this.transitions);
return newConfig;
}
@@ -1777,12 +1787,6 @@
*/
public interface TransitionApplier {
/**
- * Creates a new instance of this transition applier bound to the specified source
- * configuration.
- */
- TransitionApplier create(BuildConfiguration config);
-
- /**
* Accepts the given configuration transition. The implementation decides how to turn
* this into an actual configuration. This may be called multiple times (representing a
* request for a sequence of transitions).
@@ -1838,11 +1842,6 @@
}
@Override
- public TransitionApplier create(BuildConfiguration configuration) {
- return new StaticTransitionApplier(configuration);
- }
-
- @Override
public void applyTransition(Transition transition) {
if (transition == Attribute.ConfigurationTransition.NULL) {
toConfigurations = Lists.<BuildConfiguration>asList(null, new BuildConfiguration[0]);
@@ -1922,8 +1921,8 @@
* transitions that the caller subsequently creates configurations from.
*/
private static class DynamicTransitionApplier implements TransitionApplier {
- private final BuildOptions originalOptions;
private final Transitions transitionsManager;
+ private final DynamicTransitionMapper dynamicTransitionMapper;
private boolean splitApplied = false;
// The transition this applier applies to dep rules. When multiple transitions are requested,
@@ -1931,14 +1930,10 @@
// so calling code doesn't need special logic to support combinations.
private Transition currentTransition = Attribute.ConfigurationTransition.NONE;
- private DynamicTransitionApplier(BuildConfiguration originalConfiguration) {
- this.originalOptions = originalConfiguration.getOptions();
- this.transitionsManager = originalConfiguration.getTransitions();
- }
-
- @Override
- public TransitionApplier create(BuildConfiguration configuration) {
- return new DynamicTransitionApplier(configuration);
+ private DynamicTransitionApplier(Transitions transitionsManager,
+ DynamicTransitionMapper dynamicTransitionMapper) {
+ this.transitionsManager = transitionsManager;
+ this.dynamicTransitionMapper = dynamicTransitionMapper;
}
/**
@@ -1973,10 +1968,9 @@
// in the last segment of a ComposingSplitTransition, those optimizations wouldn't trigger.
return HostTransition.INSTANCE;
}
+
// TODO(gregce): remove this dynamic transition mapping when static configs are removed.
- Transition dynamicTransition = (transition2 instanceof PatchTransition)
- ? transition2
- : transitionsManager.getDynamicTransition(transition2);
+ Transition dynamicTransition = dynamicTransitionMapper.map(transition2);
return transition1 == Attribute.ConfigurationTransition.NONE
? dynamicTransition
: new ComposingSplitTransition(transition1, dynamicTransition);
@@ -2101,7 +2095,7 @@
*/
public TransitionApplier getTransitionApplier() {
return useDynamicConfigurations()
- ? new DynamicTransitionApplier(this)
+ ? new DynamicTransitionApplier(this.getTransitions(), dynamicTransitionMapper)
: new StaticTransitionApplier(this);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java
index 935fb97..1137280 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationCollection.java
@@ -260,17 +260,11 @@
* in which they must co-exist. Once dynamic configurations are production-ready, we'll remove
* the static configuration code entirely.
*/
+ @Deprecated
public Transition getDynamicTransition(Transition transition) {
- Preconditions.checkState(configuration.useDynamicConfigurations());
- if (transition == Attribute.ConfigurationTransition.NONE) {
- return transition;
- } else if (transition == Attribute.ConfigurationTransition.NULL) {
- return transition;
- } else if (transition == Attribute.ConfigurationTransition.HOST) {
- return HostTransition.INSTANCE;
- } else {
- throw new UnsupportedOperationException("No dynamic mapping for " + transition.toString());
- }
+ // Keep this interface for now because some other dead code is still calling it.
+ throw new UnsupportedOperationException(
+ "This interface is no longer supported and will be removed soon.");
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java
index 3a95a4b..0080786 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java
@@ -110,7 +110,7 @@
return null;
}
- result = new BuildConfiguration(directories, fragments, buildOptions, repositoryName);
+ result = new BuildConfiguration(directories, fragments, buildOptions, repositoryName, null);
cache.put(cacheKey, result);
return result;
}
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
new file mode 100644
index 0000000..1f3a78b
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/DynamicTransitionMapper.java
@@ -0,0 +1,88 @@
+// 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;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.Attribute.Transition;
+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 Attribute.ConfigurationTransition}, which defines its transitions as
+ * enums. These transitions are used all over the place. So we need a way to continue to support
+ * them.
+ *
+ * <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 Attribute.ConfigurationTransition#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.
+ */
+public final class DynamicTransitionMapper {
+ /**
+ * Use this to declare a no-op transition that keeps the input configuration.
+ */
+ public static final Transition SELF = () -> {
+ throw new UnsupportedOperationException("This is just an alias for \"keep the input "
+ + "configuration\". It shouldn't actually apply a real transition");
+ };
+
+ private final ImmutableMap<Transition, Transition> map;
+
+ /**
+ * Creates a new mapper with the given mapping. Any transition not in this mapping triggers
+ * an {@link IllegalArgumentException}.
+ */
+ public DynamicTransitionMapper(ImmutableMap<Transition, Transition> map) {
+ this.map = map;
+ }
+
+ /**
+ * Given an input transition, returns the equivalent transition Blaze's implementation logic knows
+ * how to apply.
+ *
+ * <p>When the input is a {@link PatchTransition}, this just returns the input. This is because
+ * that's the kind of transition that Blaze natively applies. For this reason, all inputs should
+ * ideally be {@link PatchTransition}s.
+ *
+ * <p>Non-{@link PatchTransition} inputs that aren't mapped here throw an
+ * {@link IllegalArgumentException}.
+ */
+ public Transition map(Transition fromTransition) {
+ if (fromTransition instanceof PatchTransition) {
+ return fromTransition;
+ }
+ Transition toTransition = map.get(fromTransition);
+ if (toTransition == SELF) {
+ return fromTransition;
+ } else if (toTransition != null) {
+ return toTransition;
+ }
+ throw new IllegalArgumentException("No dynamic mapping for " + fromTransition.toString());
+ }
+}
