Add a better failure message when no valid Android SDK is found.

Part of #16285. PiperOrigin-RevId: 572256569 Change-Id: I47075f87e6e13615391bb056ee769d024e6b089e
bazelbuild · Oct 10, 2023 · 18990f4 · 18990f4
1 parent a6cb9b0
commit 18990f4
Showing 1 changed file with 21 additions and 41 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSdkProvider.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSdkProvider.java
@@ -15,11 +15,8 @@
 
 import static com.google.devtools.build.lib.rules.android.AndroidStarlarkData.fromNoneable;
 
-import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
-import com.google.devtools.build.lib.analysis.PlatformConfiguration;
-import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
@@ -43,8 +40,6 @@ public final class AndroidSdkProvider extends NativeInfo
 
   public static final String ANDROID_SDK_TOOLCHAIN_TYPE_ATTRIBUTE_NAME =
       "$android_sdk_toolchain_type";
-  public static final String ANDROID_SDK_DUMMY_TOOLCHAIN_ATTRIBUTE_NAME =
-      "$android_sdk_dummy_toolchains";
 
   public static final Provider PROVIDER = new Provider();
 
@@ -162,12 +157,6 @@ public static AndroidSdkProvider fromRuleContext(
       return null;
     }
 
-    ResolvedToolchainContext toolchainContext = ruleContext.getToolchainContext();
-    if (usingDummyToolchain(ruleContext, configuration, toolchainContext)) {
-      // The above method will have already shown an error.
-      return null;
-    }
-
     if (toolchainType == null) {
       ruleContext.ruleError(
           String.format(
@@ -179,6 +168,7 @@ public static AndroidSdkProvider fromRuleContext(
               ANDROID_SDK_TOOLCHAIN_TYPE_ATTRIBUTE_NAME));
       return null;
     }
+
     ToolchainInfo info = ruleContext.getToolchainInfo(toolchainType);
     if (info == null) {
       ruleContext.ruleError(
@@ -189,48 +179,38 @@ public static AndroidSdkProvider fromRuleContext(
               ruleContext.getRuleClassNameForLogging(), ruleContext.getLabel(), toolchainType));
       return null;
     }
+
+    AndroidSdkProvider androidSdkProvider;
     try {
-      return (AndroidSdkProvider) info.getValue("android_sdk_info");
+      androidSdkProvider = (AndroidSdkProvider) info.getValue("android_sdk_info");
     } catch (EvalException e) {
       ruleContext.ruleError(
           String.format(
               "Android SDK toolchain for %s didn't have an 'android_sdk_info' provider: %s",
               ruleContext.getLabel(), e.getMessage()));
       return null;
     }
+
+    if (usingDummyToolchain(ruleContext, androidSdkProvider)) {
+      // The above method will have already shown an error.
+      return null;
+    }
+
+    return androidSdkProvider;
   }
 
   private static boolean usingDummyToolchain(
-      RuleContext ruleContext,
-      BuildConfigurationValue configuration,
-      ResolvedToolchainContext toolchainContext) {
-    Type<Label> depType =
-        ruleContext.getRule().getRuleClassObject().isStarlark()
-            ? BuildType.LABEL
-            : BuildType.NODEP_LABEL;
-    if (!ruleContext.attributes().has(ANDROID_SDK_DUMMY_TOOLCHAIN_ATTRIBUTE_NAME, depType)) {
-      // We can't tell, so assume not.
-      return false;
-    }
+      RuleContext ruleContext, AndroidSdkProvider androidSdkProvider) {
 
-    ImmutableSet<Label> resolvedToolchains = toolchainContext.resolvedToolchainLabels();
-    Label dummyToochain =
-        ruleContext.attributes().get(ANDROID_SDK_DUMMY_TOOLCHAIN_ATTRIBUTE_NAME, depType);
-    for (Label toolchain : resolvedToolchains) {
-      if (dummyToochain.equals(toolchain)) {
-        ruleContext.ruleError(
-            // TODO(jcater): Decide whether to rewrite message to refer to --android_platforms.
-            // It's unclear if we should always tell users to use --android_platforms, or if
-            // there are still cases where --platforms is preferred.
-            String.format(
-                "'%s' rule '%s' requested sdk toolchain resolution via"
-                    + " --incompatible_enable_android_toolchain_resolution but hasn't set an"
-                    + " appropriate --platforms value: --platforms=%s",
-                ruleContext.getRuleClassNameForLogging(),
-                ruleContext.getLabel(),
-                configuration.getFragment(PlatformConfiguration.class).getTargetPlatform()));
-        return true;
-      }
+    if (androidSdkProvider.getAndroidJar().getFilename().matches("dummy\\.jar$")) {
+      // This is an invalid SDK, and probably due to a default configuration.
+      ruleContext.ruleError(
+          String.format(
+              "'%s' rule '%s' requested an android sdk via toolchain resolution but hasn't set an"
+                  + " appropriate --android_platforms value: Either set"
+                  + " --noincompatible_enable_android_toolchain_resolution or --android_platforms.",
+              ruleContext.getRuleClassNameForLogging(), ruleContext.getLabel()));
+      return true;
     }
 
     return false;