Cleanup the interface of ApkActionsBuilder.

A few changes:

1. Remove the ApkSigningMethod input. We always read this from
AndroidConfiguration, callers shouldn't need to pass it in.
2. Add a signingKey attribute. This will be needed for dex2oat to pass in
whatever key was used to sign the input APKs.
3. Unify the resourceApk and nativeLibsZips into a "addInputZip(s)" methods.
This can also be used for inputting an existing APK, such as the one from
dex2oat.

RELNOTES: None
PiperOrigin-RevId: 157973679

2 files changed, 72 insertions, 86 deletions

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 d893f140cb..d9b260a95f 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
@@ -53,7 +53,6 @@ import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.TriState;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidBinaryType;
-import com.google.devtools.build.lib.rules.android.AndroidConfiguration.ApkSigningMethod;
 import com.google.devtools.build.lib.rules.android.AndroidRuleClasses.MultidexMode;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
 import com.google.devtools.build.lib.rules.cpp.CppHelper;
@@ -555,23 +554,23 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
       proguardOutput.addAllToSet(filesBuilder, finalProguardMap);
     }
 
-    ApkSigningMethod signingMethod =
-        ruleContext.getFragment(AndroidConfiguration.class).getApkSigningMethod();
     Artifact unsignedApk =
         ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_BINARY_UNSIGNED_APK);
     Artifact zipAlignedApk =
         ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_BINARY_APK);
+    Artifact signingKey = androidSemantics.getApkDebugSigningKey(ruleContext);
 
-    ApkActionsBuilder.create("apk", signingMethod)
+    ApkActionsBuilder.create("apk")
         .setClassesDex(finalDexes)
-        .setResourceApk(resourceApk.getArtifact())
+        .addInputZip(resourceApk.getArtifact())
         .setJavaResourceZip(dexingOutput.javaResourceJar)
-        .setNativeLibsZips(nativeLibsZips)
+        .addInputZips(nativeLibsZips)
         .setNativeLibs(nativeLibs)
         .setUnsignedApk(unsignedApk)
         .setSignedApk(zipAlignedApk)
+        .setSigningKey(signingKey)
         .setZipalignApk(true)
-        .registerActions(ruleContext, androidSemantics);
+        .registerActions(ruleContext);
 
     filesBuilder.add(binaryJar);
     filesBuilder.add(unsignedApk);
@@ -622,20 +621,21 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
     Artifact stubDex = getStubDex(ruleContext, javaSemantics, false);
     ruleContext.assertNoErrors();
 
-    ApkActionsBuilder incrementalActionsBuilder = ApkActionsBuilder
-        .create("incremental apk", signingMethod)
-        .setClassesDex(stubDex)
-        .setResourceApk(incrementalResourceApk.getArtifact())
-        .setJavaResourceZip(dexingOutput.javaResourceJar)
-        .setNativeLibsZips(nativeLibsZips)
-        .setJavaResourceFile(stubData)
-        .setSignedApk(incrementalApk);
+    ApkActionsBuilder incrementalActionsBuilder =
+        ApkActionsBuilder.create("incremental apk")
+            .setClassesDex(stubDex)
+            .addInputZip(incrementalResourceApk.getArtifact())
+            .setJavaResourceZip(dexingOutput.javaResourceJar)
+            .addInputZips(nativeLibsZips)
+            .setJavaResourceFile(stubData)
+            .setSignedApk(incrementalApk)
+            .setSigningKey(signingKey);
 
     if (!ruleContext.getFragment(AndroidConfiguration.class).useIncrementalNativeLibs()) {
       incrementalActionsBuilder.setNativeLibs(nativeLibs);
     }
 
-    incrementalActionsBuilder.registerActions(ruleContext, androidSemantics);
+    incrementalActionsBuilder.registerActions(ruleContext);
 
     Artifact argsArtifact =
         ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.MOBILE_INSTALL_ARGS);
@@ -683,10 +683,11 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
     // installation of each split (said references would eventually get installed, but it cannot
     // know that in advance)
     Artifact resourceSplitApk = getDxArtifact(ruleContext, "android_resources.apk");
-    ApkActionsBuilder.create("split Android resource apk", signingMethod)
-        .setResourceApk(splitResourceApk.getArtifact())
+    ApkActionsBuilder.create("split Android resource apk")
+        .addInputZip(splitResourceApk.getArtifact())
         .setSignedApk(resourceSplitApk)
-        .registerActions(ruleContext, androidSemantics);
+        .setSigningKey(signingKey)
+        .registerActions(ruleContext);
     splitApkSetBuilder.add(resourceSplitApk);
 
     for (int i = 0; i < dexingOutput.shardDexZips.size(); i++) {
@@ -694,32 +695,35 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
       Artifact splitApkResources = createSplitApkResources(
           ruleContext, applicationManifest, splitName, true);
       Artifact splitApk = getDxArtifact(ruleContext, splitName + ".apk");
-      ApkActionsBuilder.create("split dex apk " + (i + 1), signingMethod)
+      ApkActionsBuilder.create("split dex apk " + (i + 1))
           .setClassesDex(dexingOutput.shardDexZips.get(i))
-          .setResourceApk(splitApkResources)
+          .addInputZip(splitApkResources)
           .setSignedApk(splitApk)
-          .registerActions(ruleContext, androidSemantics);
+          .setSigningKey(signingKey)
+          .registerActions(ruleContext);
       splitApkSetBuilder.add(splitApk);
     }
 
     Artifact nativeSplitApkResources = createSplitApkResources(
         ruleContext, applicationManifest, "native", false);
     Artifact nativeSplitApk = getDxArtifact(ruleContext, "native.apk");
-    ApkActionsBuilder.create("split native apk", signingMethod)
-        .setResourceApk(nativeSplitApkResources)
+    ApkActionsBuilder.create("split native apk")
+        .addInputZip(nativeSplitApkResources)
         .setNativeLibs(nativeLibs)
         .setSignedApk(nativeSplitApk)
-        .registerActions(ruleContext, androidSemantics);
+        .setSigningKey(signingKey)
+        .registerActions(ruleContext);
     splitApkSetBuilder.add(nativeSplitApk);
 
     Artifact javaSplitApkResources = createSplitApkResources(
         ruleContext, applicationManifest, "java_resources", false);
     Artifact javaSplitApk = getDxArtifact(ruleContext, "java_resources.apk");
-    ApkActionsBuilder.create("split Java resource apk", signingMethod)
-        .setResourceApk(javaSplitApkResources)
+    ApkActionsBuilder.create("split Java resource apk")
+        .addInputZip(javaSplitApkResources)
         .setJavaResourceZip(dexingOutput.javaResourceJar)
         .setSignedApk(javaSplitApk)
-        .registerActions(ruleContext, androidSemantics);
+        .setSigningKey(signingKey)
+        .registerActions(ruleContext);
     splitApkSetBuilder.add(javaSplitApk);
 
     Artifact splitMainApkResources = getDxArtifact(ruleContext, "split_main.ap_");
@@ -737,12 +741,13 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
     Artifact splitMainApk = getDxArtifact(ruleContext, "split_main.apk");
     Artifact splitStubDex = getStubDex(ruleContext, javaSemantics, true);
     ruleContext.assertNoErrors();
-    ApkActionsBuilder.create("split main apk", signingMethod)
+    ApkActionsBuilder.create("split main apk")
         .setClassesDex(splitStubDex)
-        .setResourceApk(splitMainApkResources)
-        .setNativeLibsZips(nativeLibsZips)
+        .addInputZip(splitMainApkResources)
+        .addInputZips(nativeLibsZips)
         .setSignedApk(splitMainApk)
-        .registerActions(ruleContext, androidSemantics);
+        .setSigningKey(signingKey)
+        .registerActions(ruleContext);
     splitApkSetBuilder.add(splitMainApk);
     NestedSet<Artifact> allSplitApks = splitApkSetBuilder.build();
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java
index 98511fe03f..7734c34292 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java
@@ -19,7 +19,6 @@ import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.RunfilesSupplierImpl;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
-import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.rules.android.AndroidConfiguration.ApkSigningMethod;
 import com.google.devtools.build.lib.rules.java.JavaHelper;
 import com.google.devtools.build.lib.rules.java.JavaToolchainProvider;
@@ -37,25 +36,23 @@ import com.google.devtools.build.lib.vfs.PathFragment;
  */
 public class ApkActionsBuilder {
   private Artifact classesDex;
-  private Artifact resourceApk;
+  private ImmutableList.Builder<Artifact> inputZips = new ImmutableList.Builder<>();
   private Artifact javaResourceZip;
   private Artifact javaResourceFile;
-  private NestedSet<Artifact> nativeLibsZips;
   private NativeLibs nativeLibs = NativeLibs.EMPTY;
   private Artifact unsignedApk;
   private Artifact signedApk;
   private boolean zipalignApk = false;
+  private Artifact signingKey;
 
   private final String apkName;
-  private final ApkSigningMethod signingMethod;
 
-  public static ApkActionsBuilder create(String apkName, ApkSigningMethod signingMethod) {
-    return new ApkActionsBuilder(apkName, signingMethod);
+  public static ApkActionsBuilder create(String apkName) {
+    return new ApkActionsBuilder(apkName);
   }
 
-  private ApkActionsBuilder(String apkName, ApkSigningMethod signingMethod) {
+  private ApkActionsBuilder(String apkName) {
     this.apkName = apkName;
-    this.signingMethod = signingMethod;
   }
 
   /** Sets the native libraries to be included in the APK. */
@@ -77,9 +74,14 @@ public class ApkActionsBuilder {
     return this;
   }
 
-  /** Sets the resource APK that contains the Android resources to be bundled into the output. */
-  public ApkActionsBuilder setResourceApk(Artifact resourceApk) {
-    this.resourceApk = resourceApk;
+  /** Add a zip file that should be copied as is into the APK. */
+  public ApkActionsBuilder addInputZip(Artifact inputZip) {
+    this.inputZips.add(inputZip);
+    return this;
+  }
+
+  public ApkActionsBuilder addInputZips(Iterable<Artifact> inputZips) {
+    this.inputZips.addAll(inputZips);
     return this;
   }
 
@@ -107,11 +109,6 @@ public class ApkActionsBuilder {
     return this;
   }
 
-  public ApkActionsBuilder setNativeLibsZips(NestedSet<Artifact> nativeLibsZips) {
-    this.nativeLibsZips = nativeLibsZips;
-    return this;
-  }
-
   /** Requests an unsigned APK be built at the specified artifact. */
   public ApkActionsBuilder setUnsignedApk(Artifact unsignedApk) {
     this.unsignedApk = unsignedApk;
@@ -130,8 +127,14 @@ public class ApkActionsBuilder {
     return this;
   }
 
+  /** Sets the signing key that will be used to sign the APK. */
+  public ApkActionsBuilder setSigningKey(Artifact signingKey) {
+    this.signingKey = signingKey;
+    return this;
+  }
+
   /** Registers the actions needed to build the requested APKs in the rule context. */
-  public void registerActions(RuleContext ruleContext, AndroidSemantics semantics) {
+  public void registerActions(RuleContext ruleContext) {
     boolean useSingleJarApkBuilder =
         ruleContext.getFragment(AndroidConfiguration.class).useSingleJarApkBuilder();
 
@@ -143,7 +146,7 @@ public class ApkActionsBuilder {
     if (useSingleJarApkBuilder) {
       buildApk(ruleContext, intermediateUnsignedApk, "Generating unsigned " + apkName);
     } else {
-      legacyBuildApk(ruleContext, intermediateUnsignedApk, null, "Generating unsigned " + apkName);
+      legacyBuildApk(ruleContext, intermediateUnsignedApk, "Generating unsigned " + apkName);
     }
 
     if (signedApk != null) {
@@ -155,7 +158,7 @@ public class ApkActionsBuilder {
             AndroidBinary.getDxArtifact(ruleContext, "zipaligned_" + signedApk.getFilename());
         zipalignApk(ruleContext, intermediateUnsignedApk, apkToSign);
       }
-      signApk(ruleContext, semantics.getApkDebugSigningKey(ruleContext), apkToSign, signedApk);
+      signApk(ruleContext, apkToSign, signedApk);
     }
   }
 
@@ -165,8 +168,7 @@ public class ApkActionsBuilder {
    * <p>If {@code signingKey} is not null, the apk will be signed with it using the V1 signature
    * scheme.
    */
-  private void legacyBuildApk(RuleContext ruleContext, Artifact outApk, Artifact signingKey,
-      String message) {
+  private void legacyBuildApk(RuleContext ruleContext, Artifact outApk, String message) {
     SpawnAction.Builder actionBuilder = new SpawnAction.Builder()
         .setExecutable(AndroidSdkProvider.fromRuleContext(ruleContext).getApkBuilder())
         .setProgressMessage(message)
@@ -205,14 +207,6 @@ public class ApkActionsBuilder {
           .addInput(nativeLibs.getName());
     }
 
-    if (nativeLibsZips != null) {
-      for (Artifact nativeLibsZip : nativeLibsZips) {
-        actionBuilder
-            .addArgument("-z")
-            .addInputArgument(nativeLibsZip);
-      }
-    }
-
     if (javaResourceFile != null) {
       actionBuilder
           .addArgument("-rf")
@@ -220,16 +214,11 @@ public class ApkActionsBuilder {
           .addInput(javaResourceFile);
     }
 
-    if (signingKey == null) {
-      actionBuilder.addArgument("-u");
-    } else {
-      actionBuilder.addArgument("-ks").addArgument(signingKey.getExecPathString());
-      actionBuilder.addInput(signingKey);
-    }
+    actionBuilder.addArgument("-u");
 
-    actionBuilder
-        .addArgument("-z")
-        .addInputArgument(resourceApk);
+    for (Artifact inputZip : inputZips.build()) {
+      actionBuilder.addArgument("-z").addInputArgument(inputZip);
+    }
 
     if (classesDex != null) {
       actionBuilder
@@ -339,18 +328,8 @@ public class ApkActionsBuilder {
           .addInput(nativeLibs.getName());
     }
 
-    if (resourceApk != null) {
-      singleJarActionBuilder
-          .addArgument("--sources")
-          .addInputArgument(resourceApk);
-    }
-
-    if (nativeLibsZips != null) {
-      for (Artifact nativeLibsZip : nativeLibsZips) {
-        singleJarActionBuilder
-            .addArgument("--sources")
-            .addInputArgument(nativeLibsZip);
-      }
+    for (Artifact inputZip : inputZips.build()) {
+      singleJarActionBuilder.addArgument("--sources").addInputArgument(inputZip);
     }
 
     ImmutableList<String> noCompressExtensions =
@@ -398,11 +377,13 @@ public class ApkActionsBuilder {
 
   /**
    * Signs an APK using the ApkSignerTool. Supports both the jar signing scheme(v1) and the apk
-   * signing scheme v2. Note that zip alignment is preserved by this step. Furthermore,
-   * zip alignment cannot be performed after v2 signing without invalidating the signature.
+   * signing scheme v2. Note that zip alignment is preserved by this step. Furthermore, zip
+   * alignment cannot be performed after v2 signing without invalidating the signature.
    */
-  private void signApk(RuleContext ruleContext, Artifact signingKey,
-      Artifact unsignedApk, Artifact signedAndZipalignedApk) {
+  private void signApk(
+      RuleContext ruleContext, Artifact unsignedApk, Artifact signedAndZipalignedApk) {
+    ApkSigningMethod signingMethod =
+        ruleContext.getFragment(AndroidConfiguration.class).getApkSigningMethod();
     ruleContext.registerAction(new SpawnAction.Builder()
         .setExecutable(AndroidSdkProvider.fromRuleContext(ruleContext).getApkSigner())
         .setProgressMessage("Signing " + apkName)