Various odds and ends in preparation for adding the Android rules to BazelRuleClassProvider:

- Add stub targets to tools/android/BUILD
- Make Constants.ANDROID_DEFAULT_SDK non-constant so that the classfile can be replaced in the .jar
- Make AndroidTools complain if --android_sdk does not point to an android_sdk rule.
- Make the default visibility in the BUILD file generated by android_sdk_repository public

--
MOS_MIGRATED_REVID=95816158

6 files changed, 43 insertions, 17 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 9e18b64544..1d214e98bd 100644
--- a/src/main/java/com/google/devtools/build/lib/Constants.java
+++ b/src/main/java/com/google/devtools/build/lib/Constants.java
@@ -87,5 +87,6 @@ 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";
+  public static final String ANDROID_DEFAULT_SDK = "//tools/android:sdk".toString();
+  public static final boolean ANDROID_ALLOW_SDK_FILEGROUP = Boolean.valueOf(false);
 }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_sdk_repository_template.txt b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_sdk_repository_template.txt
index 46bfb81323..51ea29a23c 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_sdk_repository_template.txt
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_sdk_repository_template.txt
@@ -1,3 +1,5 @@
+package(default_visibility = ["//visibility:public"])
+
 android_sdk(
     name = "sdk",
     proguard = "tools/proguard/bin/proguard.sh",
@@ -12,8 +14,7 @@ android_sdk(
     annotations_jar = "tools/support/annotations.jar",
     main_dex_classes = "build-tools/%build_tools_version%/mainDexClasses.rules",
     apkbuilder = ":apkbuilder",
-    zipalign = "build-tools/%build_tools_version%/zipalign",
-    visibility = ["//visibility:public"])
+    zipalign = "build-tools/%build_tools_version%/zipalign")
 
 genrule(
     name = "main_dex_list_creator_source",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
index acd699d9a0..9764ac9fb9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
@@ -89,11 +89,15 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
 
     AndroidCommon androidCommon = new AndroidCommon(
         ruleContext, javaCommon, true /* asNeverLink */, true /* exportDeps */);
+    AndroidTools tools = AndroidTools.fromRuleContext(ruleContext);
+    if (tools == null) {
+      return null;
+    }
     try {
       RuleConfiguredTargetBuilder builder =
           init(ruleContext, filesBuilder,
               getTransitiveResourceContainers(ruleContext, ImmutableList.of("resources", "deps")),
-              javaCommon, androidCommon, javaSemantics, androidSemantics,
+              javaCommon, androidCommon, javaSemantics, androidSemantics, tools,
               ImmutableList.<String>of("deps"));
       return builder.build();
     } catch (RuleConfigurationException e) {
@@ -103,7 +107,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
     }
   }
 
-  public static RuleConfiguredTargetBuilder init(
+  private static RuleConfiguredTargetBuilder init(
       RuleContext ruleContext,
       NestedSetBuilder<Artifact> filesBuilder,
       NestedSet<ResourceContainer> resourceContainers,
@@ -111,6 +115,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
       AndroidCommon androidCommon,
       JavaSemantics javaSemantics,
       AndroidSemantics androidSemantics,
+      AndroidTools tools,
       List<String> depsAttributes) {
     // TODO(bazel-team): Find a way to simplify this code.
     // treeKeys() means that the resulting map sorts the entries by key, which is necessary to
@@ -146,8 +151,6 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
       toolchainMap.put(config.getCpu(), CppHelper.getToolchain(ruleContext));
     }
 
-    AndroidTools tools = AndroidTools.fromRuleContext(ruleContext);
-
     NativeLibs nativeLibs = shouldLinkNativeDeps(ruleContext)
         ? NativeLibs.fromLinkedNativeDeps(ruleContext, androidSemantics.getNativeDepsFileName(),
             depsByArchitecture, toolchainMap, configurationMap)
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 a048bcad1e..e2051c234f 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
@@ -66,7 +66,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     // Label of filegroup combining all Android tools used as implicit dependencies of
     // android_* rules
     @Option(name = "android_sdk",
-            defaultValue = Constants.ANDROID_DEFAULT_SDK,
+            defaultValue = "null",
             category = "version",
             converter = LabelConverter.class,
             help = "Specifies Android SDK/platform that is used to build Android applications.")
@@ -118,7 +118,8 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
         labelMap.put("android_proguard", proguard);
       }
 
-      labelMap.put("android_sdk", sdk);
+      labelMap.put("android_sdk", realSdk());
+
       labelMap.put("android_incremental_stub_application",
           AndroidRuleClasses.DEFAULT_INCREMENTAL_STUB_APPLICATION);
       labelMap.put("android_incremental_split_stub_application",
@@ -127,15 +128,25 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
       labelMap.put("android_aar_generator", AndroidRuleClasses.DEFAULT_AAR_GENERATOR);
     }
 
+    // This method is here because Constants.ANDROID_DEFAULT_SDK cannot be a constant, because we
+    // replace the class file in the .jar after compilation. However, that means that we cannot use
+    // it as an attribute value in an annotation.
+    private Label realSdk() {
+      return sdk == null
+          ? Label.parseAbsoluteUnchecked(Constants.ANDROID_DEFAULT_SDK)
+          : sdk;
+    }
+
     @Override
     public Map<String, Set<Label>> getDefaultsLabels(BuildConfiguration.Options commonOptions) {
       Map<String, Set<Label>> result = new TreeMap<>();
-      addLabel(result, "ANDROID_AIDL_TOOL", "static_aidl_tool");
-      addLabel(result, "ANDROID_AIDL_FRAMEWORK", "aidl_framework");
-      addLabel(result, "ANDROID_AAPT", "static_aapt_tool");
-      addLabel(result, "ANDROID_ADB", "static_adb_tool");
-      addLabel(result, "ANDROID_APKBUILDER", "apkbuilder_tool");
-      addLabel(result, "ANDROID_DX_JAR", "dx_jar");
+      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");
       return result;
     }
 
@@ -144,7 +155,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
       return ImmutableList.of("android_tools_defaults_jar(name = 'android_jar')");
     }
 
-    private void addLabel(Map<String, Set<Label>> map, String key, String localLabel) {
+    private void addLabel(Map<String, Set<Label>> map, Label sdk, String key, String localLabel) {
       try {
         map.put(key, ImmutableSet.of(sdk.getLocalTargetLabel(localLabel)));
       } catch (SyntaxException e) {
@@ -166,7 +177,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     public Fragment create(ConfigurationEnvironment env, BuildOptions buildOptions)
         throws InvalidConfigurationException {
       Options options = buildOptions.get(Options.class);
-      Label sdk = RedirectChaser.followRedirects(env, options.sdk, "android_sdk");
+      Label sdk = RedirectChaser.followRedirects(env, options.realSdk(), "android_sdk");
       Label incrementalStubApplication = RedirectChaser.followRedirects(env,
           AndroidRuleClasses.DEFAULT_INCREMENTAL_STUB_APPLICATION,
           "android_incremental_stub_application");
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
index 470eab497a..1f921f41b8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
@@ -70,6 +70,10 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
         collectTransitiveProguardConfigs(ruleContext);
     AndroidIdlProvider transitiveIdlImportData = collectTransitiveIdlImports(ruleContext);
     AndroidTools tools = AndroidTools.fromRuleContext(ruleContext);
+    if (tools == null) {
+      return null;
+    }
+
     if (LocalResourceContainer.definesAndroidResources(ruleContext.attributes())) {
       try {
         if (!LocalResourceContainer.validateRuleContext(ruleContext)) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidTools.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidTools.java
index 5e57c27e04..f7a2ab8783 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidTools.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidTools.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.rules.android;
 
+import com.google.devtools.build.lib.Constants;
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
@@ -55,6 +56,11 @@ public class AndroidTools {
     AndroidSdkProvider androidSdk = androidSdkDep == null
         ? null
         : androidSdkDep.getProvider(AndroidSdkProvider.class);
+    if (androidSdk == null && !Constants.ANDROID_ALLOW_SDK_FILEGROUP) {
+      ruleContext.ruleError(
+          "No Android SDK found. Use the --android_sdk command line option to specify one.");
+      return null;
+    }
 
     return new AndroidTools(
         ruleContext,