Avoid runtime errors when skyframe dependencies are missing for null transitions
Before 69dda2c, missing skyframe dependencies resulted in an error being thrown in
the configuration resolver. The error was then caught to return `null` as a way trigger the
skyframe dependency re-execution.
With 69dda2c this has changed. Instead of errors being thrown, `null` being passed-through
as a performance optimization. There is currently one missing place where the `null` case
is not being handled, resulting in a runtime `NullPointerException` when
executing Bazel. This happens when the null transition is being
resolved (and some dependencies are missing; a concrete test scenario
has been added to regression-test this).
```
java.lang.RuntimeException: Unrecoverable error while evaluating node 'ConfiguredTargetKey{label=//packages/bazel/test/ng_package/example:npm_package, config=BuildConfigurationValue.Key[7b6a838ca3e6bb36df045384b4d18f538c8f758f8f81e26d9c75781ac137c727]}' (requested by nodes )
at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:644)
at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:382)
at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(Unknown Source)
at java.base/java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(Unknown Source)
at java.base/java.util.concurrent.ForkJoinPool.scan(Unknown Source)
at java.base/java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)
Caused by: java.lang.NullPointerException
at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:880)
at com.google.common.collect.SingletonImmutableList.<init>(SingletonImmutableList.java:38)
at com.google.common.collect.ImmutableList.of(ImmutableList.java:94)
at com.google.devtools.build.lib.analysis.config.ConfigurationResolver.resolveConfiguration(ConfigurationResolver.java:253)
at com.google.devtools.build.lib.analysis.config.ConfigurationResolver.resolveConfigurations(ConfigurationResolver.java:224)
at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.computeDependencies(ConfiguredTargetFunction.java:687)
at com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.compute(ConfiguredTargetFunction.java:320)
at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:560)
... 7 more
```
Closes #14016.
PiperOrigin-RevId: 398347814
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 24b3df3..e5b25ce 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
@@ -249,8 +249,13 @@
Dependency.Builder dependencyBuilder = dependencyKey.getDependencyBuilder();
ConfigurationTransition transition = dependencyKey.getTransition();
+
if (transition == NullTransition.INSTANCE) {
- return ImmutableList.of(resolveNullTransition(dependencyBuilder, dependencyKind));
+ Dependency resolvedDep = resolveNullTransition(dependencyBuilder, dependencyKind);
+ if (resolvedDep == null) {
+ return null; // Need Skyframe deps.
+ }
+ return ImmutableList.of(resolvedDep);
} else if (transition.isHostTransition()) {
return ImmutableList.of(resolveHostTransition(dependencyBuilder, dependencyKey));
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java
index 6e9106e..1d510d6 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java
@@ -345,6 +345,50 @@
}
@Test
+ public void testNullTransitionAttributeWithMissingSkyframeDep() throws Exception {
+ writeAllowlistFile();
+ scratch.file(
+ "test/starlark/rules.bzl",
+ "load('//myinfo:myinfo.bzl', 'MyInfo')",
+ "def transition_func(settings, attr):",
+ " return {'//test/starlark/nested:flag': True}",
+ "my_transition = transition(",
+ " implementation = transition_func,",
+ " inputs = [],",
+ " outputs = ['//test/starlark/nested:flag']",
+ ")",
+ "def _impl(ctx): ",
+ " return MyInfo(data = ctx.attr.data)",
+ "my_rule = rule(",
+ " implementation = _impl,",
+ " attrs = {",
+ " 'data': attr.label_list(cfg = my_transition, allow_files = True, default = []),",
+ " '_allowlist_function_transition': attr.label(",
+ " default = '//tools/allowlists/function_transition_allowlist',",
+ " ),",
+ " }",
+ ")",
+ "def _basic_impl(ctx):",
+ " return []",
+ "bool_flag = rule(",
+ " implementation = _basic_impl,",
+ " build_setting = config.bool(flag = True),",
+ ")");
+
+ scratch.file(
+ "test/starlark/nested/BUILD",
+ "load('//test/starlark:rules.bzl', 'bool_flag')",
+ "bool_flag(name = 'flag', build_setting_default = False)");
+ scratch.file("test/starlark/some_file.txt", "Random content");
+ scratch.file(
+ "test/starlark/BUILD",
+ "load('//test/starlark:rules.bzl', 'my_rule')",
+ "my_rule(name = 'test', data = ['some_file.txt'])");
+
+ assertThat(getConfiguredTarget("//test/starlark:test")).isNotNull();
+ }
+
+ @Test
public void testTargetAndRuleNotInAllowlist() throws Exception {
writeAllowlistFile();
getAnalysisMock().ccSupport().setupCcToolchainConfigForCpu(mockToolsConfig, "armeabi-v7a");
