From 4c3098cfa6f00f90c7530b6f40d3e93062931c1d Mon Sep 17 00:00:00 2001 From: Laszlo Csomor Date: Thu, 15 Mar 2018 07:41:16 -0700 Subject: 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 --- .../build/android/AndroidResourceOutputs.java | 14 ++----------- .../build/android/AndroidResourceProcessor.java | 23 +++------------------- .../build/android/ManifestMergerAction.java | 4 ---- .../devtools/build/android/dexer/DexBuilder.java | 4 ---- 4 files changed, 5 insertions(+), 40 deletions(-) (limited to 'src/tools/android/java/com/google/devtools') diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceOutputs.java b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceOutputs.java index a4e9344c91..b277e010e2 100644 --- a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceOutputs.java +++ b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceOutputs.java @@ -30,8 +30,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; -import java.nio.file.attribute.FileTime; import java.util.ArrayList; +import java.util.Calendar; import java.util.Collection; import java.util.GregorianCalendar; import java.util.List; @@ -56,7 +56,7 @@ public class AndroidResourceOutputs { // The earliest date representable in a zip file, 1-1-1980 (the DOS epoch). private static final long ZIP_EPOCH = - new GregorianCalendar(1980, 01, 01, 0, 0).getTimeInMillis(); + new GregorianCalendar(1980, Calendar.JANUARY, 01, 0, 0).getTimeInMillis(); private final ZipOutputStream zip; @@ -299,8 +299,6 @@ public class AndroidResourceOutputs { try { Files.createDirectories(manifestOut.getParent()); Files.copy(provider.getManifest(), manifestOut); - // Set to the epoch for caching purposes. - Files.setLastModifiedTime(manifestOut, FileTime.fromMillis(0L)); } catch (IOException e) { throw new RuntimeException(e); } @@ -332,8 +330,6 @@ public class AndroidResourceOutputs { // outputs. This state occurs when there are no resource directories. Files.createFile(rOutput); } - // Set to the epoch for caching purposes. - Files.setLastModifiedTime(rOutput, FileTime.fromMillis(0L)); } catch (IOException e) { throw new RuntimeException(e); } @@ -349,8 +345,6 @@ public class AndroidResourceOutputs { visitor.writeEntries(); visitor.writeManifestContent(); } - // Set to the epoch for caching purposes. - Files.setLastModifiedTime(classJar, FileTime.fromMillis(0L)); } catch (IOException e) { throw new RuntimeException(e); } @@ -366,8 +360,6 @@ public class AndroidResourceOutputs { Files.walkFileTree(root, visitor); visitor.writeEntries(); } - // Set to the epoch for caching purposes. - Files.setLastModifiedTime(archive, FileTime.fromMillis(0L)); } catch (IOException e) { throw new RuntimeException(e); } @@ -383,8 +375,6 @@ public class AndroidResourceOutputs { Files.walkFileTree(generatedSourcesRoot, visitor); visitor.writeEntries(); } - // Set to the epoch for caching purposes. - Files.setLastModifiedTime(srcJar, FileTime.fromMillis(0L)); } catch (IOException e) { throw new RuntimeException(e); } 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 2ff8330046..a72d1d243a 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 @@ -57,7 +57,6 @@ import java.nio.charset.StandardCharsets; import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.attribute.FileTime; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -343,24 +342,11 @@ public class AndroidResourceProcessor { dependencyData, customPackageForR, androidManifest, sourceOut); } // Reset the output date stamps. - if (proguardOut != null) { - Files.setLastModifiedTime(proguardOut, FileTime.fromMillis(0L)); - } - if (mainDexProguardOut != null) { - Files.setLastModifiedTime(mainDexProguardOut, FileTime.fromMillis(0L)); - } if (packageOut != null) { - Files.setLastModifiedTime(packageOut, FileTime.fromMillis(0L)); if (!splits.isEmpty()) { - Iterable splitFilenames = findAndRenameSplitPackages(packageOut, splits); - for (Path splitFilename : splitFilenames) { - Files.setLastModifiedTime(splitFilename, FileTime.fromMillis(0L)); - } + renameSplitPackages(packageOut, splits); } } - if (publicResourcesOut != null && Files.exists(publicResourcesOut)) { - Files.setLastModifiedTime(publicResourcesOut, FileTime.fromMillis(0L)); - } return new MergedAndroidData(resourceDir, assetsDir, androidManifest); } @@ -623,8 +609,8 @@ public class AndroidResourceProcessor { } } - /** Finds aapt's split outputs and renames them according to the input flags. */ - private Iterable findAndRenameSplitPackages(Path packageOut, Iterable splits) + /** Renames aapt's split outputs according to the input flags. */ + private void renameSplitPackages(Path packageOut, Iterable splits) throws UnrecognizedSplitsException, IOException { String prefix = packageOut.getFileName().toString() + "_"; // The regex java string literal below is received as [\\{}\[\]*?] by the regex engine, @@ -641,16 +627,13 @@ public class AndroidResourceProcessor { } Map outputs = SplitConfigurationFilter.mapFilenamesToSplitFlags(filenameSuffixes.build(), splits); - ImmutableList.Builder outputPaths = new ImmutableList.Builder<>(); for (Map.Entry splitMapping : outputs.entrySet()) { Path resultPath = packageOut.resolveSibling(prefix + splitMapping.getValue()); - outputPaths.add(resultPath); if (!splitMapping.getKey().equals(splitMapping.getValue())) { Path sourcePath = packageOut.resolveSibling(prefix + splitMapping.getKey()); Files.move(sourcePath, resultPath); } } - return outputPaths.build(); } /** A logger that will print messages to a target OutputStream. */ diff --git a/src/tools/android/java/com/google/devtools/build/android/ManifestMergerAction.java b/src/tools/android/java/com/google/devtools/build/android/ManifestMergerAction.java index a7301f1030..3fb09d2dff 100644 --- a/src/tools/android/java/com/google/devtools/build/android/ManifestMergerAction.java +++ b/src/tools/android/java/com/google/devtools/build/android/ManifestMergerAction.java @@ -34,7 +34,6 @@ import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; -import java.nio.file.attribute.FileTime; import java.util.Map; import java.util.Map.Entry; import java.util.logging.Logger; @@ -217,9 +216,6 @@ public class ManifestMergerAction { if (!mergedManifest.equals(options.manifestOutput)) { Files.copy(options.manifest, options.manifestOutput, StandardCopyOption.REPLACE_EXISTING); } - - // Set to the epoch for caching purposes. - Files.setLastModifiedTime(options.manifestOutput, FileTime.fromMillis(0L)); } catch (AndroidManifestProcessor.ManifestProcessingException e) { System.exit(1); } catch (Exception e) { diff --git a/src/tools/android/java/com/google/devtools/build/android/dexer/DexBuilder.java b/src/tools/android/java/com/google/devtools/build/android/dexer/DexBuilder.java index eadc42532f..ceb41e6ff2 100644 --- a/src/tools/android/java/com/google/devtools/build/android/dexer/DexBuilder.java +++ b/src/tools/android/java/com/google/devtools/build/android/dexer/DexBuilder.java @@ -138,8 +138,6 @@ class DexBuilder { executor.shutdown(); } } - // Use input's timestamp for output file so the output file is stable. - Files.setLastModifiedTime(options.outputZip, Files.getLastModifiedTime(options.inputJar)); } /** @@ -225,8 +223,6 @@ class DexBuilder { new Dexing(context, optionsParser.getOptions(DexingOptions.class)), dexCache); } - // Use input's timestamp for output file so the output file is stable. - Files.setLastModifiedTime(options.outputZip, Files.getLastModifiedTime(options.inputJar)); } private static ZipOutputStream createZipOutputStream(Path path) throws IOException { -- cgit v1.2.3