Move Jack's tools into AndroidSdkProvider.

This also eliminates the JackRule, which is no longer necessary, as its sole
purpose was to allow Jack's tools to be found. This is now part of
AndroidSdkProvider, like all other Android tools.

For now, the new attributes on android_sdk are optional and default to their
old values. In the future, the new attributes will become mandatory with no
defaults, like the other attributes on that rule.

Also fixes a bug where errors found during AndroidCommon.initJava would not
result in a null return from init, previously obscured by the early return
from initJack.

--
MOS_MIGRATED_REVID=98305022

7 files changed, 123 insertions, 74 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
index efd9939aea..80959767da 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java
@@ -296,7 +296,6 @@ public class BazelRuleClassProvider {
     builder.addRuleDefinition(new AndroidRuleClasses.AndroidAaptBaseRule());
     builder.addRuleDefinition(new AndroidRuleClasses.AndroidResourceSupportRule());
     builder.addRuleDefinition(new AndroidRuleClasses.AndroidBinaryBaseRule());
-    builder.addRuleDefinition(new AndroidRuleClasses.JackRule());
     builder.addRuleDefinition(new AndroidBinaryOnlyRule());
     builder.addRuleDefinition(new AndroidLibraryBaseRule());
     builder.addRuleDefinition(new BazelAndroidLibraryRule());
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
index 2ee1aeb317..3533f227bc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
@@ -332,20 +332,29 @@ public class AndroidCommon {
     }
 
     JavaCompilationHelper helper = initAttributes(attributes, javaSemantics);
+    if (ruleContext.hasErrors()) {
+      return null;
+    }
 
     if (addCoverageSupport) {
       androidSemantics.addCoverageSupport(ruleContext, this, javaSemantics, true,
           attributes, artifactsBuilder);
+      if (ruleContext.hasErrors()) {
+        return null;
+      }
     }
 
     jackCompilationHelper = initJack(helper.getAttributes(), javaSemantics);
-    if (jackCompilationHelper == null) {
+    if (ruleContext.hasErrors()) {
       return null;
     }
 
     initJava(
         helper, artifactsBuilder, collectJavaCompilationArgs, resourceApk.getResourceJavaSrcJar(),
         genClassJarImplicitOutput);
+    if (ruleContext.hasErrors()) {
+      return null;
+    }
     return helper.getAttributes();
   }
 
@@ -403,6 +412,7 @@ public class AndroidCommon {
     for (Artifact resource : attributes.getResources()) {
       resourcesMap.put(javaSemantics.getJavaResourcePath(resource.getRootRelativePath()), resource);
     }
+    AndroidSdkProvider sdk = AndroidSdkProvider.fromRuleContext(ruleContext);
     return new JackCompilationHelper.Builder()
         // blaze infrastructure
         .setRuleContext(ruleContext)
@@ -410,7 +420,7 @@ public class AndroidCommon {
         .setOutputArtifact(
             ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_LIBRARY_JACK_FILE))
         // tools
-        .setAndroidBaseLibraryForJack(ruleContext.getHostPrerequisiteArtifact("$android_jack"))
+        .setAndroidSdk(sdk)
         // sources
         .addJavaSources(attributes.getSourceFiles())
         .addSourceJars(attributes.getSourceJars())
@@ -467,7 +477,7 @@ public class AndroidCommon {
     gensrcJar = helper.createGensrcJar(classJar);
     Artifact manifestProtoOutput = helper.createManifestProtoOutput(classJar);
 
-    // AndroidBinary will pass its -gen.jar output, and AndroidLibrary will pass its own. 
+    // AndroidBinary will pass its -gen.jar output, and AndroidLibrary will pass its own.
     genJar = ruleContext.getImplicitOutputArtifact(genClassJarImplicitOutput);
     helper.createGenJarAction(classJar, manifestProtoOutput, genJar);
 
@@ -493,7 +503,7 @@ public class AndroidCommon {
     javaCommon.setClassPathFragment(
         new ClasspathConfiguredFragment(
             javaCommon.getJavaCompilationArtifacts(), attributes, asNeverLink));
-    
+
     transitiveNeverlinkLibraries = collectTransitiveNeverlinkLibraries(
         ruleContext,
         javaCommon.getDependencies(),
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 925dd39f74..1cb6a45697 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
@@ -30,7 +30,6 @@ import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
 import com.google.devtools.build.lib.rules.android.AndroidRuleClasses.AndroidAaptBaseRule;
 import com.google.devtools.build.lib.rules.android.AndroidRuleClasses.AndroidBaseRule;
 import com.google.devtools.build.lib.rules.android.AndroidRuleClasses.AndroidResourceSupportRule;
-import com.google.devtools.build.lib.rules.android.AndroidRuleClasses.JackRule;
 import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
 import com.google.devtools.build.lib.rules.java.JavaSemantics;
 
@@ -171,8 +170,7 @@ public final class AndroidLibraryBaseRule implements RuleDefinition {
         .ancestors(
             AndroidBaseRule.class,
             AndroidAaptBaseRule.class,
-            AndroidResourceSupportRule.class,
-            JackRule.class)
+            AndroidResourceSupportRule.class)
         .build();
   }
 }
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 066b37c0f1..269a46cf4b 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
@@ -319,10 +319,35 @@ public final class AndroidRuleClasses {
           .add(attr("aidl", LABEL).mandatory().cfg(HOST).allowedFileTypes(ANY_FILE).exec())
           .add(attr("android_jar", LABEL).mandatory().cfg(HOST).allowedFileTypes(ANY_FILE))
           .add(attr("shrinked_android_jar", LABEL).mandatory().cfg(HOST).allowedFileTypes(ANY_FILE))
+          .add(
+              attr("android_jack", LABEL)
+                  .cfg(HOST)
+                  .allowedFileTypes(ANY_FILE)
+                  // TODO(bazel-team): Remove defaults and make mandatory when android_sdk targets
+                  // have been updated to include manually specified Jack attributes.
+                  .value(environment.getLabel("//tools/android/jack:android_jack")))
           .add(attr("annotations_jar", LABEL).mandatory().cfg(HOST).allowedFileTypes(ANY_FILE))
           .add(attr("main_dex_classes", LABEL).mandatory().cfg(HOST).allowedFileTypes(ANY_FILE))
           .add(attr("apkbuilder", LABEL).mandatory().cfg(HOST).allowedFileTypes(ANY_FILE).exec())
           .add(attr("zipalign", LABEL).mandatory().cfg(HOST).allowedFileTypes(ANY_FILE).exec())
+          .add(
+              attr("jack", LABEL)
+                  .cfg(HOST)
+                  .allowedFileTypes(ANY_FILE)
+                  .exec()
+                  .value(environment.getLabel("//tools/android/jack:jack")))
+          .add(
+              attr("jill", LABEL)
+                  .cfg(HOST)
+                  .allowedFileTypes(ANY_FILE)
+                  .exec()
+                  .value(environment.getLabel("//tools/android/jack:jill")))
+          .add(
+              attr("resource_extractor", LABEL)
+                  .cfg(HOST)
+                  .allowedFileTypes(ANY_FILE)
+                  .exec()
+                  .value(environment.getLabel("//tools/android/jack:resource_extractor")))
           .build();
     }
 
@@ -515,44 +540,6 @@ public final class AndroidRuleClasses {
   }
 
   /**
-   * Mixin for rules which compile with Jack and Jill.
-   *
-   * @see JackCompilationHelper
-   * @see GoogleJavaSemantics#collectJackLibraries
-   */
-  public static final class JackRule implements RuleDefinition {
-    @Override
-    public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
-      return builder
-          .add(attr("$jack", LABEL)
-              .cfg(HOST)
-              .exec()
-              .value(env.getLabel("//tools/android/jack:jack")))
-          .add(attr("$jill", LABEL)
-              .cfg(HOST)
-              .exec()
-              .value(env.getLabel("//tools/android/jack:jill")))
-          .add(attr("$resource_extractor", LABEL)
-              .cfg(HOST)
-              .exec()
-              .value(env.getLabel("//tools/android/jack:resource_extractor")))
-          .add(attr("$android_jack", LABEL)
-              .cfg(HOST)
-              .singleArtifact()
-              .value(env.getLabel("//tools/android/jack:android_jack")))
-          .build();
-    }
-
-    @Override
-    public Metadata getMetadata() {
-      return RuleDefinition.Metadata.builder()
-          .name("$jack_mixin")
-          .type(RuleClassType.ABSTRACT)
-          .build();
-    }
-  }
-
-  /**
    * Base class for Android rule definitions that produce binaries.
    */
   public static final class AndroidBinaryBaseRule implements RuleDefinition {
@@ -705,8 +692,7 @@ com/google/common/base/Objects.class
           .ancestors(
               AndroidRuleClasses.AndroidBaseRule.class,
               AndroidAaptBaseRule.class,
-              AndroidResourceSupportRule.class,
-              JackRule.class)
+              AndroidResourceSupportRule.class)
           .build();
       }
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSdk.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSdk.java
index 2afaacaede..827aeb2c0f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSdk.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSdk.java
@@ -46,10 +46,15 @@ public class AndroidSdk implements RuleConfiguredTargetFactory {
     FilesToRunProvider mainDexListCreator = ruleContext.getExecutablePrerequisite(
         "main_dex_list_creator", Mode.HOST);
     FilesToRunProvider zipalign = ruleContext.getExecutablePrerequisite("zipalign", Mode.HOST);
+    FilesToRunProvider jack = ruleContext.getExecutablePrerequisite("jack", Mode.HOST);
+    FilesToRunProvider jill = ruleContext.getExecutablePrerequisite("jill", Mode.HOST);
+    FilesToRunProvider resourceExtractor =
+        ruleContext.getExecutablePrerequisite("resource_extractor", Mode.HOST);
     Artifact frameworkAidl = ruleContext.getPrerequisiteArtifact("framework_aidl", Mode.HOST);
     Artifact androidJar = ruleContext.getPrerequisiteArtifact("android_jar", Mode.HOST);
     Artifact shrinkedAndroidJar =
         ruleContext.getPrerequisiteArtifact("shrinked_android_jar", Mode.HOST);
+    Artifact androidJack = ruleContext.getPrerequisiteArtifact("android_jack", Mode.HOST);
     Artifact annotationsJar = ruleContext.getPrerequisiteArtifact("annotations_jar", Mode.HOST);
     Artifact mainDexClasses = ruleContext.getPrerequisiteArtifact("main_dex_classes", Mode.HOST);
 
@@ -58,9 +63,26 @@ public class AndroidSdk implements RuleConfiguredTargetFactory {
     }
 
     return new RuleConfiguredTargetBuilder(ruleContext)
-        .add(AndroidSdkProvider.class, new AndroidSdkProvider(
-            frameworkAidl, androidJar, shrinkedAndroidJar, annotationsJar, mainDexClasses,
-            adb, dx, mainDexListCreator, aidl, aapt, apkBuilder, proguard, zipalign))
+        .add(
+            AndroidSdkProvider.class,
+            new AndroidSdkProvider(
+                frameworkAidl,
+                androidJar,
+                shrinkedAndroidJar,
+                androidJack,
+                annotationsJar,
+                mainDexClasses,
+                adb,
+                dx,
+                mainDexListCreator,
+                aidl,
+                aapt,
+                apkBuilder,
+                proguard,
+                zipalign,
+                jack,
+                jill,
+                resourceExtractor))
         .add(RunfilesProvider.class, RunfilesProvider.EMPTY)
         .setFilesToBuild(NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER))
         .build();
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
index 87121e3708..6b0169b896 100644
--- 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
@@ -29,6 +29,7 @@ public final class AndroidSdkProvider implements TransitiveInfoProvider {
   private final Artifact frameworkAidl;
   private final Artifact androidJar;
   private final Artifact shrinkedAndroidJar;
+  private final Artifact androidJack;
   private final Artifact annotationsJar;
   private final Artifact mainDexClasses;
   private final FilesToRunProvider adb;
@@ -39,17 +40,32 @@ public final class AndroidSdkProvider implements TransitiveInfoProvider {
   private final FilesToRunProvider apkBuilder;
   private final FilesToRunProvider proguard;
   private final FilesToRunProvider zipalign;
+  private final FilesToRunProvider jack;
+  private final FilesToRunProvider jill;
+  private final FilesToRunProvider resourceExtractor;
 
   public AndroidSdkProvider(
-      Artifact frameworkAidl, Artifact androidJar, Artifact shrinkedAndroidJar,
-      Artifact annotationsJar, Artifact mainDexClasses,
-      FilesToRunProvider adb, FilesToRunProvider dx,
+      Artifact frameworkAidl,
+      Artifact androidJar,
+      Artifact shrinkedAndroidJar,
+      Artifact androidJack,
+      Artifact annotationsJar,
+      Artifact mainDexClasses,
+      FilesToRunProvider adb,
+      FilesToRunProvider dx,
       FilesToRunProvider mainDexListCreator,
-      FilesToRunProvider aidl, FilesToRunProvider aapt, FilesToRunProvider apkBuilder,
-      FilesToRunProvider proguard, FilesToRunProvider zipalign) {
+      FilesToRunProvider aidl,
+      FilesToRunProvider aapt,
+      FilesToRunProvider apkBuilder,
+      FilesToRunProvider proguard,
+      FilesToRunProvider zipalign,
+      FilesToRunProvider jack,
+      FilesToRunProvider jill,
+      FilesToRunProvider resourceExtractor) {
     this.frameworkAidl = frameworkAidl;
     this.androidJar = androidJar;
     this.shrinkedAndroidJar = shrinkedAndroidJar;
+    this.androidJack = androidJack;
     this.annotationsJar = annotationsJar;
     this.mainDexClasses = mainDexClasses;
     this.adb = adb;
@@ -60,6 +76,9 @@ public final class AndroidSdkProvider implements TransitiveInfoProvider {
     this.apkBuilder = apkBuilder;
     this.proguard = proguard;
     this.zipalign = zipalign;
+    this.jack = jack;
+    this.jill = jill;
+    this.resourceExtractor = resourceExtractor;
   }
 
   /**
@@ -100,6 +119,10 @@ public final class AndroidSdkProvider implements TransitiveInfoProvider {
     return shrinkedAndroidJar;
   }
 
+  public Artifact getAndroidJack() {
+    return androidJack;
+  }
+
   public Artifact getAnnotationsJar() {
     return annotationsJar;
   }
@@ -139,4 +162,16 @@ public final class AndroidSdkProvider implements TransitiveInfoProvider {
   public FilesToRunProvider getZipalign() {
     return zipalign;
   }
+
+  public FilesToRunProvider getJack() {
+    return jack;
+  }
+
+  public FilesToRunProvider getJill() {
+    return jill;
+  }
+
+  public FilesToRunProvider getResourceExtractor() {
+    return resourceExtractor;
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java
index 247a37ffeb..d8377d218e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/JackCompilationHelper.java
@@ -25,7 +25,6 @@ import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.FileProvider;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
-import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
@@ -550,8 +549,8 @@ public final class JackCompilationHelper {
     /** Rule context used to build and register actions. */
     @Nullable private RuleContext ruleContext;
 
-    /** Jack library containing Android base classes. */
-    @Nullable private Artifact androidBaseLibraryForJack;
+    /** Set of Android tools used to pick up the Jack tools. */
+    @Nullable private AndroidSdkProvider androidSdk;
 
     /** The destination for the Jack artifact to be created. */
     @Nullable private Artifact outputArtifact;
@@ -625,11 +624,11 @@ public final class JackCompilationHelper {
     }
 
     /**
-     * Sets the artifact representing android.jack, to supply base libraries to Jack compilation.
+     * Sets the tools bundle containing Jack, Jill, the resource extractor, and the Android base
+     * library in Jack format.
      */
-    public JackCompilationHelper.Builder setAndroidBaseLibraryForJack(
-        Artifact androidBaseLibraryForJack) {
-      this.androidBaseLibraryForJack = Preconditions.checkNotNull(androidBaseLibraryForJack);
+    public JackCompilationHelper.Builder setAndroidSdk(AndroidSdkProvider androidSdk) {
+      this.androidSdk = Preconditions.checkNotNull(androidSdk);
       return this;
     }
 
@@ -803,25 +802,25 @@ public final class JackCompilationHelper {
      * JackCompilationHelpers will attempt to generate the same actions.
      */
     public JackCompilationHelper build() {
+      Preconditions.checkNotNull(ruleContext);
+      Preconditions.checkNotNull(androidSdk);
+
       boolean useSanityChecks =
           ruleContext
               .getConfiguration()
               .getFragment(AndroidConfiguration.class)
               .isJackSanityChecked();
-      FilesToRunProvider jackBinary = ruleContext.getExecutablePrerequisite("$jack", Mode.HOST);
-      FilesToRunProvider jillBinary = ruleContext.getExecutablePrerequisite("$jill", Mode.HOST);
-      FilesToRunProvider resourceExtractorBinary =
-          ruleContext.getExecutablePrerequisite("$resource_extractor", Mode.HOST);
-      if (ruleContext.hasErrors()) {
-        return null;
-      }
+      FilesToRunProvider jackBinary = androidSdk.getJack();
+      FilesToRunProvider jillBinary = androidSdk.getJill();
+      FilesToRunProvider resourceExtractorBinary = androidSdk.getResourceExtractor();
+      Artifact androidBaseLibraryForJack = androidSdk.getAndroidJack();
 
       return new JackCompilationHelper(
-          Preconditions.checkNotNull(ruleContext),
+          ruleContext,
           useSanityChecks,
-          resourceExtractorBinary,
-          jackBinary,
-          jillBinary,
+          Preconditions.checkNotNull(resourceExtractorBinary),
+          Preconditions.checkNotNull(jackBinary),
+          Preconditions.checkNotNull(jillBinary),
           Preconditions.checkNotNull(androidBaseLibraryForJack),
           Preconditions.checkNotNull(outputArtifact),
           ImmutableSet.copyOf(javaSources),