Automated rollback of commit feeccd8.

*** Reason for rollback ***

Rolling forward with correct handling for pseudo locale generation flags

*** Original change description ***

Only include generated resources when pseudo locales are asked for.

CompileResources: generate pseudo locales into a separate compiled resource file when the pseudo locale flag is true and the locale is default (e.g. absent). The generated resources must be first in the returned list of compiled resource paths. This allows the user to define custom values to the pseudo locales (which, for some obscure reason, is accepted as a reasonable practice by aapt{,2})
AndroidResourceOutputs: record the type of compiled resource in the comment field of the zip entry for fast comparison.
ResourceLinker: only include the generated resources when the pseudo locale is explicitly asked for. It's important to note that the ordering for compiled resources in the zip goes <generated>...<normal>...<default>. This means the default locale will overwrite the generated locale values. Annoying, but necessary, as that is current order before introducing this cl.

RELNOTES:None
PiperOrigin-RevId: 196159094

src/test/java/com/google/devtools/build/android/PathsSubject.java

@@ -14,10 +14,12 @@
 package com.google.devtools.build.android;
 
 import static com.google.common.truth.Truth.assertThat;
+import static java.util.stream.Collectors.joining;
 
 import com.google.common.base.Joiner;
 import com.google.common.truth.FailureMetadata;
 import com.google.common.truth.Subject;
+import com.google.devtools.build.android.aapt2.ResourceCompiler.CompiledType;
 import java.io.IOException;
 import java.io.StringReader;
 import java.io.StringWriter;
@@ -45,6 +47,14 @@
 /** A testing utility that allows assertions against Paths. */
 public class PathsSubject extends Subject<PathsSubject, Path> {
 
+  private static final String PATH_NORMALIZER =
+      String.format(
+          "(%s)/(.*/)(javatests)",
+          Arrays.stream(CompiledType.values())
+              .map(CompiledType::asPrefix)
+              .map(p -> String.format("(?:%s)", p))
+              .collect(joining("|")));
+
   PathsSubject(FailureMetadata failureMetadata, @Nullable Path subject) {
     super(failureMetadata, subject);
   }
@@ -68,10 +78,27 @@ void containsAllArchivedFilesIn(String... paths) throws IOException {
             new ZipFile(actual().toFile())
                 .stream()
                 .map(ZipEntry::getName)
+                .map(n -> n.replaceAll(PATH_NORMALIZER, "$1/$3"))
                 .collect(Collectors.toSet()))
         .containsAllIn(Arrays.asList(paths));
   }
 
+
+  void containsExactlyArchivedFilesIn(String... paths) throws IOException {
+    if (actual() == null) {
+      fail("should not be null.");
+    }
+    exists();
+
+    assertThat(
+        new ZipFile(actual().toFile())
+            .stream()
+            .map(ZipEntry::getName)
+            .map(n -> n.replaceAll(PATH_NORMALIZER, "$1/$3"))
+            .collect(Collectors.toSet()))
+        .containsExactly(paths);
+  }
+
   void containsNoArchivedFilesIn(String... paths) throws IOException {
     if (actual() == null) {
       fail("should not be null.");
@@ -81,6 +108,7 @@ void containsNoArchivedFilesIn(String... paths) throws IOException {
             new ZipFile(actual().toFile())
                 .stream()
                 .map(ZipEntry::getName)
+                .map(n -> n.replaceAll(PATH_NORMALIZER, "$1/$3"))
                 .collect(Collectors.toSet()))
         .containsNoneIn(Arrays.asList(paths));
   }

src/tools/android/java/com/google/devtools/build/android/Aapt2ResourcePackagingAction.java

@@ -175,6 +175,7 @@ public static void main(String[] args) throws Exception {
               .conditionalKeepRules(aaptConfigOptions.conditionalKeepRules == TriState.YES)
               .filterToDensity(options.densities)
               .debug(aaptConfigOptions.debug)
+              .includeGeneratedLocales(aaptConfigOptions.generatePseudoLocale)
               .includeOnlyConfigs(aaptConfigOptions.resourceConfigs)
               .link(compiled)
               .copyPackageTo(options.packagePath)

src/tools/android/java/com/google/devtools/build/android/AndroidCompiledDataDeserializer.java

@@ -302,7 +302,7 @@ private void readPackages(KeyValueConsumers consumers, ResourceTable resourceTab
               String source = sourcePool.get(sourceIndex);
               DataSource dataSource = DataSource.of(Paths.get(source));
 
-              Value resourceValue = resource.getConfigValue(0).getValue();
+              Value resourceValue = configValue.getValue();
 
               DataResource dataResource =
                   resourceValue.getItem().hasFile()

src/tools/android/java/com/google/devtools/build/android/AndroidResourceOutputs.java

@@ -18,7 +18,9 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
 import com.google.common.collect.Ordering;
+import com.google.devtools.build.android.aapt2.ResourceCompiler;
 import java.io.BufferedOutputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.Closeable;
@@ -87,6 +89,12 @@ protected long normalizeTime(String filename) {
     }
 
     protected void addEntry(String rawName, byte[] content, int storageMethod) throws IOException {
+      addEntry(rawName, content, storageMethod, null);
+    }
+
+    protected void addEntry(
+        String rawName, byte[] content, int storageMethod, @Nullable String comment)
+        throws IOException {
       // Fix the path for windows.
       String relativeName = rawName.replace('\\', '/');
       // Make sure the zip entry is not absolute.
@@ -99,15 +107,18 @@ protected void addEntry(String rawName, byte[] content, int storageMethod) throw
       CRC32 crc32 = new CRC32();
       crc32.update(content);
       entry.setCrc(crc32.getValue());
+      if (!Strings.isNullOrEmpty(comment)) {
+        entry.setComment(comment);
+      }
 
       zip.putNextEntry(entry);
       zip.write(content);
       zip.closeEntry();
     }
 
     protected void addEntry(ZipEntry entry, byte[] content) throws IOException {
-      //Create a new ZipEntry because there are occasional discrepancies
-      //between the metadata and written content.
+      // Create a new ZipEntry because there are occasional discrepancies
+      // between the metadata and written content.
       ZipEntry newEntry = new ZipEntry(entry.getName());
       zip.putNextEntry(newEntry);
       zip.write(content);
@@ -412,6 +423,7 @@ public static Path archiveCompiledResources(
     try (ZipBuilder builder = ZipBuilder.createFor(archiveOut)) {
       for (Path artifact : compiledArtifacts) {
         Path relativeName = artifact;
+
         // remove compiled resources prefix
         if (artifact.startsWith(compiledRoot)) {
           relativeName = compiledRoot.relativize(relativeName);
@@ -424,7 +436,11 @@ public static Path archiveCompiledResources(
                   relativeName.getNameCount());
         }
 
-        builder.addEntry(relativeName.toString(), Files.readAllBytes(artifact), ZipEntry.STORED);
+        builder.addEntry(
+            relativeName.toString(),
+            Files.readAllBytes(artifact),
+            ZipEntry.STORED,
+            ResourceCompiler.getCompiledType(relativeName.toString()).asComment());
       }
     }
     return archiveOut;

src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceCompiler.java

@@ -40,6 +40,7 @@
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -60,6 +61,43 @@
 
 /** Invokes aapt2 to compile resources. */
 public class ResourceCompiler {
+
+  /** Types of compiled resources. */
+  public enum CompiledType {
+    NORMAL(null),
+    GENERATED("generated"),
+    DEFAULT("default");
+
+    private final String prefix;
+
+    CompiledType(String prefix) {
+      this.prefix = prefix;
+    }
+
+    boolean prefixes(String filename) {
+      return prefix != null && filename.startsWith(prefix);
+    }
+
+    public String asPrefix() {
+      return prefix;
+    }
+
+    public String asComment() {
+      return prefix;
+    }
+
+    public String prefix(String path) {
+      return prefix + "/" + path;
+    }
+  }
+
+  public static CompiledType getCompiledType(String fileName) {
+    return Arrays.stream(CompiledType.values())
+        .filter(t -> t.prefixes(fileName))
+        .findFirst()
+        .orElse(CompiledType.NORMAL);
+  }
+
   static class CompileError extends Aapt2Exception {
 
     protected CompileError(Throwable e) {
@@ -105,19 +143,28 @@ private CompileTask(
     @Override
     public List<Path> call() throws Exception {
       final String directoryName = file.getParent().getFileName().toString();
-      Qualifiers qualifiers = Qualifiers.parseFrom(directoryName);
-      String filename = interpolateAapt2Filename(qualifiers, file.getFileName().toString());
+      final Qualifiers qualifiers = Qualifiers.parseFrom(directoryName);
+      final String filename = interpolateAapt2Filename(qualifiers, file.getFileName().toString());
 
-      List<Path> results = new ArrayList<>();
-      compile(directoryName, filename, results, compiledResourcesOut, file, false);
+      final List<Path> results = new ArrayList<>();
       if (qualifiers.asFolderType().equals(ResourceFolderType.VALUES)) {
         extractAttributes(directoryName, filename, results);
+      }
 
-        if (qualifiers.containDefaultLocale()) {
-          // aapt2 only generates pseudo locales for the default locale.
-          generatedResourcesOut.ifPresent(
-              out -> compile(directoryName, filename, results, out, file, true));
-        }
+      if (qualifiers.containDefaultLocale()
+          && qualifiers.asFolderType().equals(ResourceFolderType.VALUES)) {
+        compile(
+            directoryName,
+            filename,
+            results,
+            compiledResourcesOut.resolve(CompiledType.DEFAULT.asPrefix()),
+            file,
+            false);
+        // aapt2 only generates pseudo locales for the default locale.
+        generatedResourcesOut.ifPresent(
+            out -> compile(directoryName, filename, results, out, file, true));
+      } else {
+        compile(directoryName, filename, results, compiledResourcesOut, file, false);
       }
       return results;
     }
@@ -362,17 +409,33 @@ List<Path> getCompiledArtifacts() {
                     generatedResourcesOut)));
       }
 
-      ImmutableList.Builder<Path> builder = ImmutableList.builder();
+      ImmutableList.Builder<Path> compiled = ImmutableList.builder();
+      ImmutableList.Builder<Path> generated = ImmutableList.builder();
       List<Throwable> compilationErrors = new ArrayList<>();
       for (ListenableFuture<List<Path>> task : tasks) {
         try {
-          builder.addAll(task.get());
+          // Split the generated and non-generated resources into different collections.
+          // This allows the generated files to be placed first in the compile order,
+          // ensuring that the generated locale (en-XA and ar-XB) can be overwritten by
+          // user provided versions for those locales, as aapt2 will take the last value for
+          // a configuration when linking.
+          task.get()
+              .forEach(
+                  path -> {
+                    if (generatedResourcesOut.map(path::startsWith).orElse(false)) {
+                      generated.add(path);
+                    } else {
+                      compiled.add(path);
+                    }
+                  });
         } catch (InterruptedException | ExecutionException e) {
-          compilationErrors.add(Optional.ofNullable(e.getCause()).orElse(e));
+          compilationErrors.add(e.getCause() != null ? e.getCause() : e);
         }
       }
+      generated.addAll(compiled.build());
       if (compilationErrors.isEmpty()) {
-        return builder.build();
+        // ensure that the generated files are before the normal files.
+        return generated.build();
       }
       throw CompileError.of(compilationErrors);
     }
@@ -393,7 +456,7 @@ public static ResourceCompiler create(
             aapt2,
             buildToolsVersion,
             generatePseudoLocale
-                ? Optional.of(compiledResources.resolve("generated"))
+                ? Optional.of(compiledResources.resolve(CompiledType.GENERATED.asPrefix()))
                 : Optional.empty()));
   }