Default Android dependencies to a //external: label with a default binding to //tools/android: .

This is useful because we can then eventually implement an android_tools_repository() rule that lets Bazel download the Android tools from somewhere instead of requiring it to be in every workspace with Android tools.

The number of tools here is somewhat scary, therefore, I'm considering creating an android_tools rule which would have an attribute for each of these things.

Some non-trivial things about this CL:

- The labels to load are removed from AndroidConfiguration because they would resolve to e.g. //external:dx_jar, which labels just don't exist and I don't want to add dummy //external:labels not prefixed with android_
- RedirectChaser is taught how to chase redirect through bind() rules because the Android SDK is now found by //external:android_sdk -> //tools/android:sdk -> @androidsdk//:sdk . Ideally, it would be ///external:android_sdk -> @androidsdk//:sdk, but I figured I'd not fix that in this CL.

--
MOS_MIGRATED_REVID=96080553

6 files changed, 95 insertions, 44 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/Constants.java b/src/main/java/com/google/devtools/build/lib/Constants.java
index 1d214e98bd..60d21869c1 100644
--- a/src/main/java/com/google/devtools/build/lib/Constants.java
+++ b/src/main/java/com/google/devtools/build/lib/Constants.java
@@ -87,6 +87,7 @@ public class Constants {
    */
   public static final ImmutableSet<String> IOS_DEVICE_RULE_CLASSES = ImmutableSet.of("ios_device");
 
-  public static final String ANDROID_DEFAULT_SDK = "//tools/android:sdk".toString();
+  public static final String ANDROID_DEFAULT_SDK = "//external:android_sdk".toString();
   public static final boolean ANDROID_ALLOW_SDK_FILEGROUP = Boolean.valueOf(false);
+  public static final String ANDROID_DEP_PREFIX = "//external:android/".toString();
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RedirectChaser.java b/src/main/java/com/google/devtools/build/lib/analysis/RedirectChaser.java
index 459868460d..3400b1f0ca 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RedirectChaser.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RedirectChaser.java
@@ -82,23 +82,20 @@ public final class RedirectChaser {
         if (possibleRedirect == null) {
           return null;
         }
-        if ((possibleRedirect instanceof Rule) &&
-            "filegroup".equals(((Rule) possibleRedirect).getRuleClass())) {
-          List<Label> labels = new StaticValuedAttributeMapper((Rule) possibleRedirect)
-              .getAndValidate("srcs", Type.LABEL_LIST);
-          if (labels.size() != 1) {
-            // We can't distinguish redirects from the final filegroup, so we assume this must be
-            // the final one.
-            return label;
-          }
-          label = labels.get(0);
-          if (!visitedLabels.add(label)) {
-            throw new InvalidConfigurationException("The " + name + " points to a filegroup which "
-                + "recursively includes itself. The label " + label + " is part of the loop");
-          }
-        } else {
+        Label newLabel = getFilegroupRedirect(possibleRedirect);
+        if (newLabel == null) {
+          newLabel = getBindRedirect(possibleRedirect);
+        }
+
+        if (newLabel == null) {
           return label;
         }
+
+        label = newLabel;
+        if (!visitedLabels.add(label)) {
+          throw new InvalidConfigurationException("The " + name + " points to a filegroup which "
+              + "recursively includes itself. The label " + label + " is part of the loop");
+        }
       }
     } catch (NoSuchPackageException e) {
       throw new InvalidConfigurationException(e.getMessage(), e);
@@ -106,4 +103,36 @@ public final class RedirectChaser {
       return label;
     }
   }
+
+  private static Label getFilegroupRedirect(Target target) throws InvalidConfigurationException {
+    if (!(target instanceof Rule)) {
+      return null;
+    }
+
+    Rule rule = (Rule) target;
+    if (!rule.getRuleClass().equals("filegroup")) {
+      return null;
+    }
+
+    List<Label> labels =
+        new StaticValuedAttributeMapper(rule).getAndValidate("srcs", Type.LABEL_LIST);
+    if (labels.size() != 1) {
+      return null;
+    }
+
+    return labels.get(0);
+  }
+
+  private static Label getBindRedirect(Target target) throws InvalidConfigurationException {
+    if (!(target instanceof Rule)) {
+      return null;
+    }
+
+    Rule rule = (Rule) target;
+    if (!rule.getRuleClass().equals("bind")) {
+      return null;
+    }
+
+    return new StaticValuedAttributeMapper(rule).getAndValidate("actual", Type.LABEL);
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android.WORKSPACE b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android.WORKSPACE
new file mode 100644
index 0000000000..94b80edcf1
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android.WORKSPACE
@@ -0,0 +1,19 @@
+bind(name = "android/proguard_whitelister", actual = "//tools/android:proguard_whitelister")
+bind(name = "android/merge_manifests", actual = "//tools/android:merge_manifests")
+bind(name = "android/build_incremental_dexmanifest", actual = "//tools/android:build_incremental_dexmanifest")
+bind(name = "android/stubify_manifest", actual = "//tools/android:stubify_manifest")
+bind(name = "android/incremental_install", actual = "//tools/android:incremental_install")
+bind(name = "android/build_split_manifest", actual = "//tools/android:build_split_manifest")
+bind(name = "android/strip_resources", actual = "//tools/android:strip_resources")
+bind(name = "android/incremental_stub_application", actual = "//tools/android:incremental_stub_application")
+bind(name = "android/incremental_split_stub_application", actual = "//tools/android:incremental_split_stub_application")
+bind(name = "android/resources_processor", actual = "//tools/android:resources_processor")
+bind(name = "android/aar_generator", actual = "//tools/android:aar_generator")
+bind(name = "android/shuffle_jars", actual = "//tools/android:shuffle_jars")
+bind(name = "android/merge_dexzips", actual = "//tools/android:merge_dexzips")
+bind(name = "android/debug_keystore", actual = "//tools/android:debug_keystore")
+bind(name = "android/sdk", actual = "//tools/android:sdk")
+
+
+
+
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
index e2051c234f..0a908de2be 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
@@ -141,12 +141,14 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     public Map<String, Set<Label>> getDefaultsLabels(BuildConfiguration.Options commonOptions) {
       Map<String, Set<Label>> result = new TreeMap<>();
       Label realSdk = realSdk();
-      addLabel(result, realSdk, "ANDROID_AIDL_TOOL", "static_aidl_tool");
-      addLabel(result, realSdk, "ANDROID_AIDL_FRAMEWORK", "aidl_framework");
-      addLabel(result, realSdk, "ANDROID_AAPT", "static_aapt_tool");
-      addLabel(result, realSdk, "ANDROID_ADB", "static_adb_tool");
-      addLabel(result, realSdk, "ANDROID_APKBUILDER", "apkbuilder_tool");
-      addLabel(result, realSdk, "ANDROID_DX_JAR", "dx_jar");
+      if (Constants.ANDROID_ALLOW_SDK_FILEGROUP) {
+        addLabel(result, realSdk, "ANDROID_AIDL_TOOL", "static_aidl_tool");
+        addLabel(result, realSdk, "ANDROID_AIDL_FRAMEWORK", "aidl_framework");
+        addLabel(result, realSdk, "ANDROID_AAPT", "static_aapt_tool");
+        addLabel(result, realSdk, "ANDROID_ADB", "static_adb_tool");
+        addLabel(result, realSdk, "ANDROID_APKBUILDER", "apkbuilder_tool");
+        addLabel(result, realSdk, "ANDROID_DX_JAR", "dx_jar");
+      }
       return result;
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibraryBaseRule.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibraryBaseRule.java
index 5a9d5e4dd7..1addcc98e4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibraryBaseRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibraryBaseRule.java
@@ -20,6 +20,7 @@ import static com.google.devtools.build.lib.packages.Type.LABEL;
 import static com.google.devtools.build.lib.packages.Type.LABEL_LIST;
 import static com.google.devtools.build.lib.packages.Type.STRING;
 
+import com.google.devtools.build.lib.Constants;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 import com.google.devtools.build.lib.packages.Attribute;
@@ -163,7 +164,7 @@ public final class AndroidLibraryBaseRule implements RuleDefinition {
                       @Override
                       public Object getDefault(AttributeMap rule) {
                         return rule.isAttributeValueExplicitlySpecified("proguard_specs")
-                            ? env.getLabel("//tools/android:proguard_whitelister")
+                            ? env.getLabel(Constants.ANDROID_DEP_PREFIX + "proguard_whitelister")
                             : null;
                       }
                     }))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
index 6c72ab8b87..103dcc4bc6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
@@ -118,24 +118,31 @@ public final class AndroidRuleClasses {
   public static final SafeImplicitOutputsFunction JAVA_RESOURCES_JAR =
       fromTemplates("%{name}_files/java_resources.jar");
   public static final String MANIFEST_MERGE_TOOL_LABEL =
-      "//tools/android:merge_manifests";
+      Constants.ANDROID_DEP_PREFIX + "merge_manifests";
   public static final String BUILD_INCREMENTAL_DEXMANIFEST_LABEL =
-      "//tools/android:build_incremental_dexmanifest";
-  public static final String STUBIFY_MANIFEST_LABEL = "//tools/android:stubify_manifest";
-  public static final String INCREMENTAL_INSTALL_LABEL = "//tools/android:incremental_install";
-  public static final String BUILD_SPLIT_MANIFEST_LABEL = "//tools/android:build_split_manifest";
-  public static final String STRIP_RESOURCES_LABEL = "//tools/android:strip_resources";
+      Constants.ANDROID_DEP_PREFIX + "build_incremental_dexmanifest";
+  public static final String STUBIFY_MANIFEST_LABEL =
+      Constants.ANDROID_DEP_PREFIX + "stubify_manifest";
+  public static final String INCREMENTAL_INSTALL_LABEL =
+      Constants.ANDROID_DEP_PREFIX + "incremental_install";
+  public static final String BUILD_SPLIT_MANIFEST_LABEL =
+      Constants.ANDROID_DEP_PREFIX + "build_split_manifest";
+  public static final String STRIP_RESOURCES_LABEL =
+      Constants.ANDROID_DEP_PREFIX + "strip_resources";
 
   public static final Label DEFAULT_ANDROID_SDK =
-      Label.parseAbsoluteUnchecked(Constants.ANDROID_DEFAULT_SDK);
+      Label.parseAbsoluteUnchecked(
+          Constants.ANDROID_DEFAULT_SDK);
   public static final Label DEFAULT_INCREMENTAL_STUB_APPLICATION =
-      Label.parseAbsoluteUnchecked("//tools/android:incremental_stub_application");
+      Label.parseAbsoluteUnchecked(
+          Constants.ANDROID_DEP_PREFIX + "incremental_stub_application");
   public static final Label DEFAULT_INCREMENTAL_SPLIT_STUB_APPLICATION =
-      Label.parseAbsoluteUnchecked("//tools/android:incremental_split_stub_application");
+      Label.parseAbsoluteUnchecked(
+          Constants.ANDROID_DEP_PREFIX + "incremental_split_stub_application");
   public static final Label DEFAULT_RESOURCES_PROCESSOR =
-      Label.parseAbsoluteUnchecked("//tools/android:resources_processor");
+      Label.parseAbsoluteUnchecked(Constants.ANDROID_DEP_PREFIX + "resources_processor");
   public static final Label DEFAULT_AAR_GENERATOR =
-      Label.parseAbsoluteUnchecked("//tools/android:aar_generator");
+      Label.parseAbsoluteUnchecked(Constants.ANDROID_DEP_PREFIX + "aar_generator");
 
   public static final LateBoundLabel<BuildConfiguration> INCREMENTAL_STUB_APPLICATION =
       new LateBoundLabel<BuildConfiguration>(DEFAULT_INCREMENTAL_STUB_APPLICATION) {
@@ -503,13 +510,6 @@ public final class AndroidRuleClasses {
           These compiler options are passed to javac after the global compiler options.</p>
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
           .add(attr("javacopts", STRING_LIST))
-          // $android_jar must be in the target configuration because it points to an
-          // android_tools_defaults_jar rule, and that needs the configuration to fetch the actual
-          // android.jar .
-          .add(attr("$android_jar", LABEL)
-              .value(env.getLabel("//tools/defaults:android_jar")))
-          .add(attr("$android_dx_jar", LABEL).cfg(HOST)
-              .value(env.getLabel("//tools/defaults:android_dx_jar")))
           // TODO(ahumesky): It would be better to put this dependency in //tools/android somehow
           // like all the rest of android tools.
           .add(attr("$jarjar_bin", LABEL).cfg(HOST).exec()
@@ -627,12 +627,12 @@ public final class AndroidRuleClasses {
               attr("$shuffle_jars", LABEL)
                   .cfg(HOST)
                   .exec()
-                  .value(env.getLabel("//tools/android:shuffle_jars")))
+                  .value(env.getLabel(Constants.ANDROID_DEP_PREFIX + "shuffle_jars")))
           .add(
               attr("$merge_dexzips", LABEL)
                   .cfg(HOST)
                   .exec()
-                  .value(env.getLabel("//tools/android:merge_dexzips")))
+                  .value(env.getLabel(Constants.ANDROID_DEP_PREFIX + "merge_dexzips")))
           .add(
               attr("$incremental_install", LABEL)
                   .cfg(HOST)
@@ -654,7 +654,6 @@ public final class AndroidRuleClasses {
           .add(
               attr(":incremental_split_stub_application", LABEL)
                   .value(AndroidRuleClasses.INCREMENTAL_SPLIT_STUB_APPLICATION))
-
           /* <!-- #BLAZE_RULE($android_binary_base).ATTRIBUTE(dexopts) -->
            Additional command-line flags for the dx tool when generating classes.dex.
            ${SYNOPSIS}