Android tools: remove mtime-modifications

The Android tools no longer modify output file
mtimes in hopes of achievening better action
cache hits.

Modifying the mtimes was confusing Bazel and
causing correctness bugs.

Modifying the mtimes is unnecessary because Bazel
is smart about picking up filesystem changes and
observes more signals than just the mtime, though
as the corresponding bug shows it's sadly not
bullet-proof.

Fixes https://github.com/bazelbuild/bazel/issues/4734

Change-Id: I4aa8abf29486841ba8133f927e2816d7f85881fe

Closes #4848.

Change-Id: I0615fae1f20d786771d742705ab4a6ddf7f2306e
PiperOrigin-RevId: 189183742

1 files changed, 40 insertions, 7 deletions

diff --git a/src/test/java/com/google/devtools/build/android/RClassGeneratorActionTest.java b/src/test/java/com/google/devtools/build/android/RClassGeneratorActionTest.java
index e03348cdaa..7e1d8741ca 100644
--- a/src/test/java/com/google/devtools/build/android/RClassGeneratorActionTest.java
+++ b/src/test/java/com/google/devtools/build/android/RClassGeneratorActionTest.java
@@ -23,11 +23,12 @@ import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.attribute.FileTime;
+import java.time.Instant;
 import java.util.Collections;
 import java.util.List;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -123,7 +124,6 @@ public class RClassGeneratorActionTest {
             .toArray(new String[0]));
 
     assertThat(Files.exists(jarPath)).isTrue();
-    assertThat(Files.getLastModifiedTime(jarPath)).isEqualTo(FileTime.fromMillis(0));
 
     try (ZipFile zip = new ZipFile(jarPath.toFile())) {
       List<? extends ZipEntry> zipEntries = Collections.list(zip.entries());
@@ -144,6 +144,7 @@ public class RClassGeneratorActionTest {
               "com/google/app/R$string.class",
               "com/google/app/R.class",
               "META-INF/MANIFEST.MF");
+      ZipMtimeAsserter.assertEntries(zipEntries);
     }
   }
 
@@ -175,7 +176,6 @@ public class RClassGeneratorActionTest {
             .toArray(new String[0]));
 
     assertThat(Files.exists(jarPath)).isTrue();
-    assertThat(Files.getLastModifiedTime(jarPath)).isEqualTo(FileTime.fromMillis(0));
 
     try (ZipFile zip = new ZipFile(jarPath.toFile())) {
       List<? extends ZipEntry> zipEntries = Collections.list(zip.entries());
@@ -190,6 +190,7 @@ public class RClassGeneratorActionTest {
               "com/google/bar/R$drawable.class",
               "com/google/bar/R.class",
               "META-INF/MANIFEST.MF");
+      ZipMtimeAsserter.assertEntries(zipEntries);
     }
   }
 
@@ -219,7 +220,6 @@ public class RClassGeneratorActionTest {
     ).toArray(new String[0]));
 
     assertThat(Files.exists(jarPath)).isTrue();
-    assertThat(Files.getLastModifiedTime(jarPath)).isEqualTo(FileTime.fromMillis(0));
 
     try (ZipFile zip = new ZipFile(jarPath.toFile())) {
       List<? extends ZipEntry> zipEntries = Collections.list(zip.entries());
@@ -233,6 +233,7 @@ public class RClassGeneratorActionTest {
               "com/google/app/R$string.class",
               "com/google/app/R.class",
               "META-INF/MANIFEST.MF");
+      ZipMtimeAsserter.assertEntries(zipEntries);
     }
   }
 
@@ -244,12 +245,12 @@ public class RClassGeneratorActionTest {
     ).toArray(new String[0]));
 
     assertThat(Files.exists(jarPath)).isTrue();
-    assertThat(Files.getLastModifiedTime(jarPath)).isEqualTo(FileTime.fromMillis(0));
 
     try (ZipFile zip = new ZipFile(jarPath.toFile())) {
       List<? extends ZipEntry> zipEntries = Collections.list(zip.entries());
       Iterable<String> entries = getZipFilenames(zipEntries);
       assertThat(entries).containsExactly("META-INF/MANIFEST.MF");
+      ZipMtimeAsserter.assertEntries(zipEntries);
     }
   }
 
@@ -285,7 +286,6 @@ public class RClassGeneratorActionTest {
             .toArray(new String[0]));
 
     assertThat(Files.exists(jarPath)).isTrue();
-    assertThat(Files.getLastModifiedTime(jarPath)).isEqualTo(FileTime.fromMillis(0));
 
     try (ZipFile zip = new ZipFile(jarPath.toFile())) {
       List<? extends ZipEntry> zipEntries = Collections.list(zip.entries());
@@ -299,6 +299,7 @@ public class RClassGeneratorActionTest {
               "com/custom/er/R$string.class",
               "com/custom/er/R.class",
               "META-INF/MANIFEST.MF");
+      ZipMtimeAsserter.assertEntries(zipEntries);
     }
   }
 
@@ -323,12 +324,12 @@ public class RClassGeneratorActionTest {
             .toArray(new String[0]));
 
     assertThat(Files.exists(jarPath)).isTrue();
-    assertThat(Files.getLastModifiedTime(jarPath)).isEqualTo(FileTime.fromMillis(0));
 
     try (ZipFile zip = new ZipFile(jarPath.toFile())) {
       List<? extends ZipEntry> zipEntries = Collections.list(zip.entries());
       Iterable<String> entries = getZipFilenames(zipEntries);
       assertThat(entries).containsExactly("META-INF/MANIFEST.MF");
+      ZipMtimeAsserter.assertEntries(zipEntries);
     }
   }
 
@@ -349,4 +350,36 @@ public class RClassGeneratorActionTest {
           }
         });
   }
+
+  private static final class ZipMtimeAsserter {
+    private static final long ZIP_EPOCH = Instant.parse("1980-01-01T00:00:00Z").getEpochSecond();
+    private static final long ZIP_EPOCH_PLUS_ONE_DAY =
+        Instant.parse("1980-01-02T00:00:00Z").getEpochSecond();
+
+    public static void assertEntry(ZipEntry e) {
+      // getLastModifiedTime().toMillis() returns milliseconds, Instant.getEpochSecond() returns
+      // seconds.
+      long mtime = e.getLastModifiedTime().toMillis() / 1000;
+      // The ZIP epoch is the same as the MS-DOS epoch, 1980-01-01T00:00:00Z.
+      // AndroidResourceOutputs.ZipBuilder sets this to most of its entries, except for .class files
+      // for which the ZipBuilder increments the timestamp by 2 seconds.
+      // We don't care about the details of this logic and asserting exact timestamps would couple
+      // the test to the code too tightly, so here we only assert that the timestamp is on
+      // 1980-01-01, ignoring the exact time.
+      // AndroidResourceOutputs.ZipBuilde sets the ZIP epoch (same as the MS-DOS epoch,
+      // 1980-01-01T00:00:00Z) as the timestamp for all of its entries (except .class files, for
+      // which it sets a timestamp 2 seconds later than the DOS epoch).
+      // We don't care about the exact timestamps though, only that they are stable, so let's just
+      // assert that they are all on the day of 1980-01-01.
+      if (mtime < ZIP_EPOCH || mtime > ZIP_EPOCH_PLUS_ONE_DAY) {
+        Assert.fail(String.format("e=(%s) mtime=(%s)", e.getName(), e.getLastModifiedTime()));
+      }
+    }
+
+    public static void assertEntries(Iterable<? extends ZipEntry> entries) throws Exception {
+      for (ZipEntry e : entries) {
+        assertEntry(e);
+      }
+    }
+  }
 }