AndroidResourceProcessor: sort files before writing zips

walkFileTree can be non-determinisitic, so the zip entries
can be listed in a different order:
"A file tree is walked depth first, but you cannot make any
assumptions about the iteration order that subdirectories
are visited."

--
PiperOrigin-RevId: 149345145
MOS_MIGRATED_REVID=149345145

2 files changed, 58 insertions, 14 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 20b0e93c83..66d921b384 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
@@ -20,6 +20,7 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Stopwatch;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Ordering;
 import com.google.devtools.build.android.Converters.ExistingPathConverter;
 import com.google.devtools.build.android.Converters.PathConverter;
 import com.google.devtools.build.android.Converters.UnvalidatedAndroidDataConverter;
@@ -34,6 +35,8 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
@@ -189,8 +192,9 @@ public class AarGeneratorAction {
       zipOut.write(Files.readAllBytes(classes));
       zipOut.closeEntry();
 
-      Files.walkFileTree(
-          data.getResourceDir(), new ZipDirectoryWriter(zipOut, data.getResourceDir(), "res"));
+      ZipDirectoryWriter resWriter = new ZipDirectoryWriter(zipOut, data.getResourceDir(), "res");
+      Files.walkFileTree(data.getResourceDir(), resWriter);
+      resWriter.writeEntries();
 
       ZipEntry r = new ZipEntry("R.txt");
       r.setTime(EPOCH);
@@ -199,8 +203,10 @@ public class AarGeneratorAction {
       zipOut.closeEntry();
 
       if (Files.exists(data.getAssetDir()) && data.getAssetDir().toFile().list().length > 0) {
-        Files.walkFileTree(
-            data.getAssetDir(), new ZipDirectoryWriter(zipOut, data.getAssetDir(), "assets"));
+        ZipDirectoryWriter assetWriter =
+            new ZipDirectoryWriter(zipOut, data.getAssetDir(), "assets");
+        Files.walkFileTree(data.getAssetDir(), assetWriter);
+        assetWriter.writeEntries();
       }
     }
     aar.toFile().setLastModified(EPOCH);
@@ -210,6 +216,8 @@ public class AarGeneratorAction {
     private final ZipOutputStream zipOut;
     private final Path root;
     private final String dirName;
+    private final Collection<Path> directories = new ArrayList<>();
+    private final Collection<Path> files = new ArrayList<>();
 
     public ZipDirectoryWriter(ZipOutputStream zipOut, Path root, String dirName) {
       this.zipOut = zipOut;
@@ -219,23 +227,40 @@ public class AarGeneratorAction {
 
     @Override
     public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
+      files.add(file);
+      return FileVisitResult.CONTINUE;
+    }
+
+    @Override
+    public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
+        throws IOException {
+      directories.add(dir);
+      return FileVisitResult.CONTINUE;
+    }
+
+    void writeEntries() throws IOException {
+      for (Path dir : Ordering.natural().immutableSortedCopy(directories)) {
+        writeDirectoryEntry(dir);
+      }
+      for (Path file : Ordering.natural().immutableSortedCopy(files)) {
+        writeFileEntry(file);
+      }
+    }
+
+    private void writeFileEntry(Path file) throws IOException {
       ZipEntry entry = new ZipEntry(new File(dirName, root.relativize(file).toString()).toString());
       entry.setTime(EPOCH);
       zipOut.putNextEntry(entry);
       zipOut.write(Files.readAllBytes(file));
       zipOut.closeEntry();
-      return FileVisitResult.CONTINUE;
     }
 
-    @Override
-    public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
-        throws IOException {
+    private void writeDirectoryEntry(Path dir) throws IOException {
       ZipEntry entry =
           new ZipEntry(new File(dirName, root.relativize(dir).toString()).toString() + "/");
       entry.setTime(EPOCH);
       zipOut.putNextEntry(entry);
       zipOut.closeEntry();
-      return FileVisitResult.CONTINUE;
     }
   }
 }
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 ec50361bb5..e466b3d357 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
@@ -56,6 +56,7 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Multimap;
+import com.google.common.collect.Ordering;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.ListeningExecutorService;
@@ -339,6 +340,7 @@ public class AndroidResourceProcessor {
         SymbolFileSrcJarBuildingVisitor visitor =
             new SymbolFileSrcJarBuildingVisitor(zip, generatedSourcesRoot, staticIds);
         Files.walkFileTree(generatedSourcesRoot, visitor);
+        visitor.writeEntries();
       }
       // Set to the epoch for caching purposes.
       Files.setLastModifiedTime(srcJar, FileTime.fromMillis(0L));
@@ -357,6 +359,7 @@ public class AndroidResourceProcessor {
           new BufferedOutputStream(Files.newOutputStream(classJar)))) {
         ClassJarBuildingVisitor visitor = new ClassJarBuildingVisitor(zip, generatedClassesRoot);
         Files.walkFileTree(generatedClassesRoot, visitor);
+        visitor.writeEntries();
         visitor.writeManifestContent();
       }
       // Set to the epoch for caching purposes.
@@ -401,11 +404,13 @@ public class AndroidResourceProcessor {
         ZipBuilderVisitor visitor = new ZipBuilderVisitor(zout, resourcesRoot, "res");
         visitor.setCompress(compress);
         Files.walkFileTree(resourcesRoot, visitor);
+        visitor.writeEntries();
       }
       if (Files.exists(assetsRoot)) {
         ZipBuilderVisitor visitor = new ZipBuilderVisitor(zout, assetsRoot, "assets");
         visitor.setCompress(compress);
         Files.walkFileTree(assetsRoot, visitor);
+        visitor.writeEntries();
       }
     }
   }
@@ -1279,6 +1284,7 @@ public class AndroidResourceProcessor {
     protected final Path root;
     private final String directoryPrefix;
     private int storageMethod = ZipEntry.STORED;
+    private final Collection<Path> paths = new ArrayList<>();
 
     ZipBuilderVisitor(ZipOutputStream zip, Path root, String directory) {
       this.zip = zip;
@@ -1291,6 +1297,17 @@ public class AndroidResourceProcessor {
     }
 
     /**
+     * Iterate through collected file paths in a deterministic order and write to the zip.
+     *
+     * @throws IOException if there is an error reading from the source or writing to the zip.
+     */
+    void writeEntries() throws IOException {
+      for (Path path : Ordering.natural().immutableSortedCopy(paths)) {
+        writeFileEntry(path);
+      }
+    }
+
+    /**
      * Normalize timestamps for deterministic builds. Stamp .class files to be a bit newer
      * than .java files. See:
      * {@link com.google.devtools.build.buildjar.jarhelper.JarHelper#normalizedTimestamp(String)}
@@ -1321,9 +1338,13 @@ public class AndroidResourceProcessor {
 
     @Override
     public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
+      paths.add(file);
+      return FileVisitResult.CONTINUE;
+    }
+
+    protected void writeFileEntry(Path file) throws IOException {
       byte[] content = Files.readAllBytes(file);
       addEntry(file, content);
-      return FileVisitResult.CONTINUE;
     }
   }
 
@@ -1375,7 +1396,7 @@ public class AndroidResourceProcessor {
     }
 
     @Override
-    public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
+    protected void writeFileEntry(Path file) throws IOException {
       if (file.getFileName().endsWith("R.java")) {
         byte[] content = Files.readAllBytes(file);
         if (staticIds) {
@@ -1385,7 +1406,6 @@ public class AndroidResourceProcessor {
         }
         addEntry(file, content);
       }
-      return FileVisitResult.CONTINUE;
     }
   }
 
@@ -1399,14 +1419,13 @@ public class AndroidResourceProcessor {
     }
 
     @Override
-    public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
+    protected void writeFileEntry(Path file) throws IOException {
       Path filename = file.getFileName();
       String name = filename.toString();
       if (name.endsWith(".class")) {
         byte[] content = Files.readAllBytes(file);
         addEntry(file, content);
       }
-      return FileVisitResult.CONTINUE;
     }
 
     private byte[] manifestContent() throws IOException {