Begin removing code that checks use_toolchain_transition.
Part of #15346.
PiperOrigin-RevId: 445250559
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 816e8a0..b0bced2 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
@@ -29,7 +29,6 @@
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
import com.google.devtools.build.lib.analysis.config.Fragment;
-import com.google.devtools.build.lib.analysis.config.HostTransition;
import com.google.devtools.build.lib.analysis.config.TransitionResolver;
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
@@ -78,41 +77,6 @@
public abstract class DependencyResolver {
/**
- * Returns whether or not to use the new toolchain transition. Checks the global incompatible
- * change flag and the rule's toolchain transition readiness attribute.
- */
- // TODO(#10523): Remove this when the migration period for toolchain transitions has ended.
- public static boolean shouldUseToolchainTransition(
- @Nullable BuildConfigurationValue configuration, Target target) {
- return shouldUseToolchainTransition(
- configuration, target instanceof Rule ? (Rule) target : null);
- }
-
- /**
- * Returns whether or not to use the new toolchain transition. Checks the global incompatible
- * change flag and the rule's toolchain transition readiness attribute.
- */
- // TODO(#10523): Remove this when the migration period for toolchain transitions has ended.
- public static boolean shouldUseToolchainTransition(
- @Nullable BuildConfigurationValue configuration, @Nullable Rule rule) {
- // Check whether the global incompatible change flag is set.
- if (configuration != null) {
- PlatformOptions platformOptions = configuration.getOptions().get(PlatformOptions.class);
- if (platformOptions != null && platformOptions.overrideToolchainTransition) {
- return true;
- }
- }
-
- // Check the rule definition to see if it is ready.
- if (rule != null && rule.getRuleClassObject().useToolchainTransition()) {
- return true;
- }
-
- // Default to false.
- return false;
- }
-
- /**
* What we know about a dependency edge after factoring in the properties of the configured target
* that the edge originates from, but not the properties of target it points to.
*/
@@ -184,7 +148,6 @@
@Nullable Aspect aspect,
ImmutableMap<Label, ConfigMatchingProvider> configConditions,
@Nullable ToolchainCollection<ToolchainContext> toolchainContexts,
- boolean useToolchainTransition,
@Nullable TransitionFactory<RuleTransitionData> trimmingTransitionFactory)
throws Failure, InterruptedException, InconsistentAspectOrderException {
NestedSetBuilder<Cause> rootCauses = NestedSetBuilder.stableOrder();
@@ -194,7 +157,6 @@
aspect != null ? ImmutableList.of(aspect) : ImmutableList.of(),
configConditions,
toolchainContexts,
- useToolchainTransition,
rootCauses,
trimmingTransitionFactory);
if (!rootCauses.isEmpty()) {
@@ -237,7 +199,6 @@
Iterable<Aspect> aspects,
ImmutableMap<Label, ConfigMatchingProvider> configConditions,
@Nullable ToolchainCollection<ToolchainContext> toolchainContexts,
- boolean useToolchainTransition,
NestedSetBuilder<Cause> rootCauses,
@Nullable TransitionFactory<RuleTransitionData> trimmingTransitionFactory)
throws Failure, InterruptedException, InconsistentAspectOrderException {
@@ -297,7 +258,6 @@
fromRule,
attributeMap,
toolchainContexts,
- useToolchainTransition,
aspects);
OrderedSetMultimap<DependencyKind, DependencyKey> outgoingEdges =
@@ -322,7 +282,6 @@
@Nullable Rule fromRule,
@Nullable ConfiguredAttributeMapper attributeMap,
@Nullable ToolchainCollection<ToolchainContext> toolchainContexts,
- boolean useToolchainTransition,
Iterable<Aspect> aspects)
throws Failure {
OrderedSetMultimap<DependencyKind, PartiallyResolvedDependency> partiallyResolvedDeps =
@@ -337,29 +296,18 @@
// use sensible defaults. Not depending on their package makes the error message reporting
// a missing toolchain a bit better.
// TODO(lberki): This special-casing is weird. Find a better way to depend on toolchains.
- // TODO(#10523): Remove check when this is fully released.
// This logic needs to stay in sync with the dep finding logic in
// //third_party/bazel/src/main/java/com/google/devtools/build/lib/analysis/Util.java#findImplicitDeps.
- if (useToolchainTransition) {
- ToolchainDependencyKind tdk = (ToolchainDependencyKind) entry.getKey();
- ToolchainContext toolchainContext =
- toolchainContexts.getToolchainContext(tdk.getExecGroupName());
- partiallyResolvedDeps.put(
- entry.getKey(),
- PartiallyResolvedDependency.builder()
- .setLabel(toLabel)
- .setTransition(NoTransition.INSTANCE)
- .setExecutionPlatformLabel(toolchainContext.executionPlatform().label())
- .build());
- } else {
- // Legacy approach: use a HostTransition.
- partiallyResolvedDeps.put(
- entry.getKey(),
- PartiallyResolvedDependency.builder()
- .setLabel(toLabel)
- .setTransition(HostTransition.INSTANCE)
- .build());
- }
+ ToolchainDependencyKind tdk = (ToolchainDependencyKind) entry.getKey();
+ ToolchainContext toolchainContext =
+ toolchainContexts.getToolchainContext(tdk.getExecGroupName());
+ partiallyResolvedDeps.put(
+ entry.getKey(),
+ PartiallyResolvedDependency.builder()
+ .setLabel(toLabel)
+ .setTransition(NoTransition.INSTANCE)
+ .setExecutionPlatformLabel(toolchainContext.executionPlatform().label())
+ .build());
continue;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Util.java b/src/main/java/com/google/devtools/build/lib/analysis/Util.java
index 5484214..193cb93 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Util.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Util.java
@@ -101,23 +101,13 @@
// This logic should stay up to date with the dep creation logic in
// DependencyResolver#partiallyResolveDependencies.
BuildConfigurationValue targetConfiguration = ruleContext.getConfiguration();
- BuildConfigurationValue hostConfiguration = ruleContext.getHostConfiguration();
for (Label toolchain : toolchainContext.resolvedToolchainLabels()) {
- if (DependencyResolver.shouldUseToolchainTransition(
- targetConfiguration, ruleContext.getRule())) {
- maybeImplicitDeps.add(
- ConfiguredTargetKey.builder()
- .setLabel(toolchain)
- .setConfiguration(targetConfiguration)
- .setExecutionPlatformLabel(toolchainContext.executionPlatform().label())
- .build());
- } else {
- maybeImplicitDeps.add(
- ConfiguredTargetKey.builder()
- .setLabel(toolchain)
- .setConfiguration(hostConfiguration)
- .build());
- }
+ maybeImplicitDeps.add(
+ ConfiguredTargetKey.builder()
+ .setLabel(toolchain)
+ .setConfiguration(targetConfiguration)
+ .setExecutionPlatformLabel(toolchainContext.executionPlatform().label())
+ .build());
}
}
return ImmutableSet.copyOf(Sets.difference(maybeImplicitDeps, explicitDeps));
diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java
index 53d7632..b286e26 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java
@@ -131,7 +131,6 @@
/*aspect=*/ null,
configConditions,
toolchainContexts,
- DependencyResolver.shouldUseToolchainTransition(config, target),
trimmingTransitionFactory);
} catch (DependencyResolver.Failure | InconsistentAspectOrderException e) {
// This is an abuse of InterruptedException.
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 bb7c9d7..326b851 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
@@ -37,7 +37,6 @@
import com.google.devtools.build.lib.analysis.DuplicateException;
import com.google.devtools.build.lib.analysis.ExecGroupCollection.InvalidExecGroupException;
import com.google.devtools.build.lib.analysis.InconsistentAspectOrderException;
-import com.google.devtools.build.lib.analysis.PlatformOptions;
import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
import com.google.devtools.build.lib.analysis.TargetAndConfiguration;
import com.google.devtools.build.lib.analysis.ToolchainCollection;
@@ -61,7 +60,6 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.Aspect;
-import com.google.devtools.build.lib.packages.AspectDefinition;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
import com.google.devtools.build.lib.packages.NativeAspectClass;
@@ -309,7 +307,6 @@
: ToolchainCollection.builder()
.addDefaultContext(unloadedToolchainContext)
.build(),
- shouldUseToolchainTransition(configuration, aspect.getDefinition()),
ruleClassProvider,
buildViewProvider.getSkyframeBuildView().getHostConfiguration());
} catch (ConfiguredValueCreationException e) {
@@ -573,25 +570,6 @@
}
/**
- * Returns whether or not to use the new toolchain transition. Checks the global incompatible
- * change flag and the aspect's toolchain transition readiness attribute.
- */
- // TODO(#10523): Remove this when the migration period for toolchain transitions has ended.
- private static boolean shouldUseToolchainTransition(
- @Nullable BuildConfigurationValue configuration, AspectDefinition definition) {
- // Check whether the global incompatible change flag is set.
- if (configuration != null) {
- PlatformOptions platformOptions = configuration.getOptions().get(PlatformOptions.class);
- if (platformOptions != null && platformOptions.overrideToolchainTransition) {
- return true;
- }
- }
-
- // Check the aspect definition to see if it is ready.
- return definition.useToolchainTransition();
- }
-
- /**
* Collects {@link AspectKey} dependencies by performing a postorder traversal over {@link
* AspectKey#getBaseKeys}.
*
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 7b9db36..1fdee52 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
@@ -346,8 +346,6 @@
unloadedToolchainContexts == null
? null
: unloadedToolchainContexts.asToolchainContexts(),
- DependencyResolver.shouldUseToolchainTransition(
- targetAndConfiguration.getConfiguration(), targetAndConfiguration.getTarget()),
ruleClassProvider,
view.getHostConfiguration());
if (!state.transitiveRootCauses.isEmpty()) {
@@ -734,7 +732,6 @@
Iterable<Aspect> aspects,
ImmutableMap<Label, ConfigMatchingProvider> configConditions,
@Nullable ToolchainCollection<ToolchainContext> toolchainContexts,
- boolean useToolchainTransition,
RuleClassProvider ruleClassProvider,
BuildConfigurationValue hostConfiguration)
throws DependencyEvaluationException, ConfiguredValueCreationException,
@@ -763,7 +760,6 @@
aspects,
configConditions,
toolchainContexts,
- useToolchainTransition,
transitiveRootCauses,
((ConfiguredRuleClassProvider) ruleClassProvider)
.getTrimmingTransitionFactory());
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java
index 550893f..300b3eb 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java
@@ -94,7 +94,6 @@
aspect != null ? Aspect.forNative(aspect) : null,
ImmutableMap.of(),
/*toolchainContexts=*/ null,
- /*useToolchainTransition=*/ false,
/*trimmingTransitionFactory=*/ null);
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
index 3cff934..34df45d 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
@@ -446,7 +446,6 @@
ctgNode,
toolchainContexts == null ? null : toolchainContexts.getTargetPlatform()),
toolchainContexts,
- DependencyResolver.shouldUseToolchainTransition(configuration, target),
ruleClassProvider.getTrimmingTransitionFactory());
}
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 42e6301..5c61cec 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
@@ -156,7 +156,6 @@
ImmutableList.of(),
ImmutableMap.of(),
toolchainContexts,
- /*useToolchainTransition=*/ true,
stateProvider.lateBoundRuleClassProvider(),
stateProvider.lateBoundHostConfig());
return env.valuesMissing() ? null : new Value(depMap);