Generate a separate psuedo locale .flat file only for default locale on values resources.

RELNOTES:None
PiperOrigin-RevId: 195821595

3 files changed, 180 insertions, 108 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/FullyQualifiedName.java b/src/tools/android/java/com/google/devtools/build/android/FullyQualifiedName.java
index 64fbab796a..e29361ab74 100644
--- a/src/tools/android/java/com/google/devtools/build/android/FullyQualifiedName.java
+++ b/src/tools/android/java/com/google/devtools/build/android/FullyQualifiedName.java
@@ -486,7 +486,8 @@ public class FullyQualifiedName implements DataKey {
   /** Represents the configuration qualifiers in a resource directory. */
   public static class Qualifiers {
 
-    private static final Qualifiers EMPTY_QUALIFIERS = new Qualifiers(null, ImmutableList.of());
+    private static final Qualifiers EMPTY_QUALIFIERS =
+        new Qualifiers(null, ImmutableList.of(), false);
 
     // Qualifiers are reasonably expensive to create, so cache them on directory names.
     private static final ConcurrentMap<String, Qualifiers> qualifierCache =
@@ -495,10 +496,13 @@ public class FullyQualifiedName implements DataKey {
     public static final String INVALID_QUALIFIERS = "%s contains invalid qualifiers.";
     private final ResourceFolderType folderType;
     private final ImmutableList<String> qualifiers;
+    private boolean defaultLocale;
 
-    private Qualifiers(ResourceFolderType folderType, ImmutableList<String> qualifiers) {
+    private Qualifiers(
+        ResourceFolderType folderType, ImmutableList<String> qualifiers, boolean defaultLocale) {
       this.folderType = folderType;
       this.qualifiers = qualifiers;
+      this.defaultLocale = defaultLocale;
     }
 
     public static Qualifiers parseFrom(String directoryName) {
@@ -559,7 +563,7 @@ public class FullyQualifiedName implements DataKey {
       for (int i = 0; i < FolderConfiguration.getQualifierCount(); ++i) {
         addIfNotNull(config.getQualifier(i), builder);
       }
-      return new Qualifiers(folderType, builder.build());
+      return new Qualifiers(folderType, builder.build(), config.getLocaleQualifier() == null);
     }
 
     private static void addIfNotNull(
@@ -578,9 +582,15 @@ public class FullyQualifiedName implements DataKey {
       return folderType;
     }
 
+    /** Creates a Qualifiers assuming that they are in the values directory. */
     @VisibleForTesting
-    public static Qualifiers fromList(List<String> qualifiers) {
-      return new Qualifiers(null, ImmutableList.copyOf(qualifiers));
+    public static Qualifiers forValuesFolderFrom(List<String> qualifiers) {
+      return Qualifiers.getQualifiers(
+          ImmutableList.builder().add("values").addAll(qualifiers).build().toArray(new String[0]));
+    }
+
+    public boolean containDefaultLocale() {
+      return defaultLocale;
     }
   }
 
@@ -636,7 +646,7 @@ public class FullyQualifiedName implements DataKey {
 
     @VisibleForTesting
     public static Factory from(List<String> qualifiers, String pkg) {
-      return using(Qualifiers.fromList(qualifiers), pkg);
+      return using(Qualifiers.forValuesFolderFrom(qualifiers), pkg);
     }
 
     @VisibleForTesting
diff --git a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceCompiler.java b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceCompiler.java
index 355fcbf4d0..68e12e146e 100644
--- a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceCompiler.java
+++ b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceCompiler.java
@@ -14,8 +14,10 @@
 
 package com.google.devtools.build.android.aapt2;
 
+import com.android.SdkConstants;
 import com.android.builder.core.VariantType;
 import com.android.repository.Revision;
+import com.android.resources.ResourceFolderType;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.ListenableFuture;
@@ -25,6 +27,7 @@ import com.google.devtools.build.android.AndroidDataSerializer;
 import com.google.devtools.build.android.DataResourceXml;
 import com.google.devtools.build.android.FullyQualifiedName;
 import com.google.devtools.build.android.FullyQualifiedName.Factory;
+import com.google.devtools.build.android.FullyQualifiedName.Qualifiers;
 import com.google.devtools.build.android.FullyQualifiedName.VirtualType;
 import com.google.devtools.build.android.XmlResourceValues;
 import com.google.devtools.build.android.xml.Namespaces;
@@ -38,11 +41,10 @@ import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.ArrayList;
 import java.util.Iterator;
-import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.logging.Logger;
@@ -85,96 +87,125 @@ public class ResourceCompiler {
     private final Path compiledResourcesOut;
     private final Path aapt2;
     private final Revision buildToolsVersion;
-    private final boolean generatePseudoLocale;
+    private final Optional<Path> generatedResourcesOut;
 
     private CompileTask(
         Path file,
         Path compiledResourcesOut,
         Path aapt2,
         Revision buildToolsVersion,
-        boolean generatePseudoLocale) {
+        Optional<Path> generatedResourcesOut) {
       this.file = file;
       this.compiledResourcesOut = compiledResourcesOut;
       this.aapt2 = aapt2;
       this.buildToolsVersion = buildToolsVersion;
-      this.generatePseudoLocale = generatePseudoLocale;
+      this.generatedResourcesOut = generatedResourcesOut;
     }
 
     @Override
     public List<Path> call() throws Exception {
-      logger.fine(
-          new AaptCommandBuilder(aapt2)
-              .forBuildToolsVersion(buildToolsVersion)
-              .forVariantType(VariantType.LIBRARY)
-              .add("compile")
-              .add("-v")
-              .add("--legacy")
-              .when(generatePseudoLocale)
-              .thenAdd("--pseudo-localize")
-              .add("-o", compiledResourcesOut.toString())
-              .add(file.toString())
-              .execute("Compiling " + file));
-
-      String type = file.getParent().getFileName().toString();
-      String filename = file.getFileName().toString();
+      final String directoryName = file.getParent().getFileName().toString();
+      Qualifiers qualifiers = Qualifiers.parseFrom(directoryName);
+      String filename = interpolateAapt2Filename(qualifiers, file.getFileName().toString());
 
       List<Path> results = new ArrayList<>();
-      if (type.startsWith("values")) {
-        filename =
-            (filename.indexOf('.') != -1 ? filename.substring(0, filename.indexOf('.')) : filename)
-                + ".arsc";
+      compile(directoryName, filename, results, compiledResourcesOut, file, false);
+      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));
+        }
+      }
+      return results;
+    }
 
-        XMLEventReader xmlEventReader = null;
-        try {
-          // aapt2 compile strips out namespaces and attributes from the resources tag.
-          // Read them here separately and package them with the other flat files.
-          xmlEventReader =
-              XMLInputFactory.newInstance()
-                  .createXMLEventReader(new FileInputStream(file.toString()));
-
-          // Iterate through the XML until we find a start element.
-          // This should mimic xmlEventReader.nextTag() except that it also skips DTD elements.
-          StartElement rootElement = null;
-          while (xmlEventReader.hasNext()) {
-            XMLEvent event = xmlEventReader.nextEvent();
-            if (event.getEventType() != XMLStreamConstants.COMMENT
-                && event.getEventType() != XMLStreamConstants.DTD
-                && event.getEventType() != XMLStreamConstants.PROCESSING_INSTRUCTION
-                && event.getEventType() != XMLStreamConstants.SPACE
-                && event.getEventType() != XMLStreamConstants.START_DOCUMENT) {
-
-              // If the event should not be skipped, try parsing it as a start element here.
-              // If the event is not a start element, an appropriate exception will be thrown.
-              rootElement = event.asStartElement();
-              break;
-            }
-          }
+    static String interpolateAapt2Filename(Qualifiers qualifiers, String filename) {
+      return qualifiers.asFolderType().equals(ResourceFolderType.VALUES)
+          ? (filename.indexOf('.') != -1 ? filename.substring(0, filename.indexOf('.')) : filename)
+              + ".arsc"
+          : filename;
+    }
+
+    private void compile(
+        String type,
+        String filename,
+        List<Path> results,
+        Path compileOutRoot,
+        Path file,
+        boolean generatePseudoLocale) {
+      try {
+        Path destination = CompilingVisitor.destinationPath(file, compileOutRoot);
+        final Path compiledResourcePath = destination.resolve(type + "_" + filename + ".flat");
+
+        logger.fine(
+            new AaptCommandBuilder(aapt2)
+                .forBuildToolsVersion(buildToolsVersion)
+                .forVariantType(VariantType.LIBRARY)
+                .add("compile")
+                .add("-v")
+                .add("--legacy")
+                .when(generatePseudoLocale)
+                .thenAdd("--pseudo-localize")
+                .add("-o", destination.toString())
+                .add(file.toString())
+                .execute("Compiling " + file));
+
+        Preconditions.checkArgument(
+            Files.exists(compiledResourcePath),
+            "%s does not exists after aapt2 ran.",
+            compiledResourcePath);
+        results.add(compiledResourcePath);
+      } catch (IOException e) {
+        throw new CompileError(e);
+      }
+    }
 
-          if (rootElement == null) {
-            throw new Exception("No start element found in resource XML file: " + file.toString());
+    private void extractAttributes(String type, String filename, List<Path> results)
+        throws Exception {
+      XMLEventReader xmlEventReader = null;
+      try {
+        // aapt2 compile strips out namespaces and attributes from the resources tag.
+        // Read them here separately and package them with the other flat files.
+        xmlEventReader =
+            XMLInputFactory.newInstance()
+                .createXMLEventReader(new FileInputStream(file.toString()));
+
+        // Iterate through the XML until we find a start element.
+        // This should mimic xmlEventReader.nextTag() except that it also skips DTD elements.
+        StartElement rootElement = null;
+        while (xmlEventReader.hasNext()) {
+          XMLEvent event = xmlEventReader.nextEvent();
+          if (event.getEventType() != XMLStreamConstants.COMMENT
+              && event.getEventType() != XMLStreamConstants.DTD
+              && event.getEventType() != XMLStreamConstants.PROCESSING_INSTRUCTION
+              && event.getEventType() != XMLStreamConstants.SPACE
+              && event.getEventType() != XMLStreamConstants.START_DOCUMENT) {
+
+            // If the event should not be skipped, try parsing it as a start element here.
+            // If the event is not a start element, an appropriate exception will be thrown.
+            rootElement = event.asStartElement();
+            break;
           }
+        }
+
+        if (rootElement == null) {
+          throw new Exception("No start element found in resource XML file: " + file.toString());
+        }
 
-          Iterator<Attribute> attributeIterator =
-              XmlResourceValues.iterateAttributesFrom(rootElement);
+        Iterator<Attribute> attributeIterator =
+            XmlResourceValues.iterateAttributesFrom(rootElement);
 
-          if (attributeIterator.hasNext()) {
-            results.add(createAttributesProto(type, filename, attributeIterator));
-          }
-        } finally {
-          if (xmlEventReader != null) {
-            xmlEventReader.close();
-          }
+        if (attributeIterator.hasNext()) {
+          results.add(createAttributesProto(type, filename, attributeIterator));
+        }
+      } finally {
+        if (xmlEventReader != null) {
+          xmlEventReader.close();
         }
       }
-
-      final Path compiledResourcePath =
-          compiledResourcesOut.resolve(type + "_" + filename + ".flat");
-      Preconditions.checkArgument(
-          Files.exists(compiledResourcePath),
-          "%s does not exists after aapt2 ran.",
-          compiledResourcePath);
-      results.add(compiledResourcePath);
-      return results;
     }
 
     private Path createAttributesProto(
@@ -194,7 +225,8 @@ public class ResourceCompiler {
         Namespaces namespaces = Namespaces.from(qName);
         String attributeName = namespaceUri.isEmpty() ? localPart : prefix + ":" + localPart;
 
-        Factory fqnFactory = Factory.fromDirectoryName(type);
+        final String[] dirNameAndQualifiers = type.split(SdkConstants.RES_QUALIFIER_SEP);
+        Factory fqnFactory = Factory.fromDirectoryName(dirNameAndQualifiers);
         FullyQualifiedName fqn =
             fqnFactory.create(VirtualType.RESOURCES_ATTRIBUTE, qName.toString());
         ResourcesAttribute resourceAttribute =
@@ -218,23 +250,23 @@ public class ResourceCompiler {
   private static class CompilingVisitor extends SimpleFileVisitor<Path> {
 
     private final ListeningExecutorService executorService;
-    private final Path compiledResources;
-    private final Map<Path, Path> pathToProcessed = new LinkedHashMap<>();
+    private final Path compiledResourcesOut;
+    private final Set<Path> pathToProcessed = new LinkedHashSet<>();
     private final Path aapt2;
     private final Revision buildToolsVersion;
-    private final boolean generatePseudoLocale;
+    private final Optional<Path> generatedResourcesOut;
 
     public CompilingVisitor(
         ListeningExecutorService executorService,
-        Path compiledResources,
+        Path compiledResourcesOut,
         Path aapt2,
         Revision buildToolsVersion,
-        boolean generatePseudoLocale) {
+        Optional<Path> generatedResourcesOut) {
       this.executorService = executorService;
-      this.compiledResources = compiledResources;
+      this.compiledResourcesOut = compiledResourcesOut;
       this.aapt2 = aapt2;
       this.buildToolsVersion = buildToolsVersion;
-      this.generatePseudoLocale = generatePseudoLocale;
+      this.generatedResourcesOut = generatedResourcesOut;
     }
 
     static final Pattern REGION_PATTERN =
@@ -244,14 +276,7 @@ public class ResourceCompiler {
     public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
       // Ignore directories and "hidden" files that start with .
       if (!Files.isDirectory(file) && !file.getFileName().toString().startsWith(".")) {
-        // Creates a relative output path based on the input path under the
-        // compiledResources path.
-        Path outputDirectory =
-            Files.createDirectories(
-                compiledResources.resolve(
-                    (file.isAbsolute() ? file.getRoot().relativize(file) : file)
-                        .getParent()
-                        .getParent()));
+        Path outputDirectory = destinationPath(file, compiledResourcesOut);
 
         Path maybeFixedPath =
             file.getParent()
@@ -263,18 +288,17 @@ public class ResourceCompiler {
           if (!Files.exists(maybeFixedPath)) {
             logger.severe(
                 String.format(
-                    "The locale identifier in %s is not supported by aapt2. Converting to %s. "
+                    "The locale identifier  in %s is not supported by aapt2. Converting to %s. "
                         + "This will be an error in the future.",
                     file, maybeFixedPath));
             // Only use the processed path if doesn't exist. If it exists, there are is already
             // resources for that region.
-            pathToProcessed.put(
+            pathToProcessed.add(
                 Files.copy(
                     file,
                     Files.createDirectories(
                             outputDirectory.resolve(maybeFixedPath.getParent().getFileName()))
-                        .resolve(file.getFileName())),
-                outputDirectory);
+                        .resolve(file.getFileName())));
           } else {
             logger.severe(
                 String.format(
@@ -284,12 +308,26 @@ public class ResourceCompiler {
                     file, maybeFixedPath));
           }
         } else {
-          pathToProcessed.put(file, outputDirectory);
+          pathToProcessed.add(file);
         }
       }
       return super.visitFile(file, attrs);
     }
 
+    public static Path destinationPath(Path file, Path compiledResourcesOut) {
+      // Creates a relative output path based on the input path under the
+      // compiledResources path.
+      try {
+        return Files.createDirectories(
+            compiledResourcesOut.resolve(
+                (file.isAbsolute() ? file.getRoot().relativize(file) : file)
+                    .getParent()
+                    .getParent()));
+      } catch (IOException e) {
+        throw new CompileError(e);
+      }
+    }
+
     /** Aapt cannot interpret these regions so we rename them to get them to compile. */
     static Path maybeFixRegion(Path p) {
       Matcher matcher = REGION_PATTERN.matcher(p.toString());
@@ -303,16 +341,25 @@ public class ResourceCompiler {
     }
 
     List<Path> getCompiledArtifacts() {
+      generatedResourcesOut.ifPresent(
+          out -> {
+            try {
+              Files.createDirectories(out);
+            } catch (IOException e) {
+              throw new CompileError(e);
+            }
+          });
+
       List<ListenableFuture<List<Path>>> tasks = new ArrayList<>();
-      for (Entry<Path, Path> entry : pathToProcessed.entrySet()) {
+      for (Path uncompiled : pathToProcessed) {
         tasks.add(
             executorService.submit(
                 new CompileTask(
-                    entry.getKey(),
-                    entry.getValue(),
+                    uncompiled,
+                    compiledResourcesOut,
                     aapt2,
                     buildToolsVersion,
-                    generatePseudoLocale)));
+                    generatedResourcesOut)));
       }
 
       ImmutableList.Builder<Path> builder = ImmutableList.builder();
@@ -338,9 +385,16 @@ public class ResourceCompiler {
       Path aapt2,
       Revision buildToolsVersion,
       boolean generatePseudoLocale) {
+
     return new ResourceCompiler(
         new CompilingVisitor(
-            executorService, compiledResources, aapt2, buildToolsVersion, generatePseudoLocale));
+            executorService,
+            compiledResources,
+            aapt2,
+            buildToolsVersion,
+            generatePseudoLocale
+                ? Optional.of(compiledResources.resolve("generated"))
+                : Optional.empty()));
   }
 
   private ResourceCompiler(CompilingVisitor compilingVisitor) {
diff --git a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java
index adaeabbe72..c5c3236043 100644
--- a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java
+++ b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java
@@ -40,6 +40,7 @@ import java.util.Optional;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
 import java.util.function.Function;
+import java.util.function.Predicate;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
@@ -182,7 +183,10 @@ public class ResourceLinker {
               .thenAdd("--no-version-vectors")
               .when(outputAsProto)
               .thenAdd("--proto-format")
-              .addParameterableRepeated("-R", compiledResourcesToPaths(compiled), workingDirectory)
+              .addParameterableRepeated(
+                  "-R",
+                  compiledResourcesToPaths(compiled, s -> s.endsWith(".flat")),
+                  workingDirectory)
               .addRepeated("-I", pathsToLinkAgainst)
               .add("--auto-add-overlay")
               .add("-o", outPath)
@@ -232,19 +236,20 @@ public class ResourceLinker {
     }
   }
 
-  private List<String> compiledResourcesToPaths(CompiledResources compiled) throws IOException {
+  private List<String> compiledResourcesToPaths(
+      CompiledResources compiled, Predicate<String> shouldKeep) throws IOException {
     // Using sequential streams to maintain the overlay order for aapt2.
     return Stream.concat(include.stream(), Stream.of(compiled))
         .sequential()
         .map(CompiledResources::getZip)
-        .map(z -> executorService.submit(() -> filterZip(z)))
+        .map(z -> executorService.submit(() -> filterZip(z, shouldKeep)))
         .map(rethrowLinkError(Future::get))
         // the process will always take as long as the longest Future
         .map(Path::toString)
         .collect(toList());
   }
 
-  private Path filterZip(Path path) throws IOException {
+  private Path filterZip(Path path, Predicate<String> shouldKeep) throws IOException {
     Path outPath =
         workingDirectory
             .resolve("filtered")
@@ -262,7 +267,7 @@ public class ResourceLinker {
       final ZipOut zipOut = new ZipOut(outChannel, outPath.toString());
       zipIn.scanEntries(
           (in, header, dirEntry, data) -> {
-            if (header.getFilename().endsWith(".flat")) {
+            if (shouldKeep.test(header.getFilename())) {
               zipOut.nextEntry(dirEntry);
               zipOut.write(header);
               zipOut.write(data);
@@ -331,7 +336,10 @@ public class ResourceLinker {
                           compiled.getAssetsStrings().stream())
                       .collect(toList()))
               .addRepeated("-I", StaticLibrary.toPathStrings(linkAgainst))
-              .addParameterableRepeated("-R", compiledResourcesToPaths(compiled), workingDirectory)
+              .addParameterableRepeated(
+                  "-R",
+                  compiledResourcesToPaths(compiled, s -> s.endsWith(".flat")),
+                  workingDirectory)
               // Never compress apks.
               .add("-0", "apk")
               // Add custom no-compress extensions.