Refactoring the ApplicationManifest for clarity:

    * Split the aar, library and binary packWith calls.
    * Remove unused argument from the new binary and library packWith calls.
RELNOTES: None

PiperOrigin-RevId: 160414867

4 files changed, 186 insertions, 122 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java
index 6d1629e563..6b9663bba0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java
@@ -101,7 +101,7 @@ public class AarImport implements RuleConfiguredTargetFactory {
         ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_ZIP);
 
     ResourceApk resourceApk =
-        androidManifest.packWithDataAndResources(
+        androidManifest.packAarWithDataAndResources(
             ruleContext,
             new LocalResourceContainer.Builder(ruleContext)
                 .withResources(ImmutableList.of(resourcesProvider))
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 a89b9dbd67..efe7c54973 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
@@ -225,91 +225,64 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
                   .getApk()
               : null;
 
-      resourceApk = applicationManifest.packWithDataAndResources(
-          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_APK),
-          ruleContext,
-          false, /* isLibrary */
-          resourceDeps,
-          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
-          null, /* Artifact symbolsTxt */
-          ResourceFilter.fromRuleContext(ruleContext),
-          ruleContext.getTokenizedStringListAttr("nocompress_extensions"),
-          ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
-          false, /* incremental */
-          ProguardHelper.getProguardConfigArtifact(ruleContext, ""),
-          createMainDexProguardSpec(ruleContext),
-          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_PROCESSED_MANIFEST),
-          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_ZIP),
-          DataBinding.isEnabled(ruleContext) ? DataBinding.getLayoutInfoFile(ruleContext) : null,
-          featureOfArtifact,
-          featureAfterArtifact);
-      ruleContext.assertNoErrors();
-
-      incrementalResourceApk = applicationManifest
-          .addMobileInstallStubApplication(ruleContext)
-          .packWithDataAndResources(
-              ruleContext
-                  .getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_INCREMENTAL_RESOURCES_APK),
+      resourceApk =
+          applicationManifest.packBinaryWithDataAndResources(
               ruleContext,
-              false, /* isLibrary */
+              ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_APK),
               resourceDeps,
-              null, /* Artifact rTxt */
-              null, /* Artifact symbolsTxt */
+              ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
               ResourceFilter.fromRuleContext(ruleContext),
               ruleContext.getTokenizedStringListAttr("nocompress_extensions"),
               ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
-              true, /* incremental */
-              ProguardHelper.getProguardConfigArtifact(ruleContext, "incremental"),
-              null, /* mainDexProguardCfg */
-              null, /* manifestOut */
-              null, /* mergedResourcesOut */
-              null, /* dataBindingInfoZip */
-              null, /* featureOf */
-              null /* featureAfter */);
+              ProguardHelper.getProguardConfigArtifact(ruleContext, ""),
+              createMainDexProguardSpec(ruleContext),
+              ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_PROCESSED_MANIFEST),
+              ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_ZIP),
+              DataBinding.isEnabled(ruleContext)
+                  ? DataBinding.getLayoutInfoFile(ruleContext)
+                  : null,
+              featureOfArtifact,
+              featureAfterArtifact);
       ruleContext.assertNoErrors();
 
-      instantRunResourceApk = applicationManifest
-          .addInstantRunStubApplication(ruleContext)
-          .packWithDataAndResources(
-              getDxArtifact(ruleContext, "android_instant_run.ap_"),
-              ruleContext,
-              false, /* isLibrary */
-              resourceDeps,
-              null, /* Artifact rTxt */
-              null, /* Artifact symbolsTxt */
-              ResourceFilter.fromRuleContext(ruleContext),
-              ruleContext.getTokenizedStringListAttr("nocompress_extensions"),
-              ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
-              true, /* incremental */
-              ProguardHelper.getProguardConfigArtifact(ruleContext, "instant_run"),
-              null, /* mainDexProguardCfg */
-              null, /* manifestOut */
-              null, /* mergedResourcesOut */
-              null, /* dataBindingInfoZip */
-              null, /* featureOf */
-              null /* featureAfter */);
+      incrementalResourceApk =
+          applicationManifest
+              .addMobileInstallStubApplication(ruleContext)
+              .packIncrementalBinaryWithDataAndResources(
+                  ruleContext,
+                  ruleContext.getImplicitOutputArtifact(
+                      AndroidRuleClasses.ANDROID_INCREMENTAL_RESOURCES_APK),
+                  resourceDeps,
+                  ResourceFilter.fromRuleContext(ruleContext),
+                  ruleContext.getTokenizedStringListAttr("nocompress_extensions"),
+                  ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
+                  ProguardHelper.getProguardConfigArtifact(ruleContext, "incremental"));
       ruleContext.assertNoErrors();
 
-      splitResourceApk = applicationManifest
-          .createSplitManifest(ruleContext, "android_resources", false)
-          .packWithDataAndResources(
-              getDxArtifact(ruleContext, "android_resources.ap_"),
-              ruleContext,
-              false, /* isLibrary */
-              resourceDeps,
-              null, /* Artifact rTxt */
-              null, /* Artifact symbolsTxt */
-              ResourceFilter.fromRuleContext(ruleContext),
-              ruleContext.getTokenizedStringListAttr("nocompress_extensions"),
-              ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
-              true, /* incremental */
-              ProguardHelper.getProguardConfigArtifact(ruleContext, "incremental_split"),
-              null, /* mainDexProguardCfg */
-              null, /* manifestOut */
-              null, /* mergedResourcesOut */
-              null, /* dataBindingInfoZip */
-              null, /* featureOf */
-              null /* featureAfter */);
+      instantRunResourceApk =
+          applicationManifest
+              .addInstantRunStubApplication(ruleContext)
+              .packIncrementalBinaryWithDataAndResources(
+                  ruleContext,
+                  getDxArtifact(ruleContext, "android_instant_run.ap_"),
+                  resourceDeps,
+                  ResourceFilter.fromRuleContext(ruleContext),
+                  ruleContext.getTokenizedStringListAttr("nocompress_extensions"),
+                  ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
+                  ProguardHelper.getProguardConfigArtifact(ruleContext, "instant_run"));
+      ruleContext.assertNoErrors();
+
+      splitResourceApk =
+          applicationManifest
+              .createSplitManifest(ruleContext, "android_resources", false)
+              .packIncrementalBinaryWithDataAndResources(
+                  ruleContext,
+                  getDxArtifact(ruleContext, "android_resources.ap_"),
+                  resourceDeps,
+                  ResourceFilter.fromRuleContext(ruleContext),
+                  ruleContext.getTokenizedStringListAttr("nocompress_extensions"),
+                  ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
+                  ProguardHelper.getProguardConfigArtifact(ruleContext, "incremental_split"));
       ruleContext.assertNoErrors();
 
     } else {
@@ -981,7 +954,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
 
   private static void createApkManifestAction(
       RuleContext ruleContext,
-      Artifact apkManfiest,
+      Artifact apkManifest,
       boolean textProto,
       final AndroidCommon androidCommon,
       JavaTargetAttributes resourceClasses,
@@ -1010,15 +983,16 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
 
     AndroidSdkProvider sdk = AndroidSdkProvider.fromRuleContext(ruleContext);
 
-    ApkManifestAction manifestAction = new ApkManifestAction(
-        ruleContext.getActionOwner(),
-        apkManfiest,
-        textProto,
-        sdk,
-        filteredJars,
-        resourceApk,
-        nativeLibs,
-        debugKeystore);
+    ApkManifestAction manifestAction =
+        new ApkManifestAction(
+            ruleContext.getActionOwner(),
+            apkManifest,
+            textProto,
+            sdk,
+            filteredJars,
+            resourceApk,
+            nativeLibs,
+            debugKeystore);
 
     ruleContext.registerAction(manifestAction);
   }
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 a6d7f39718..1252f292d5 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
@@ -91,24 +91,19 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
     if (definesLocalResources) {
       ApplicationManifest applicationManifest = androidSemantics.getManifestForRule(ruleContext)
           .renamePackage(ruleContext, AndroidCommon.getJavaPackage(ruleContext));
-      resourceApk = applicationManifest.packWithDataAndResources(
-          null, /* resourceApk -- not needed for library */
-          ruleContext,
-          true, /* isLibrary */
-          ResourceDependencies.fromRuleDeps(ruleContext, JavaCommon.isNeverLink(ruleContext)),
-          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
-          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_MERGED_SYMBOLS),
-          ResourceFilter.empty(ruleContext),
-          ImmutableList.<String>of(), /* uncompressedExtensions */
-          false, /* crunchPng */
-          false, /* incremental */
-          null, /* proguardCfgOut */
-          null, /* mainDexProguardCfg */
-          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_PROCESSED_MANIFEST),
-          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_ZIP),
-          DataBinding.isEnabled(ruleContext) ? DataBinding.getLayoutInfoFile(ruleContext) : null,
-          null, /* featureOf */
-          null /* featureAfter */);
+      resourceApk =
+          applicationManifest.packLibraryWithDataAndResources(
+              ruleContext,
+              null /* resourceApk, optional */,
+              ResourceDependencies.fromRuleDeps(ruleContext, JavaCommon.isNeverLink(ruleContext)),
+              ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
+              ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_MERGED_SYMBOLS),
+              ResourceFilter.empty(ruleContext),
+              ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_PROCESSED_MANIFEST),
+              ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_ZIP),
+              DataBinding.isEnabled(ruleContext)
+                  ? DataBinding.getLayoutInfoFile(ruleContext)
+                  : null);
       if (ruleContext.hasErrors()) {
         return null;
       }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
index 6b550d7504..152548be26 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
@@ -293,15 +293,15 @@ public final class ApplicationManifest {
     return new ApplicationManifest(ruleContext, outputManifest);
   }
 
-  /** Packages up the manifest with assets from the rule and dependent resources.
-   * @throws InterruptedException */
+  /** Packages up the manifest with assets from the rule and dependent resources. */
   public ResourceApk packWithAssets(
       Artifact resourceApk,
       RuleContext ruleContext,
       ResourceDependencies resourceDeps,
       Artifact rTxt,
       boolean incremental,
-      Artifact proguardCfg) throws InterruptedException {
+      Artifact proguardCfg)
+      throws InterruptedException {
     LocalResourceContainer data = new LocalResourceContainer.Builder(ruleContext)
         .withAssets(
             AndroidCommon.getAssetDir(ruleContext),
@@ -336,7 +336,7 @@ public final class ApplicationManifest {
   }
 
   /** Packages up the manifest with resource and assets from the LocalResourceContainer. */
-  public ResourceApk packWithDataAndResources(
+  public ResourceApk packAarWithDataAndResources(
       RuleContext ruleContext,
       LocalResourceContainer data,
       ResourceDependencies resourceDeps,
@@ -344,7 +344,8 @@ public final class ApplicationManifest {
       Artifact symbols,
       Artifact manifestOut,
       Artifact mergedResources,
-      boolean alwaysExportManifest) throws InterruptedException {
+      boolean alwaysExportManifest)
+      throws InterruptedException {
     if (ruleContext.hasErrors()) {
       return null;
     }
@@ -377,26 +378,70 @@ public final class ApplicationManifest {
         null /* Artifact featureAfter */);
   }
 
+  /* Creates an incremental apk from assets and data. */
+  public ResourceApk packIncrementalBinaryWithDataAndResources(
+      RuleContext ruleContext,
+      Artifact resourceApk,
+      ResourceDependencies resourceDeps,
+      ResourceFilter resourceFilter,
+      List<String> uncompressedExtensions,
+      boolean crunchPng,
+      Artifact proguardCfg)
+      throws InterruptedException {
+    LocalResourceContainer data =
+        new LocalResourceContainer.Builder(ruleContext)
+            .withAssets(
+                AndroidCommon.getAssetDir(ruleContext),
+                ruleContext.getPrerequisitesIf(
+                    // TODO(bazel-team): Remove the ResourceType construct.
+                    ResourceType.ASSETS.getAttribute(), Mode.TARGET, FileProvider.class))
+            .withResources(
+                ruleContext.getPrerequisites("resource_files", Mode.TARGET, FileProvider.class))
+            .build();
+    if (ruleContext.hasErrors()) {
+      return null;
+    }
+    return createApk(
+        ruleContext,
+        false /* isLibrary */,
+        resourceDeps,
+        resourceFilter,
+        uncompressedExtensions,
+        crunchPng,
+        true,
+        ResourceContainer.builderFromRule(ruleContext)
+            .setAssetsAndResourcesFrom(data)
+            .setManifest(getManifest())
+            .setApk(resourceApk)
+            .build(),
+        data,
+        proguardCfg,
+        null, /* mainDexProguardCfg */
+        null, /* manifestOut */
+        null, /* mergedResources */
+        null, /* dataBindingInfoZip */
+        null, /* featureOf */
+        null /* featureAfter */);
+  }
+
   /** Packages up the manifest with resource and assets from the rule and dependent resources. */
   // TODO(bazel-team): this method calls for some refactoring, 15+ params including some nullables.
-  public ResourceApk packWithDataAndResources(
-      @Nullable Artifact resourceApk,
+  public ResourceApk packBinaryWithDataAndResources(
       RuleContext ruleContext,
-      boolean isLibrary,
+      Artifact resourceApk,
       ResourceDependencies resourceDeps,
       Artifact rTxt,
-      Artifact symbols,
       ResourceFilter resourceFilter,
       List<String> uncompressedExtensions,
       boolean crunchPng,
-      boolean incremental,
       Artifact proguardCfg,
       @Nullable Artifact mainDexProguardCfg,
       Artifact manifestOut,
       Artifact mergedResources,
       Artifact dataBindingInfoZip,
       @Nullable Artifact featureOf,
-      @Nullable Artifact featureAfter) throws InterruptedException {
+      @Nullable Artifact featureAfter)
+      throws InterruptedException {
     LocalResourceContainer data = new LocalResourceContainer.Builder(ruleContext)
         .withAssets(
             AndroidCommon.getAssetDir(ruleContext),
@@ -415,17 +460,16 @@ public final class ApplicationManifest {
     }
     return createApk(
         ruleContext,
-        isLibrary,
+        false /* isLibrary */,
         resourceDeps,
         resourceFilter,
         uncompressedExtensions,
         crunchPng,
-        incremental,
+        false /* incremental */,
         ResourceContainer.builderFromRule(ruleContext)
             .setAssetsAndResourcesFrom(data)
             .setManifest(getManifest())
             .setRTxt(rTxt)
-            .setSymbols(symbols)
             .setApk(resourceApk)
             .build(),
         data,
@@ -438,6 +482,55 @@ public final class ApplicationManifest {
         featureAfter);
   }
 
+  public ResourceApk packLibraryWithDataAndResources(
+      RuleContext ruleContext,
+      @Nullable Artifact resourceApk,
+      ResourceDependencies resourceDeps,
+      Artifact rTxt,
+      Artifact symbols,
+      ResourceFilter resourceFilter,
+      Artifact manifestOut,
+      Artifact mergedResources,
+      Artifact dataBindingInfoZip)
+      throws InterruptedException {
+    LocalResourceContainer data =
+        new LocalResourceContainer.Builder(ruleContext)
+            .withAssets(
+                AndroidCommon.getAssetDir(ruleContext),
+                ruleContext.getPrerequisitesIf(
+                    // TODO(bazel-team): Remove the ResourceType construct.
+                    ResourceType.ASSETS.getAttribute(), Mode.TARGET, FileProvider.class))
+            .withResources(
+                ruleContext.getPrerequisites("resource_files", Mode.TARGET, FileProvider.class))
+            .build();
+    if (ruleContext.hasErrors()) {
+      return null;
+    }
+    return createApk(
+        ruleContext,
+        true /* isLibrary */,
+        resourceDeps,
+        resourceFilter,
+        ImmutableList.<String>of(),
+        false /* crunchPng */,
+        false /* incremental */,
+        ResourceContainer.builderFromRule(ruleContext)
+            .setAssetsAndResourcesFrom(data)
+            .setManifest(getManifest())
+            .setRTxt(rTxt)
+            .setSymbols(symbols)
+            .setApk(null)
+            .build(),
+        data,
+        null /* proguardCfg */,
+        null /* mainDexProguardCfg */,
+        manifestOut,
+        mergedResources,
+        dataBindingInfoZip,
+        null /* featureOf */,
+        null /* featureAfter */);
+  }
+
   private ResourceApk createApk(
       RuleContext ruleContext,
       boolean isLibrary,
@@ -586,9 +679,10 @@ public final class ApplicationManifest {
 
   /**
    * Packages up the manifest with resources, and generates the R.java.
-   * @throws InterruptedException
    *
-   * @deprecated in favor of {@link ApplicationManifest#packWithDataAndResources}.
+   * @throws InterruptedException
+   * @deprecated in favor of {@link ApplicationManifest#packBinaryWithDataAndResources} and {@link
+   *     ApplicationManifest#packLibraryWithDataAndResources}.
    */
   @Deprecated
   public ResourceApk packWithResources(
@@ -597,7 +691,8 @@ public final class ApplicationManifest {
       ResourceDependencies resourceDeps,
       boolean createSource,
       Artifact proguardCfg,
-      @Nullable Artifact mainDexProguardCfg) throws InterruptedException {
+      @Nullable Artifact mainDexProguardCfg)
+      throws InterruptedException {
 
     TransitiveInfoCollection resourcesPrerequisite =
         ruleContext.getPrerequisite("resources", Mode.TARGET);