Split manifest processing out of AndroidResourceProcessor#processResources() as preparation for the ResourceShrinkerAction which does not require it as part of its aapt invocation.

--
MOS_MIGRATED_REVID=114755551

4 files changed, 79 insertions, 61 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/AarGeneratorAction.java b/src/tools/android/java/com/google/devtools/build/android/AarGeneratorAction.java
index 31c837805d..e3a1d9535f 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AarGeneratorAction.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AarGeneratorAction.java
@@ -206,17 +206,17 @@ public class AarGeneratorAction {
       zipOut.write(Files.readAllBytes(classes));
       zipOut.closeEntry();
 
-      Files.walkFileTree(data.getResourceDirFile().toPath(),
-          new ZipDirectoryWriter(zipOut, data.getResourceDirFile().toPath(), "res"));
+      Files.walkFileTree(data.getResourceDir(),
+          new ZipDirectoryWriter(zipOut, data.getResourceDir(), "res"));
 
       ZipEntry r = new ZipEntry("R.txt");
       zipOut.putNextEntry(r);
       zipOut.write(Files.readAllBytes(rtxt));
       zipOut.closeEntry();
 
-      if (data.getAssetDirFile().exists() && data.getAssetDirFile().list().length > 0) {
-        Files.walkFileTree(data.getAssetDirFile().toPath(),
-            new ZipDirectoryWriter(zipOut, data.getAssetDirFile().toPath(), "assets"));
+      if (Files.exists(data.getAssetDir()) && data.getAssetDir().toFile().list().length > 0) {
+        Files.walkFileTree(data.getAssetDir(),
+            new ZipDirectoryWriter(zipOut, data.getAssetDir(), "assets"));
       }
     }
     aar.toFile().setLastModified(EPOCH);
diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessingAction.java b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessingAction.java
index c02ab41ae0..8a853146e7 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessingAction.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessingAction.java
@@ -88,7 +88,7 @@ public class AndroidResourceProcessingAction {
         category = "config",
         help = "Version of the build tools (e.g. aapt) being used, e.g. 23.0.2")
     public FullRevision buildToolsVersion;
-    
+
     @Option(name = "aapt",
         defaultValue = "null",
         converter = ExistingPathConverter.class,
@@ -282,8 +282,7 @@ public class AndroidResourceProcessingAction {
     options = optionsParser.getOptions(Options.class);
     FileSystem fileSystem = FileSystems.getDefault();
     Path working = fileSystem.getPath("").toAbsolutePath();
-    final AndroidResourceProcessor resourceProcessor =
-        new AndroidResourceProcessor(STD_LOGGER);
+    final AndroidResourceProcessor resourceProcessor = new AndroidResourceProcessor(STD_LOGGER);
 
     try {
       final Path tmp = Files.createTempDirectory("android_resources_tmp");
@@ -296,6 +295,7 @@ public class AndroidResourceProcessingAction {
       final Path mergedResources = tmp.resolve("merged_resources");
       final Path filteredResources = tmp.resolve("resources-filtered");
       final Path densityManifest = tmp.resolve("manifest-filtered/AndroidManifest.xml");
+      final Path processedManifest = tmp.resolve("manifest-processed/AndroidManifest.xml");
 
       Path generatedSources = null;
       if (options.srcJarOutput != null || options.rOutput != null
@@ -329,32 +329,42 @@ public class AndroidResourceProcessingAction {
           true);
 
       LOGGER.fine(String.format("Merging finished at %sms", timer.elapsed(TimeUnit.MILLISECONDS)));
+
       final DensityFilteredAndroidData filteredData = mergedData.filter(
           new DensitySpecificResourceFilter(options.densities, filteredResources, mergedResources),
           new DensitySpecificManifestProcessor(options.densities, densityManifest));
-      LOGGER.fine(
-          String.format("Density filtering finished at %sms",
+
+      LOGGER.fine(String.format("Density filtering finished at %sms",
               timer.elapsed(TimeUnit.MILLISECONDS)));
+
+      final MergedAndroidData processedManifestData = resourceProcessor.processManifest(
+          options.packageType,
+          options.packageForR,
+          options.applicationId,
+          options.versionCode,
+          options.versionName,
+          filteredData,
+          processedManifest);
+
       resourceProcessor.processResources(
           options.aapt,
           options.androidJar,
+          options.buildToolsVersion,
           options.packageType,
           options.debug,
           options.packageForR,
           new FlagAaptOptions(),
           options.resourceConfigs,
-          options.applicationId,
-          options.versionCode,
-          options.versionName,
-          filteredData,
+          processedManifestData,
           data,
-          tmp.resolve("processed_manifest"),
           generatedSources,
           options.packagePath,
-          options.proguardOutput,
-          options.manifestOutput,
-          options.buildToolsVersion);
+          options.proguardOutput);
       LOGGER.fine(String.format("appt finished at %sms", timer.elapsed(TimeUnit.MILLISECONDS)));
+
+      if (options.manifestOutput != null) {
+        resourceProcessor.copyManifestToOutput(processedManifestData, options.manifestOutput);
+      }
       if (options.srcJarOutput != null) {
         resourceProcessor.createSrcJar(generatedSources, options.srcJarOutput,
             VariantConfiguration.Type.LIBRARY == options.packageType);
diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessor.java b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessor.java
index e676cef67b..7a8c91f32d 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessor.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceProcessor.java
@@ -127,30 +127,43 @@ public class AndroidResourceProcessor {
       Throwables.propagate(e);
     }
   }
-  
-  // TODO(bazel-team): Clean up this method call -- 19 params is too many.
+
+  /**
+   * Copies the AndroidManifest.xml to the specified output location.
+   *
+   * @param androidData The MergedAndroidData which contains the manifest to be written to
+   *     manifestOut.
+   * @param manifestOut The Path to write the AndroidManifest.xml.
+   */
+  public void copyManifestToOutput(MergedAndroidData androidData, Path manifestOut) {
+    try {
+      Files.createDirectories(manifestOut.getParent());
+      Files.copy(androidData.getManifest(), manifestOut);
+      // Set to the epoch for caching purposes.
+      Files.setLastModifiedTime(manifestOut, FileTime.fromMillis(0L));
+    } catch (IOException e) {
+      Throwables.propagate(e);
+    }
+  }
+
+  // TODO(bazel-team): Clean up this method call -- 13 params is too many.
   /**
    * Processes resources for generated sources, configs and packaging resources.
    */
   public void processResources(
       Path aapt,
       Path androidJar,
+      @Nullable FullRevision buildToolsVersion,
       VariantConfiguration.Type variantType,
       boolean debug,
       String customPackageForR,
       AaptOptions aaptOptions,
       Collection<String> resourceConfigs,
-      String applicationId,
-      int versionCode,
-      String versionName,
       MergedAndroidData primaryData,
       List<DependencyAndroidData> dependencyData,
-      Path workingDirectory,
       Path sourceOut,
       Path packageOut,
-      Path proguardOut,
-      Path manifestOut,
-      @Nullable FullRevision buildToolsVersion)
+      Path proguardOut)
       throws IOException, InterruptedException, LoggedErrorException {
     List<SymbolFileProvider> libraries = new ArrayList<>();
     List<String> packages = new ArrayList<>();
@@ -160,17 +173,9 @@ public class AndroidResourceProcessor {
       packages.add(VariantConfiguration.getManifestPackage(library.getManifest()));
     }
 
-    Path androidManifest = processManifest(
-        variantType == VariantConfiguration.Type.DEFAULT ? applicationId : customPackageForR,
-        versionCode,
-        versionName,
-        primaryData,
-        workingDirectory,
-        variantType == VariantConfiguration.Type.DEFAULT
-            ? ManifestMerger2.MergeType.APPLICATION : ManifestMerger2.MergeType.LIBRARY);
-
-    Path resFolder = primaryData.getResourceDirFile().toPath();
-    Path assetsDir = primaryData.getAssetDirFile().toPath();
+    Path androidManifest = primaryData.getManifest();
+    Path resourceDir = primaryData.getResourceDir();
+    Path assetsDir = primaryData.getAssetDir();
 
     AaptCommandBuilder commandBuilder =
         new AaptCommandBuilder(aapt, buildToolsVersion, variantType, "package")
@@ -187,7 +192,7 @@ public class AndroidResourceProcessor {
         // Add the manifest for validation.
         .add("-M", androidManifest.toAbsolutePath())
         // Maybe add the resources if they exist
-        .maybeAdd("-S", resFolder, Files.isDirectory(resFolder))
+        .maybeAdd("-S", resourceDir, Files.isDirectory(resourceDir))
         // Maybe add the assets if they exist
         .maybeAdd("-A", assetsDir, Files.isDirectory(assetsDir))
         // Outputs
@@ -226,10 +231,6 @@ public class AndroidResourceProcessor {
     if (packageOut != null) {
       Files.setLastModifiedTime(packageOut, FileTime.fromMillis(0L));
     }
-    if (manifestOut != null) {
-      Files.copy(androidManifest, manifestOut);
-      Files.setLastModifiedTime(manifestOut, FileTime.fromMillis(0L));
-    }
   }
 
   private void writeDependencyPackageRs(VariantConfiguration.Type variantType,
@@ -287,21 +288,28 @@ public class AndroidResourceProcessor {
     }
   }
 
-  private Path processManifest(
-      String newManifestPackage,
+  public MergedAndroidData processManifest(
+      VariantConfiguration.Type variantType,
+      String customPackageForR,
+      String applicationId,
       int versionCode,
       String versionName,
       MergedAndroidData primaryData,
-      Path workingDirectory,
-      ManifestMerger2.MergeType mergeType) throws IOException {
+      Path processedManifest) throws IOException {
+
+    ManifestMerger2.MergeType mergeType = variantType == VariantConfiguration.Type.DEFAULT
+        ? ManifestMerger2.MergeType.APPLICATION : ManifestMerger2.MergeType.LIBRARY;
+
+    String newManifestPackage = variantType == VariantConfiguration.Type.DEFAULT
+        ? applicationId : customPackageForR;
+
     if (versionCode != -1 || versionName != null || newManifestPackage != null) {
-      Path androidManifest =
-          Files.createDirectories(workingDirectory).resolve("AndroidManifest.xml");
+      Files.createDirectories(processedManifest.getParent());
 
       // The generics on Invoker don't make sense, so ignore them.
       @SuppressWarnings("unchecked")
       Invoker<?> manifestMergerInvoker =
-          ManifestMerger2.newMerger(primaryData.getManifestFile(), stdLogger, mergeType);
+          ManifestMerger2.newMerger(primaryData.getManifest().toFile(), stdLogger, mergeType);
       // Stamp new package
       if (newManifestPackage != null) {
         manifestMergerInvoker.setOverride(SystemProperty.PACKAGE, newManifestPackage);
@@ -323,10 +331,10 @@ public class AndroidResourceProcessor {
         switch (mergingReport.getResult()) {
           case WARNING:
             mergingReport.log(stdLogger);
-            writeMergedManifest(mergingReport, androidManifest);
+            writeMergedManifest(mergingReport, processedManifest);
             break;
           case SUCCESS:
-            writeMergedManifest(mergingReport, androidManifest);
+            writeMergedManifest(mergingReport, processedManifest);
             break;
           case ERROR:
             mergingReport.log(stdLogger);
@@ -338,9 +346,10 @@ public class AndroidResourceProcessor {
           IOException | SAXException | ParserConfigurationException | MergeFailureException e) {
         Throwables.propagate(e);
       }
-      return androidManifest;
+      return new MergedAndroidData(primaryData.getResourceDir(), primaryData.getAssetDir(),
+          processedManifest);
     }
-    return primaryData.getManifestFile().toPath();
+    return primaryData;
   }
 
   private void writeMergedManifest(MergingReport mergingReport,
diff --git a/src/tools/android/java/com/google/devtools/build/android/MergedAndroidData.java b/src/tools/android/java/com/google/devtools/build/android/MergedAndroidData.java
index 70322edb12..a69b02d71b 100644
--- a/src/tools/android/java/com/google/devtools/build/android/MergedAndroidData.java
+++ b/src/tools/android/java/com/google/devtools/build/android/MergedAndroidData.java
@@ -13,7 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.android;
 
-import java.io.File;
 import java.nio.file.Path;
 
 /**
@@ -38,16 +37,16 @@ class MergedAndroidData {
     this.manifest = manifest;
   }
 
-  public File getResourceDirFile() {
-    return resourceDir.toFile();
+  public Path getResourceDir() {
+    return resourceDir;
   }
 
-  public File getAssetDirFile() {
-    return assetDir != null ? assetDir.toFile() : null;
+  public Path getAssetDir() {
+    return assetDir;
   }
 
-  public File getManifestFile() {
-    return manifest.toFile();
+  public Path getManifest() {
+    return manifest;
   }
 
   public DensityFilteredAndroidData filter(