Error reporting for invalid dynamic configurations

also, AppleConfiguration no longer throws NPE with invalid cpu.

RELNOTES: None.
PiperOrigin-RevId: 167013760

5 files changed, 53 insertions, 4 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
index 618647aaef..45e23dc058 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
@@ -468,6 +468,11 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
     return ApplePlatform.forTarget(applePlatformType, getSingleArchitecture());
   }
 
+  private boolean hasValidSingleArchPlatform() {
+    return ApplePlatform.isApplePlatform(
+        ApplePlatform.cpuStringForTarget(applePlatformType, getSingleArchitecture()));
+  }
+
   /**
    * Gets the current configuration {@link ApplePlatform} for the given {@link PlatformType}.
    * ApplePlatform is determined via a combination between the given platform type and the
@@ -581,7 +586,7 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
     structField = true
   )
   public AppleBitcodeMode getBitcodeMode() {
-    if (getSingleArchPlatform().isDevice()) {
+    if (hasValidSingleArchPlatform() && getSingleArchPlatform().isDevice()) {
       return bitcodeMode;
     } else {
       return AppleBitcodeMode.NONE;
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 214aa02380..7e396ec338 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
@@ -305,6 +305,7 @@ public final class ConfiguredTargetFunction implements SkyFunction {
             new ConfiguredValueCreationException(cause.getMessage(), target.getLabel()));
       } else if (e.getCause() instanceof InvalidConfigurationException) {
         InvalidConfigurationException cause = (InvalidConfigurationException) e.getCause();
+        env.getListener().handle(Event.error(cause.getMessage()));
         throw new ConfiguredTargetFunctionException(
             new ConfiguredValueCreationException(cause.getMessage(), target.getLabel()));
       } else {
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 f61ba9c54a..bfd7cb7fcf 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
@@ -1487,12 +1487,16 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
         for (BuildOptions toOptions : ConfiguredTargetFunction.getDynamicTransitionOptions(
             fromOptions, key.getTransition(), depFragments, ruleClassProvider, true)) {
           SkyKey configKey = BuildConfigurationValue.key(depFragments, toOptions);
-          builder.put(key,
-              ((BuildConfigurationValue) configsResult.get(configKey)).getConfiguration());
+          BuildConfigurationValue configValue =
+              ((BuildConfigurationValue) configsResult.get(configKey));
+          // configValue will be null here if there was an exception thrown during configuration
+          // creation. This will be reported elsewhere.
+          if (configValue != null) {
+            builder.put(key, configValue.getConfiguration());
+          }
         }
       }
     }
-
     return builder;
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
index 457b916555..7f5ef54e5c 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
@@ -17,6 +17,7 @@ import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.getFirstArtifactEndingWith;
+import static org.junit.Assert.fail;
 
 import com.google.common.base.Joiner;
 import com.google.common.base.Predicate;
@@ -37,6 +38,7 @@ import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.packages.BuildType;
+import com.google.devtools.build.lib.packages.util.BazelMockAndroidSupport;
 import com.google.devtools.build.lib.rules.android.AndroidRuleClasses.MultidexMode;
 import com.google.devtools.build.lib.rules.android.deployinfo.AndroidDeployInfoOuterClass.AndroidDeployInfo;
 import com.google.devtools.build.lib.rules.cpp.CppFileTypes;
@@ -77,6 +79,42 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
   }
 
   @Test
+  public void testAndroidSplitTransitionWithInvalidCpu() throws Exception {
+    scratch.file(
+        "test/skylark/my_rule.bzl",
+        "def impl(ctx): ",
+        "  return struct(",
+        "    split_attr_deps = ctx.split_attr.deps,",
+        "    split_attr_dep = ctx.split_attr.dep,",
+        "    k8_deps = ctx.split_attr.deps.get('k8', None),",
+        "    attr_deps = ctx.attr.deps,",
+        "    attr_dep = ctx.attr.dep)",
+        "my_rule = rule(",
+        "  implementation = impl,",
+        "  attrs = {",
+        "    'deps': attr.label_list(cfg = android_common.multi_cpu_configuration),",
+        "    'dep':  attr.label(cfg = android_common.multi_cpu_configuration),",
+        "  })");
+
+    scratch.file(
+        "test/skylark/BUILD",
+        "load('/test/skylark/my_rule', 'my_rule')",
+        "my_rule(name = 'test', deps = [':main'], dep = ':main')",
+        "cc_binary(name = 'main', srcs = ['main.c'])");
+    BazelMockAndroidSupport.setupNdk(mockToolsConfig);
+
+    // --android_cpu with --android_crosstool_top also triggers the split transition.
+    useConfiguration("--fat_apk_cpu=doesnotexist",
+        "--android_crosstool_top=//android/crosstool:everything");
+    try {
+      getConfiguredTarget("//test/skylark:test");
+      fail("Expected an error that no toolchain matched.");
+    } catch (AssertionError e) {
+      assertThat(e.getMessage()).contains("No toolchain found for cpu 'doesnotexist'");
+    }
+  }
+
+  @Test
   public void testAssetsInExternalRepository() throws Exception {
     FileSystemUtils.appendIsoLatin1(
         scratch.resolve("WORKSPACE"), "local_repository(name='r', path='/r')");
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/BUILD b/src/test/java/com/google/devtools/build/lib/rules/android/BUILD
index 2a8d0e1394..a3309e152b 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/BUILD
@@ -69,6 +69,7 @@ java_test(
         "//src/main/protobuf:android_deploy_info_java_proto",
         "//src/test/java/com/google/devtools/build/lib:actions_testutil",
         "//src/test/java/com/google/devtools/build/lib:analysis_testutil",
+        "//src/test/java/com/google/devtools/build/lib:packages_testutil",
         "//src/test/java/com/google/devtools/build/lib:testutil",
         "//third_party:guava",
         "//third_party:junit4",