Properly rename manifests

Make AndroidSkylarkData abstract and add a getAndroidSemantics() method

The Skylark stamp_manifest rule should properly use AndroidSemantics to rename
the manifest as well, in keeping with the android_library rule.

AndroidLocalTest should do the same manifest renaming regardless of which
AndroidSemantics it is run under. Have it use a seperate method to do so.

RELNOTES: none
PiperOrigin-RevId: 195992300

6 files changed, 91 insertions, 45 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTest.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTest.java
index 685562d738..2f66c3f8d5 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTest.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidLocalTest.java
@@ -24,7 +24,6 @@ import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTa
 import com.google.devtools.build.lib.bazel.rules.java.BazelJavaSemantics;
 import com.google.devtools.build.lib.rules.android.AndroidLocalTestBase;
 import com.google.devtools.build.lib.rules.android.AndroidMigrationSemantics;
-import com.google.devtools.build.lib.rules.android.AndroidSemantics;
 import com.google.devtools.build.lib.rules.java.JavaCommon;
 import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts;
 import com.google.devtools.build.lib.rules.java.JavaCompilationHelper;
@@ -38,11 +37,6 @@ public class BazelAndroidLocalTest extends AndroidLocalTestBase {
   Artifact androidAllJarsPropFile;
 
   @Override
-  protected AndroidSemantics createAndroidSemantics() {
-    return BazelAndroidSemantics.INSTANCE;
-  }
-
-  @Override
   protected AndroidMigrationSemantics createAndroidMigrationSemantics() {
     return BazelAndroidMigrationSemantics.INSTANCE;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
index 05a9cc32e1..d9f1feeaeb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java
@@ -77,7 +77,6 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor
     ruleContext.checkSrcsSamePackage(true);
 
     JavaSemantics javaSemantics = createJavaSemantics();
-    AndroidSemantics androidSemantics = createAndroidSemantics();
     createAndroidMigrationSemantics().validateRuleContext(ruleContext);
     AndroidLocalTestConfiguration androidLocalTestConfiguration =
         ruleContext.getFragment(AndroidLocalTestConfiguration.class);
@@ -92,7 +91,7 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor
 
     if (AndroidResources.decoupleDataProcessing(ruleContext)) {
       StampedAndroidManifest manifest =
-          StampedAndroidManifest.from(ruleContext, androidSemantics).mergeWithDeps(ruleContext);
+          AndroidManifest.from(ruleContext).mergeWithDeps(ruleContext);
 
       resourceApk =
           ProcessedAndroidData.processLocalTestDataFrom(ruleContext, manifest)
@@ -103,7 +102,7 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor
           ResourceDependencies.fromRuleDeps(ruleContext, /* neverlink= */ false);
 
       ApplicationManifest applicationManifest =
-          getApplicationManifest(ruleContext, androidSemantics, resourceDependencies);
+          getApplicationManifest(ruleContext, resourceDependencies);
 
       // Create the final merged R class
       resourceApk =
@@ -403,15 +402,13 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor
    * @throws RuleErrorException
    */
   private ApplicationManifest getApplicationManifest(
-      RuleContext ruleContext,
-      AndroidSemantics androidSemantics,
-      ResourceDependencies resourceDependencies)
+      RuleContext ruleContext, ResourceDependencies resourceDependencies)
       throws InterruptedException, RuleErrorException {
     ApplicationManifest applicationManifest;
 
     if (AndroidResources.definesAndroidResources(ruleContext.attributes())) {
       AndroidResources.validateRuleContext(ruleContext);
-      ApplicationManifest ruleManifest = androidSemantics.getManifestForRule(ruleContext);
+      ApplicationManifest ruleManifest = ApplicationManifest.renamedFromRule(ruleContext);
       applicationManifest = ruleManifest.mergeWith(ruleContext, resourceDependencies);
     } else {
       // we don't have a manifest, merge like android_library with a stub manifest
@@ -510,8 +507,7 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor
     // The dep may be a simple JAR and not a java rule, hence we can't simply do
     // dep.getProvider(JavaCompilationArgsProvider.class).getRecursiveJavaCompilationArgs(),
     // so we reuse the logic within JavaCompilationArgs to handle both scenarios.
-    return JavaCompilationArgsProvider.legacyFromTargets(ImmutableList.of(deps))
-        .getRuntimeJars();
+    return JavaCompilationArgsProvider.legacyFromTargets(ImmutableList.of(deps)).getRuntimeJars();
   }
 
   private static String getAndCheckTestClass(
@@ -551,9 +547,6 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor
   /** Get JavaSemantics */
   protected abstract JavaSemantics createJavaSemantics();
 
-  /** Get AndroidSemantics */
-  protected abstract AndroidSemantics createAndroidSemantics();
-
   /** Get AndroidMigrationSemantics */
   protected abstract AndroidMigrationSemantics createAndroidMigrationSemantics();
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java
index 02b741dd1d..dfe7a014d0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidManifest.java
@@ -45,6 +45,26 @@ public class AndroidManifest {
    */
   public static AndroidManifest from(RuleContext ruleContext, AndroidSemantics androidSemantics)
       throws RuleErrorException, InterruptedException {
+    return innerFrom(ruleContext, androidSemantics);
+  }
+
+  /**
+   * Gets the manifest for this rule.
+   *
+   * <p>If no manifest is specified in the rule's attributes, an empty manifest will be generated.
+   *
+   * <p>Unlike {@link #from(RuleContext, AndroidSemantics)}, the AndroidSemantics-specific manifest
+   * processing methods will not be applied in this method. The manifest returned by this method
+   * will be the same regardless of the AndroidSemantics being used.
+   */
+  public static AndroidManifest from(RuleContext ruleContext)
+      throws InterruptedException, RuleErrorException {
+    return innerFrom(ruleContext, null);
+  }
+
+  private static AndroidManifest innerFrom(
+      RuleContext ruleContext, @Nullable AndroidSemantics androidSemantics)
+      throws RuleErrorException, InterruptedException {
     if (!AndroidResources.definesAndroidResources(ruleContext.attributes())) {
       // Generate a dummy manifest
       return StampedAndroidManifest.createEmpty(ruleContext, /* exported = */ false);
@@ -52,8 +72,17 @@ public class AndroidManifest {
 
     AndroidResources.validateRuleContext(ruleContext);
 
+    Artifact rawManifest = ApplicationManifest.getManifestFromAttributes(ruleContext);
+
+    Artifact renamedManifest;
+    if (androidSemantics != null) {
+      renamedManifest = androidSemantics.renameManifest(ruleContext, rawManifest);
+    } else {
+      renamedManifest = ApplicationManifest.renameManifestIfNeeded(ruleContext, rawManifest);
+    }
+
     return new AndroidManifest(
-        androidSemantics.getManifestForRule(ruleContext).getManifest(),
+        renamedManifest,
         getAndroidPackage(ruleContext),
         AndroidCommon.getExportsManifest(ruleContext));
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSemantics.java
index 3726808cb5..1620f3bc30 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSemantics.java
@@ -18,7 +18,6 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
-import com.google.devtools.build.lib.analysis.actions.SymlinkAction;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts;
@@ -40,26 +39,14 @@ public interface AndroidSemantics {
    */
   default ApplicationManifest getManifestForRule(RuleContext ruleContext)
       throws InterruptedException, RuleErrorException {
-    ApplicationManifest result = ApplicationManifest.fromRule(ruleContext);
-    Artifact manifest = result.getManifest();
-    if (manifest.getFilename().equals("AndroidManifest.xml")) {
-      return result;
-    } else {
-      /*
-       * If the manifest file is not named AndroidManifest.xml, we create a symlink named
-       * AndroidManifest.xml to it. aapt requires the manifest to be named as such.
-       */
-      Artifact manifestSymlink =
-          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_SYMLINKED_MANIFEST);
-      SymlinkAction symlinkAction =
-          new SymlinkAction(
-              ruleContext.getActionOwner(),
-              manifest,
-              manifestSymlink,
-              "Renaming Android manifest for " + ruleContext.getLabel());
-      ruleContext.registerAction(symlinkAction);
-      return ApplicationManifest.fromExplicitManifest(ruleContext, manifestSymlink);
-    }
+    Artifact rawManifest = ApplicationManifest.getManifestFromAttributes(ruleContext);
+    return ApplicationManifest.fromExplicitManifest(
+        ruleContext, renameManifest(ruleContext, rawManifest));
+  }
+
+  default Artifact renameManifest(RuleContext ruleContext, Artifact rawManifest)
+      throws InterruptedException {
+    return ApplicationManifest.renameManifestIfNeeded(ruleContext, rawManifest);
   }
 
   /** Returns the name of the file in which the file names of native dependencies are listed. */
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java
index 5fa56a3be0..cb13f6a559 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java
@@ -47,7 +47,9 @@ import javax.annotation.Nullable;
     doc =
         "Utilities for working with Android data (manifests, resources, and assets). "
             + "This API is non-final and subject to change without warning; do not rely on it.")
-public class AndroidSkylarkData {
+public abstract class AndroidSkylarkData {
+
+  public abstract AndroidSemantics getAndroidSemantics();
 
   /**
    * Skylark API for getting a asset provider for android_library targets that don't specify assets.
@@ -201,7 +203,8 @@ public class AndroidSkylarkData {
       },
       doc = "Stamps a manifest with package information.")
   public AndroidManifestInfo stampAndroidManifest(
-      SkylarkRuleContext ctx, Object manifest, Object customPackage, boolean exported) {
+      SkylarkRuleContext ctx, Object manifest, Object customPackage, boolean exported)
+      throws InterruptedException {
     String pkg = fromNoneable(customPackage, String.class);
     if (pkg == null) {
       pkg = AndroidManifest.getDefaultPackage(ctx.getRuleContext());
@@ -212,7 +215,11 @@ public class AndroidSkylarkData {
       return StampedAndroidManifest.createEmpty(ctx.getRuleContext(), pkg, exported).toProvider();
     }
 
-    return new AndroidManifest(primaryManifest, pkg, exported)
+    // If needed, rename the manifest to "AndroidManifest.xml", which aapt expects.
+    Artifact renamedManifest =
+        getAndroidSemantics().renameManifest(ctx.getRuleContext(), primaryManifest);
+
+    return new AndroidManifest(renamedManifest, pkg, exported)
         .stamp(ctx.getRuleContext())
         .toProvider();
   }
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 8ecb28d045..783f0f284c 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
@@ -25,6 +25,7 @@ import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
+import com.google.devtools.build.lib.analysis.actions.SymlinkAction;
 import com.google.devtools.build.lib.analysis.config.CompilationMode;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -126,9 +127,44 @@ public final class ApplicationManifest {
     return stubManifest;
   }
 
-  public static ApplicationManifest fromRule(RuleContext ruleContext) throws RuleErrorException {
+  public static Artifact getManifestFromAttributes(RuleContext ruleContext) {
+    return ruleContext.getPrerequisiteArtifact("manifest", Mode.TARGET);
+  }
+
+  /**
+   * Gets the manifest specified in the "manifest" attribute, renaming it if needed.
+   *
+   * <p>Unlike {@link AndroidSemantics#getManifestForRule(RuleContext)}, this method will not
+   * perform AndroidSemantics-specific manifest processing. This method will do the same work
+   * regardless of the AndroidSemantics implementation being used; that method may do different work
+   * depending on the implementation.
+   */
+  public static ApplicationManifest renamedFromRule(RuleContext ruleContext)
+      throws InterruptedException, RuleErrorException {
     return fromExplicitManifest(
-        ruleContext, ruleContext.getPrerequisiteArtifact("manifest", Mode.TARGET));
+        ruleContext, renameManifestIfNeeded(ruleContext, getManifestFromAttributes(ruleContext)));
+  }
+
+  static Artifact renameManifestIfNeeded(RuleContext ruleContext, Artifact manifest)
+      throws InterruptedException {
+    if (manifest.getFilename().equals("AndroidManifest.xml")) {
+      return manifest;
+    } else {
+      /*
+       * If the manifest file is not named AndroidManifest.xml, we create a symlink named
+       * AndroidManifest.xml to it. aapt requires the manifest to be named as such.
+       */
+      Artifact manifestSymlink =
+          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_SYMLINKED_MANIFEST);
+      SymlinkAction symlinkAction =
+          new SymlinkAction(
+              ruleContext.getActionOwner(),
+              manifest,
+              manifestSymlink,
+              "Renaming Android manifest for " + ruleContext.getLabel());
+      ruleContext.registerAction(symlinkAction);
+      return manifestSymlink;
+    }
   }
 
   public static ApplicationManifest fromExplicitManifest(RuleContext ruleContext, Artifact manifest)