Make --android_crosstool_top default to the android_ndk_repository specified in the WORKSPACE file.

The error reporting if an android_ndk_repository rule is present is not very user-friendly (it just uses the non-Android toolchain, resulting in compile errors) but given that --android_crosstool_top is an interim solution until we get reasonable multi-platform support, I suppose it's fine.

As a side effect, instead of prefixing fat APK output directories with "fat-apk-", we prefix Android output directories with "android-".

This makes it possible to build Android apps with zero command line options. Rejoice!

--
MOS_MIGRATED_REVID=98624120

9 files changed, 114 insertions, 15 deletions

diff --git a/examples/android/README.md b/examples/android/README.md
index 004d05f6ae..3ac5607633 100644
--- a/examples/android/README.md
+++ b/examples/android/README.md
@@ -15,11 +15,10 @@ android_ndk_repository(
 Then the following command can be used to build the example app:
 
 ```
-bazel build --android_crosstool_top=@androidndk//:toolchain //examples/android/java/bazel:hello_world
+bazel build //examples/android/java/bazel:hello_world
 ```
 
 Yes, we know that this is a little clunky. We are working on the following things (and more):
- * Eliminating the need for the `--android_crosstool_top` command line option
  * Supporting other architectures than `armeabi-v7a` and compilers other than GCC 4.9
  * Eliminating the big ugly deprecation message from the console output of Bazel
 
@@ -28,8 +27,8 @@ We also have a nice way to speed up the edit-compile-install development cycle f
  * Set the `multidex` attribute to `native`
  * Set the `dex_shards` attribute to a number between 2 and 200. This controls the size of chunks the code is split into. As this number is increased, compilation and installation becomes faster but app startup becomes slower. A good initial guess is 10.
  * Connect your device over USB to your workstation and enable USB debugging on it
- * Run `bazel mobile-install --android_crosstool_top=@androidndk//:toolchain <android_binary rule>`
+ * Run `bazel mobile-install <android_binary rule>`
  * Edit Java code or Android resources
- * Run `blaze mobile-install --android_crosstool_top=@androidndk//:toolchain --incremental <android_binary rule>`
+ * Run `blaze mobile-install --incremental <android_binary rule>`
 
 Note that if you change anything other than Java code or Android resources (C++ code or something on the device), you must omit the `--incremental` command line option. Yes, we know that this is also clunky and we are working on improving it.
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 edcdea3aec..b178b826a7 100644
--- a/src/main/java/com/google/devtools/build/lib/Constants.java
+++ b/src/main/java/com/google/devtools/build/lib/Constants.java
@@ -19,6 +19,8 @@ import com.google.common.collect.ImmutableSet;
 
 /**
  * Various constants required by Bazel.
+ * <p>The extra {@code .toString()} calls are there so that javac doesn't inline these constants
+ * so that we can replace this class file within the Bazel binary.
  */
 public class Constants {
   private Constants() {
@@ -81,5 +83,6 @@ public class Constants {
   public static final ImmutableSet<String> IOS_DEVICE_RULE_CLASSES = ImmutableSet.of("ios_device");
 
   public static final String ANDROID_DEFAULT_SDK = "//external:android/sdk".toString();
+  public static final String ANDROID_DEFAULT_CROSSTOOL = "//external:android/crosstool".toString();
   public static final String ANDROID_DEP_PREFIX = "//external:android/".toString();
 }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryRule.java
index c939241a2a..73b1091a84 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryRule.java
@@ -17,13 +17,21 @@ import static com.google.devtools.build.lib.packages.Attribute.attr;
 import static com.google.devtools.build.lib.packages.Type.INTEGER;
 import static com.google.devtools.build.lib.packages.Type.STRING;
 
+import com.google.common.base.Function;
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 import com.google.devtools.build.lib.bazel.rules.workspace.WorkspaceBaseRule;
 import com.google.devtools.build.lib.bazel.rules.workspace.WorkspaceConfiguredTargetFactory;
+import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
+import com.google.devtools.build.lib.syntax.Label;
+
+import java.util.Map;
+
+import javax.annotation.Nullable;
 
 /**
  * Definition of the {@code android_ndk_repository} rule.
@@ -31,11 +39,23 @@ import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 public class AndroidNdkRepositoryRule implements RuleDefinition {
   public static final String NAME = "android_ndk_repository";
 
+  private static final Function<? super Rule, Map<String, Label>> BINDINGS_FUNCTION =
+      new Function< Rule, Map<String, Label>>() {
+        @Nullable
+        @Override
+        public Map<String, Label> apply(Rule rule) {
+          return ImmutableMap.of(
+              "android/crosstool",
+              Label.parseAbsoluteUnchecked("@" + rule.getName() + "//:toolchain"));
+        }
+      };
+
   @Override
   public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) {
     return builder
         .setUndocumented()
         .setWorkspaceOnly()
+        .setExternalBindingsFunction(BINDINGS_FUNCTION)
         .add(attr("path", STRING).mandatory().nonconfigurable("WORKSPACE rule"))
         .add(attr("api_level", INTEGER).mandatory().nonconfigurable("WORKSPACE rule"))
         .build();
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java
index a061728520..5e97f303a0 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryRule.java
@@ -39,7 +39,7 @@ import javax.annotation.Nullable;
 public class AndroidSdkRepositoryRule implements RuleDefinition {
   public static final String NAME = "android_sdk_repository";
 
-  public static final Function<? super Rule, Map<String, Label>> BINDINGS_FUNCTION =
+  private static final Function<? super Rule, Map<String, Label>> BINDINGS_FUNCTION =
       new Function< Rule, Map<String, Label>>() {
         @Nullable
         @Override
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
index ccc6a56e1e..e59df8ea64 100644
--- 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
@@ -12,3 +12,4 @@ 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/sdk")
+bind(name = "android/crosstool", actual = "//tools/cpp:toolchain")
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 ee55622edb..0383f03191 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
@@ -31,6 +31,7 @@ import com.google.devtools.build.lib.analysis.config.InvalidConfigurationExcepti
 import com.google.devtools.build.lib.packages.Attribute.SplitTransition;
 import com.google.devtools.build.lib.syntax.Label;
 import com.google.devtools.common.options.Converters;
+import com.google.devtools.common.options.EnumConverter;
 import com.google.devtools.common.options.Option;
 
 import java.util.List;
@@ -40,9 +41,48 @@ import java.util.List;
  */
 public class AndroidConfiguration extends BuildConfiguration.Fragment {
   /**
+   * Value used to avoid multiple configurations from conflicting.
+   *
+   * <p>This is set to {@code ANDROID} in Android configurations and to {@code MAIN} otherwise. This
+   * influences the output directory name: if it didn't, an Android and a non-Android configuration
+   * would conflict if they had the same toolchain identifier.
+   *
+   * <p>Note that this is not just a theoretical concern: even if {@code --crosstool_top} and
+   * {@code --android_crosstool_top} point to different labels, they may end up being redirected to
+   * the same thing, and this is exactly what happens on OSX X.
+   */
+  public enum ConfigurationDistinguisher {
+    MAIN(null),
+    ANDROID("android");
+
+    private final String suffix;
+
+    private ConfigurationDistinguisher(String suffix) {
+      this.suffix = suffix;
+    };
+  }
+
+  /**
+   * Converter for {@link com.google.devtools.build.lib.rules.android.AndroidConfiguration.ConfigurationDistinguisher}
+   */
+  public static final class ConfigurationDistinguisherConverter
+      extends EnumConverter<ConfigurationDistinguisher> {
+    public ConfigurationDistinguisherConverter() {
+      super(ConfigurationDistinguisher.class, "Android configuration distinguisher");
+    }
+  }
+
+  /**
    * Android configuration options.
    */
   public static class Options extends FragmentOptions {
+    // Spaces make it impossible to specify this on the command line
+    @Option(name = "Android configuration distinguisher",
+        defaultValue = "MAIN",
+        converter = ConfigurationDistinguisherConverter.class,
+        category = "undocumented")
+    public ConfigurationDistinguisher configurationDistinguisher;
+
     @Option(name = "android_crosstool_top",
         defaultValue = "null",
         category = "semantics",
@@ -120,8 +160,8 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
         labelMap.put("android_proguard", proguard);
       }
 
-      if (androidCrosstoolTop != null) {
-        labelMap.put("android_crosstool_top", androidCrosstoolTop);
+      if (realAndroidCrosstoolTop() != null) {
+        labelMap.put("android_crosstool_top", realAndroidCrosstoolTop());
       }
 
       labelMap.put("android_sdk", realSdk());
@@ -136,6 +176,21 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
           : sdk;
     }
 
+    // This method is here because Constants.ANDROID_DEFAULT_CROSSTOOL 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.
+    public Label realAndroidCrosstoolTop() {
+      if (androidCrosstoolTop != null) {
+        return androidCrosstoolTop;
+      }
+
+      if (Constants.ANDROID_DEFAULT_CROSSTOOL.equals("null")) {
+        return null;
+      }
+
+      return Label.parseAbsoluteUnchecked(Constants.ANDROID_DEFAULT_CROSSTOOL);
+    }
+
     @Override
     public ImmutableList<String> getDefaultsRules() {
       return ImmutableList.of("android_tools_defaults_jar(name = 'android_jar')");
@@ -173,6 +228,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
   private final boolean legacyNativeSupport;
   private final String cpu;
   private final boolean fatApk;
+  private final ConfigurationDistinguisher configurationDistinguisher;
   private final Label proguard;
   private final boolean useJackForDexing;
   private final boolean jackSanityChecks;
@@ -183,6 +239,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     this.legacyNativeSupport = options.legacyNativeSupport;
     this.cpu = options.cpu;
     this.fatApk = !options.fatApkCpus.isEmpty();
+    this.configurationDistinguisher = options.configurationDistinguisher;
     this.proguard = options.proguard;
     this.useJackForDexing = options.useJackForDexing;
     this.jackSanityChecks = options.jackSanityChecks;
@@ -230,7 +287,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
 
   @Override
   public String getOutputDirectoryName() {
-    return fatApk ? "fat-apk" : null;
+    return configurationDistinguisher.suffix;
   }
 
   public Label getProguardLabel() {
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 269a46cf4b..0f576bab21 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
@@ -45,6 +45,7 @@ import com.google.devtools.build.lib.packages.RuleClass.Builder;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 import com.google.devtools.build.lib.packages.TriState;
 import com.google.devtools.build.lib.packages.Type;
+import com.google.devtools.build.lib.rules.android.AndroidConfiguration.ConfigurationDistinguisher;
 import com.google.devtools.build.lib.rules.cpp.CppOptions;
 import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
 import com.google.devtools.build.lib.rules.java.JavaSemantics;
@@ -178,21 +179,31 @@ public final class AndroidRuleClasses {
     }
 
     private void setCrosstoolToAndroid(BuildOptions output, BuildOptions input) {
-      AndroidConfiguration.Options androidOptions = input.get(AndroidConfiguration.Options.class);
+      AndroidConfiguration.Options inputAndroidOptions =
+          input.get(AndroidConfiguration.Options.class);
+      AndroidConfiguration.Options outputAndroidOptions =
+          output.get(AndroidConfiguration.Options.class);
+
       CppOptions cppOptions = output.get(CppOptions.class);
-      if (androidOptions.androidCrosstoolTop != null) {
+      if (inputAndroidOptions.realAndroidCrosstoolTop() != null
+          && !cppOptions.crosstoolTop.equals(inputAndroidOptions.realAndroidCrosstoolTop())) {
         if (cppOptions.hostCrosstoolTop == null) {
           cppOptions.hostCrosstoolTop = cppOptions.crosstoolTop;
         }
-        cppOptions.crosstoolTop = androidOptions.androidCrosstoolTop;
+        cppOptions.crosstoolTop = inputAndroidOptions.realAndroidCrosstoolTop();
       }
+
+      outputAndroidOptions.configurationDistinguisher = ConfigurationDistinguisher.ANDROID;
     }
 
     @Override
     public List<BuildOptions> split(BuildOptions buildOptions) {
       AndroidConfiguration.Options androidOptions =
           buildOptions.get(AndroidConfiguration.Options.class);
-      if (androidOptions.fatApkCpus.isEmpty() && androidOptions.androidCrosstoolTop == null) {
+      CppOptions cppOptions = buildOptions.get(CppOptions.class);
+      Label androidCrosstoolTop = androidOptions.realAndroidCrosstoolTop();
+      if (androidOptions.fatApkCpus.isEmpty()
+          && (androidCrosstoolTop == null || androidCrosstoolTop.equals(cppOptions.crosstoolTop))) {
         return ImmutableList.of();
       }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java
index 0a927afb15..9f405e7b53 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java
@@ -151,7 +151,7 @@ public class ObjcCommandLineOptions extends FragmentOptions {
   // transitions for this purpose.
   // TODO(bazel-team): Remove this once we have dynamic configurations but make sure that different
   // configurations (e.g. by min os version) always use different output paths.
-  @Option(name = "DO_NOT_USE_configuration_distinguisher",
+  @Option(name = "iOS configuration distinguisher",
       defaultValue = "UNKNOWN",
       converter = ConfigurationDistinguisherConverter.class,
       category = "undocumented")
@@ -189,7 +189,7 @@ public class ObjcCommandLineOptions extends FragmentOptions {
   public static final class ConfigurationDistinguisherConverter
       extends EnumConverter<ConfigurationDistinguisher> {
     public ConfigurationDistinguisherConverter() {
-      super(ConfigurationDistinguisher.class, "configuration distinguisher");
+      super(ConfigurationDistinguisher.class, "Objective C configuration distinguisher");
     }
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
index 7c922494e2..a569a7a98d 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java
@@ -23,11 +23,15 @@ import com.google.devtools.build.lib.analysis.util.AnalysisMock;
 import com.google.devtools.build.lib.bazel.rules.BazelConfiguration;
 import com.google.devtools.build.lib.bazel.rules.BazelConfigurationCollection;
 import com.google.devtools.build.lib.bazel.rules.BazelRuleClassProvider;
+import com.google.devtools.build.lib.bazel.rules.python.BazelPythonConfiguration;
 import com.google.devtools.build.lib.packages.util.MockToolsConfig;
+import com.google.devtools.build.lib.rules.android.AndroidConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CppConfigurationLoader;
+import com.google.devtools.build.lib.rules.java.J2ObjcConfiguration;
 import com.google.devtools.build.lib.rules.java.JavaConfigurationLoader;
 import com.google.devtools.build.lib.rules.java.JvmConfigurationLoader;
 import com.google.devtools.build.lib.rules.objc.ObjcConfigurationLoader;
+import com.google.devtools.build.lib.rules.python.PythonConfigurationLoader;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 
@@ -107,9 +111,13 @@ public class BazelAnalysisMock extends AnalysisMock {
     return new ConfigurationFactory(new BazelConfigurationCollection(),
         new BazelConfiguration.Loader(),
         new CppConfigurationLoader(Functions.<String>identity()),
+        new PythonConfigurationLoader(Functions.<String>identity()),
+        new BazelPythonConfiguration.Loader(),
         new JvmConfigurationLoader(false, BazelRuleClassProvider.JAVA_CPU_SUPPLIER),
         new JavaConfigurationLoader(),
-        new ObjcConfigurationLoader());
+        new ObjcConfigurationLoader(),
+        new J2ObjcConfiguration.Loader(),
+        new AndroidConfiguration.Loader());
   }
 
   @Override