Remove BuildConfiguration.useDynamicConfigurations.
This is always true.
Part of the static config cleanup effort.
PiperOrigin-RevId: 165628823
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 3ae5a8a..2644780 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
@@ -875,10 +875,7 @@
nodes.add(new TargetAndConfiguration(target, target.isConfigurable() ? config : null));
}
}
- return ImmutableList.copyOf(
- configurations.useDynamicConfigurations()
- ? getDynamicConfigurations(nodes, eventHandler)
- : nodes);
+ return ImmutableList.copyOf(getDynamicConfigurations(nodes, eventHandler));
}
/**
@@ -960,7 +957,6 @@
DynamicTransitionMapper dynamicTransitionMapper) {
Target target = targetAndConfig.getTarget();
BuildConfiguration fromConfig = targetAndConfig.getConfiguration();
- Preconditions.checkArgument(fromConfig.useDynamicConfigurations());
// Top-level transitions (chosen by configuration fragments):
Transition topLevelTransition = fromConfig.topLevelConfigurationHook(target);
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 c6f56b8..ee6e256 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
@@ -769,7 +769,7 @@
AspectCollection aspects =
requiredAspects(this.aspects, attributeAndOwner, toTarget, rule);
Dependency dep;
- if (config.useDynamicConfigurations() && !applyNullTransition) {
+ if (!applyNullTransition) {
// Pass a transition rather than directly feeding the configuration so deps get trimmed.
dep = Dependency.withTransitionAndAspects(
depLabel, new FixedTransition(config.getOptions()), aspects);
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 c5f59d6..5165403 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
@@ -86,12 +86,9 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import java.util.Queue;
import java.util.Set;
import java.util.TreeMap;
import javax.annotation.Nullable;
@@ -1094,8 +1091,6 @@
private final String checksum;
- private Set<BuildConfiguration> allReachableConfigurations;
-
private final ImmutableMap<Class<? extends Fragment>, Fragment> fragments;
private final ImmutableMap<String, Class<? extends Fragment>> skylarkVisibleFragments;
private final RepositoryName mainRepositoryName;
@@ -1266,20 +1261,6 @@
if (this == other) {
return true;
}
- if (!useDynamicConfigurations()) {
- // Static configurations aren't safe for value equality because they include transition
- // references to other configurations (see setConfigurationTransitions). For example, imagine
- // in one build target config A has a reference to data config B. Now imagine a second build
- // where target config C has a reference to data config D. If A and B are value-equal, that
- // means a call to ConfiguredTargetKey("//foo", C) might return the SkyKey for ("//foo", A).
- // This is not just possible but *likely* due to SkyKey interning (see
- // SkyKey.SKY_KEY_INTERNER). This means a data transition on that config could incorrectly
- // return B, which is not safe because B is not necessarily value-equal to D.
- //
- // This becomes safe with dynamic configurations: transitions are completely triggered by
- // external logic and configs have no awareness of them at all.
- return false;
- }
if (!(other instanceof BuildConfiguration)) {
return false;
}
@@ -1295,9 +1276,6 @@
@Override
public int hashCode() {
- if (!useDynamicConfigurations()) {
- return BuildConfiguration.super.hashCode();
- }
return hashCode;
}
@@ -1596,38 +1574,6 @@
}
/**
- * For static configurations, returns all configurations that can be reached from this one through
- * any kind of configuration transition.
- *
- * <p>For dynamic configurations, returns the current configuration (since configurations aren't
- * reached through other configurations).
- */
- public synchronized Collection<BuildConfiguration> getAllReachableConfigurations() {
- if (allReachableConfigurations == null) {
- // This is needed for every configured target in skyframe m2, so we cache it.
- // We could alternatively make the corresponding dependencies into a skyframe node.
- this.allReachableConfigurations = computeAllReachableConfigurations();
- }
- return allReachableConfigurations;
- }
-
- private Set<BuildConfiguration> computeAllReachableConfigurations() {
- if (useDynamicConfigurations()) {
- return ImmutableSet.of(this);
- }
- Set<BuildConfiguration> result = new LinkedHashSet<>();
- Queue<BuildConfiguration> queue = new LinkedList<>();
- queue.add(this);
- while (!queue.isEmpty()) {
- BuildConfiguration config = queue.remove();
- if (!result.add(config)) {
- continue;
- }
- }
- return result;
- }
-
- /**
* A common interface for static vs. dynamic configuration implementations that allows
* common configuration and transition-selection logic to seamlessly work with either.
*
@@ -1893,9 +1839,7 @@
* Returns the {@link TransitionApplier} that should be passed to {#evaluateTransition} calls.
*/
public TransitionApplier getTransitionApplier() {
- return useDynamicConfigurations()
- ? new DynamicTransitionApplier(dynamicTransitionMapper)
- : new StaticTransitionApplier(this);
+ return new DynamicTransitionApplier(dynamicTransitionMapper);
}
/**
@@ -2409,17 +2353,6 @@
}
/**
- * Returns whether we should use dynamically instantiated build configurations vs. static
- * configurations (e.g. predefined in {@link
- * com.google.devtools.build.lib.analysis.ConfigurationCollectionFactory}).
- */
- @Deprecated
- public boolean useDynamicConfigurations() {
- // TODO(gregce): remove this interface, which is now redundant.
- return true;
- }
-
- /**
* Returns whether we should trim dynamic configurations to only include the fragments needed
* to correctly analyze a rule.
*/
@@ -2516,7 +2449,6 @@
*/
@Nullable
public PatchTransition getArtifactOwnerTransition() {
- Preconditions.checkState(useDynamicConfigurations());
PatchTransition ownerTransition = null;
for (Fragment fragment : fragments.values()) {
PatchTransition fragmentTransition = fragment.getArtifactOwnerTransition();
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 72a9e3d..3f0d4b2 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
@@ -17,12 +17,9 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import java.io.Serializable;
-import java.util.Collection;
import java.util.HashMap;
-import java.util.LinkedHashSet;
import java.util.List;
import java.util.Objects;
-import java.util.Set;
/**
* The primary container for all main {@link BuildConfiguration} instances,
@@ -51,7 +48,7 @@
// Except for the host configuration (which may be identical across target configs), the other
// configurations must all have different cache keys or we will end up with problems.
HashMap<String, BuildConfiguration> cacheKeyConflictDetector = new HashMap<>();
- for (BuildConfiguration config : getAllConfigurations()) {
+ for (BuildConfiguration config : targetConfigurations) {
String cacheKey = config.checksum();
if (cacheKeyConflictDetector.containsKey(cacheKey)) {
throw new InvalidConfigurationException("Conflicting configurations: " + config + " & "
@@ -77,28 +74,6 @@
return hostConfiguration;
}
- /**
- * For static configurations, returns all configurations that can be reached from the target
- * configurations through any kind of configuration transition.
- *
- * <p>For dynamic configurations, returns the target configurations (since configurations aren't
- * reached through other configurations).
- */
- public Collection<BuildConfiguration> getAllConfigurations() {
- Set<BuildConfiguration> result = new LinkedHashSet<>();
- for (BuildConfiguration config : targetConfigurations) {
- result.addAll(config.getAllReachableConfigurations());
- }
- return result;
- }
-
- /**
- * Returns whether this build uses dynamic configurations.
- */
- public boolean useDynamicConfigurations() {
- return getTargetConfigurations().get(0).useDynamicConfigurations();
- }
-
@Override
public boolean equals(Object obj) {
if (this == obj) {
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 87de6cf..214aa02 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
@@ -148,10 +148,6 @@
this.removeActionsAfterEvaluation = Preconditions.checkNotNull(removeActionsAfterEvaluation);
}
- private static boolean useDynamicConfigurations(BuildConfiguration config) {
- return config != null && config.useDynamicConfigurations();
- }
-
@Override
public SkyValue compute(SkyKey key, Environment env) throws ConfiguredTargetFunctionException,
InterruptedException {
@@ -197,7 +193,7 @@
// associates the corresponding error with this target, as expected. Without this line,
// the first TransitiveTargetValue call happens on its dep (in trimConfigurations), so Bazel
// associates the error with the dep, which is misleading.
- if (useDynamicConfigurations(configuration) && configuration.trimConfigurations()
+ if (configuration != null && configuration.trimConfigurations()
&& env.getValue(TransitiveTargetValue.key(lc.getLabel())) == null) {
return null;
}
@@ -402,8 +398,8 @@
}
// Trim each dep's configuration so it only includes the fragments needed by its transitive
- // closure (only dynamic configurations support this).
- if (useDynamicConfigurations(ctgValue.getConfiguration())) {
+ // closure.
+ if (ctgValue.getConfiguration() != null) {
depValueNames = getDynamicConfigurations(env, ctgValue, depValueNames, hostConfiguration,
ruleClassProvider);
// It's important that we don't use "if (env.missingValues()) { return null }" here (or
@@ -754,7 +750,6 @@
@Nullable
private static Set<Class<? extends BuildConfiguration.Fragment>> getTransitiveFragments(
Environment env, Label dep, BuildConfiguration parentConfig) throws InterruptedException {
- Preconditions.checkArgument(parentConfig.useDynamicConfigurations());
if (!parentConfig.trimConfigurations()) {
return parentConfig.getAllFragments().keySet();
}
@@ -1067,20 +1062,16 @@
return null;
}
-
// No need to get new configs from Skyframe - config_setting rules always use the current
// target's config.
// TODO(bazel-team): remove the need for this special transformation. We can probably do this by
// simply passing this through trimConfigurations.
- BuildConfiguration targetConfig = ctgValue.getConfiguration();
- if (useDynamicConfigurations(targetConfig)) {
- ImmutableList.Builder<Dependency> staticConfigs = ImmutableList.builder();
- for (Dependency dep : configValueNames) {
- staticConfigs.add(
- Dependency.withConfigurationAndAspects(dep.getLabel(), targetConfig, dep.getAspects()));
- }
- configValueNames = staticConfigs.build();
+ ImmutableList.Builder<Dependency> staticConfigs = ImmutableList.builder();
+ for (Dependency dep : configValueNames) {
+ staticConfigs.add(Dependency.withConfigurationAndAspects(dep.getLabel(),
+ ctgValue.getConfiguration(), dep.getAspects()));
}
+ configValueNames = staticConfigs.build();
Map<SkyKey, ConfiguredTarget> configValues = resolveConfiguredTargetDependencies(
env, configValueNames, transitivePackages, transitiveLoadingRootCauses);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java
index 7789164..b064b5b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java
@@ -108,7 +108,7 @@
deps =
resolver.dependentNodeMap(
ctgValue, hostConfiguration, /*aspect=*/ null, configConditions);
- if (ct.getConfiguration() != null && ct.getConfiguration().useDynamicConfigurations()) {
+ if (ct.getConfiguration() != null) {
deps = ConfiguredTargetFunction.getDynamicConfigurations(env, ctgValue, deps,
hostConfiguration, ruleClassProvider);
}
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 4f1d96e..90773dc 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
@@ -512,7 +512,7 @@
* correct host configuration at the top-level.
*/
public BuildConfiguration getHostConfiguration(BuildConfiguration config) {
- if (config == null || !config.useDynamicConfigurations()) {
+ if (config == null) {
return topLevelHostConfiguration;
}
// TODO(bazel-team): have the fragment classes be those required by the consuming target's
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 f119ecc..f61ba9c 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
@@ -1571,8 +1571,7 @@
}
/**
- * Returns a particular configured target. If dynamic configurations are active, applies the given
- * configuration transition.
+ * Returns a particular configured target after applying the given transition.
*/
@VisibleForTesting
@Nullable
@@ -1581,35 +1580,19 @@
Label label,
BuildConfiguration configuration,
Attribute.Transition transition) {
-
- if (configuration != null && !configuration.useDynamicConfigurations()
- && transition != ConfigurationTransition.NONE) {
- // It's illegal to apply this transition over a statically configured build. But C++ LIPO
- // support works by applying a rule configurator for static configurations and a rule
- // transition applier for dynamic configurations. Dynamically configured builds skip
- // the configurator and this code makes statically configured builds skip the rule transition
- // applier.
- //
- // This will all get a lot simpler once static configurations are removed entirely.
- transition = ConfigurationTransition.NONE;
- }
-
if (memoizingEvaluator.getExistingValueForTesting(
PrecomputedValue.WORKSPACE_STATUS_KEY.getKeyForTesting()) == null) {
injectWorkspaceStatusData(label.getWorkspaceRoot());
}
- Dependency dep;
- if (configuration == null) {
- dep = Dependency.withNullConfiguration(label);
- } else if (configuration.useDynamicConfigurations()) {
- dep = Dependency.withTransitionAndAspects(label, transition, AspectCollection.EMPTY);
- } else {
- dep = Dependency.withConfiguration(label, configuration);
- }
-
return Iterables.getFirst(
- getConfiguredTargets(eventHandler, configuration, ImmutableList.of(dep), false),
+ getConfiguredTargets(
+ eventHandler,
+ configuration,
+ ImmutableList.of(configuration == null
+ ? Dependency.withNullConfiguration(label)
+ : Dependency.withTransitionAndAspects(label, transition, AspectCollection.EMPTY)),
+ false),
null);
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
index 72c18f2..183c8c0 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -17,7 +17,6 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.devtools.build.lib.packages.Attribute.attr;
-import static com.google.devtools.build.lib.testutil.MoreAsserts.assertEventCount;
import static com.google.devtools.build.lib.testutil.MoreAsserts.assertEventCountAtLeast;
import static org.junit.Assert.fail;
@@ -393,21 +392,12 @@
Iterable<Dependency> targets = getView().getDirectPrerequisiteDependenciesForTesting(
reporter, top, getBuildConfigurationCollection()).values();
- Dependency innerDependency;
- Dependency fileDependency;
- if (top.getConfiguration().useDynamicConfigurations()) {
- innerDependency =
- Dependency.withTransitionAndAspects(
- Label.parseAbsolute("//package:inner"),
- Attribute.ConfigurationTransition.NONE,
- AspectCollection.EMPTY);
- } else {
- innerDependency =
- Dependency.withConfiguration(
- Label.parseAbsolute("//package:inner"),
- getTargetConfiguration());
- }
- fileDependency =
+ Dependency innerDependency =
+ Dependency.withTransitionAndAspects(
+ Label.parseAbsolute("//package:inner"),
+ Attribute.ConfigurationTransition.NONE,
+ AspectCollection.EMPTY);
+ Dependency fileDependency =
Dependency.withNullConfiguration(
Label.parseAbsolute("//package:file"));
@@ -541,11 +531,9 @@
// the transitive target closure) and in the normal configured target cycle detection path.
// So we get an additional instance of this check (which varies depending on whether Skyframe
// loading phase is enabled).
- // TODO(gregce): refactor away this variation. Note that the duplicate doesn't make it into
+ // TODO(gregce): Fix above and uncomment the below. Note that the duplicate doesn't make it into
// real user output (it only affects tests).
- if (!getTargetConfiguration().useDynamicConfigurations()) {
- assertEventCount(3, eventCollector);
- }
+ // assertEventCount(3, eventCollector);
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index ff655ba..5c88265 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -286,7 +286,7 @@
throws InterruptedException {
BuildConfiguration targetConfig =
Iterables.getOnlyElement(masterConfig.getTargetConfigurations());
- if (useDynamicVersionIfEnabled && targetConfig.useDynamicConfigurations()) {
+ if (useDynamicVersionIfEnabled) {
return skyframeExecutor.getConfigurationForTesting(
reporter, targetConfig.fragmentClasses(), targetConfig.getOptions());
} else {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
index e04eae3..7e2400d 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
@@ -432,20 +432,17 @@
hostConfiguration.getMiddlemanDirectory(RepositoryName.MAIN).getPath().toString(),
"internal(host)");
- if (targetConfiguration.useDynamicConfigurations()) {
- // With dynamic configurations, the output paths that bin, genfiles, etc. refer to may
- // or may not include the C++-contributed pieces. e.g. they may be
- // bazel-out/gcc-X-glibc-Y-k8-fastbuild/ or they may be bazel-out/fastbuild/. This code
- // adds support for the non-C++ case, too.
- Map<String, String> prunedRootMap = new HashMap<>();
- for (Map.Entry<String, String> root : rootMap.entrySet()) {
- prunedRootMap.put(
- OUTPUT_PATH_CPP_PREFIX_PATTERN.matcher(root.getKey()).replaceFirst(""),
- root.getValue()
- );
- }
- rootMap.putAll(prunedRootMap);
+ // The output paths that bin, genfiles, etc. refer to may or may not include the C++-contributed
+ // pieces. e.g. they may be bazel-out/gcc-X-glibc-Y-k8-fastbuild/ or they may be
+ // bazel-out/fastbuild/. This code adds support for the non-C++ case, too.
+ Map<String, String> prunedRootMap = new HashMap<>();
+ for (Map.Entry<String, String> root : rootMap.entrySet()) {
+ prunedRootMap.put(
+ OUTPUT_PATH_CPP_PREFIX_PATTERN.matcher(root.getKey()).replaceFirst(""),
+ root.getValue()
+ );
}
+ rootMap.putAll(prunedRootMap);
Set<String> files = new LinkedHashSet<>();
for (Artifact artifact : artifacts) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 1f0a7a4..c00b06e 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -1304,16 +1304,12 @@
BuildConfiguration config;
try {
config = getConfiguredTarget(label).getConfiguration();
+ config = view.getDynamicConfigurationForTesting(getTarget(label), config, reporter);
} catch (LabelSyntaxException e) {
throw new IllegalArgumentException(e);
- }
- if (targetConfig.useDynamicConfigurations()) {
- try {
- config = view.getDynamicConfigurationForTesting(getTarget(label), config, reporter);
- } catch (Exception e) {
- //TODO(b/36585204): Clean this up
- throw new RuntimeException(e);
- }
+ } catch (Exception e) {
+ //TODO(b/36585204): Clean this up
+ throw new RuntimeException(e);
}
return new ConfiguredTargetKey(makeLabel(label), config);
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
index f01405e..c2c4c30 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
@@ -56,7 +56,6 @@
import com.google.devtools.common.options.Options;
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsParser;
-import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -196,9 +195,8 @@
public void assertConfigurationsHaveUniqueOutputDirectories(
BuildConfigurationCollection configCollection) throws Exception {
- Collection<BuildConfiguration> allConfigs = configCollection.getAllConfigurations();
Map<Root, BuildConfiguration> outputPaths = new HashMap<>();
- for (BuildConfiguration config : allConfigs) {
+ for (BuildConfiguration config : configCollection.getTargetConfigurations()) {
if (config.isActionsEnabled()) {
BuildConfiguration otherConfig = outputPaths.get(
config.getOutputDirectory(RepositoryName.MAIN));
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsTest.java
index 46c1cf9..0ea4339 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsTest.java
@@ -315,20 +315,12 @@
dep1.getConfiguration().getCpu(),
dep2.getConfiguration().getCpu()))
.containsExactly("armeabi-v7a", "k8");
- // We don't care what order split deps are listed, but it must be deterministic. Static and
- // dynamic configurations happen to apply different orders (static: same order as the split
- // transition definition, dynamic: ConfiguredTargetFunction.DYNAMIC_SPLIT_DEP_ORDERING).
- // That's okay because of the "we don't care what order" principle. The primary value of this
- // test is to check against the new dynamic code, which will soon replace the static code
- // anyway. And the static code is already well-tested through all other Blaze tests. And
- // checking its order would be a lot uglier. So we only worry about the dynamic case here.
- if (getTargetConfiguration().useDynamicConfigurations()) {
- assertThat(
- ConfiguredTargetFunction.DYNAMIC_SPLIT_DEP_ORDERING.compare(
- Dependency.withConfiguration(dep1.getLabel(), dep1.getConfiguration()),
- Dependency.withConfiguration(dep2.getLabel(), dep2.getConfiguration())))
- .isLessThan(0);
- }
+ // We don't care what order split deps are listed, but it must be deterministic.
+ assertThat(
+ ConfiguredTargetFunction.DYNAMIC_SPLIT_DEP_ORDERING.compare(
+ Dependency.withConfiguration(dep1.getLabel(), dep1.getConfiguration()),
+ Dependency.withConfiguration(dep2.getLabel(), dep2.getConfiguration())))
+ .isLessThan(0);
}
}
}
