Always do filtering in execution (even after doing it in analysis)

Filtering only in analysis was neglecting the possibility of resources being in
filesets, the contents of which are not available in analysis. As such, we must
*always* filter in execution, even though it's usually just going to be a
no-op.

Also, add some documentation of same.

RELNOTES: none
PiperOrigin-RevId: 184722564

2 files changed, 12 insertions, 16 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
index a79a9ba0c1..4f9d7c6628 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProcessorBuilder.java
@@ -313,10 +313,6 @@ public class AndroidResourcesProcessorBuilder {
       builder.add("--conditionalKeepRules");
     }
 
-    if (resourceFilterFactory.hasDensities()) {
-      builder.add("--densities", resourceFilterFactory.getDensityString());
-    }
-
     configureCommonFlags(outs, inputs, builder);
 
     ParamFileInfo.Builder paramFileInfo = ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED);
@@ -382,15 +378,6 @@ public class AndroidResourcesProcessorBuilder {
     builder.addExecPath("--aapt", sdk.getAapt().getExecutable());
     configureCommonFlags(outs, inputs, builder);
 
-    if (resourceFilterFactory.hasDensities()) {
-      // If we did not filter by density in analysis, filter in execution. Otherwise, don't filter
-      // in execution, but still pass the densities so they can be added to the manifest.
-      if (resourceFilterFactory.isPrefiltering()) {
-        builder.add("--densitiesForManifest", resourceFilterFactory.getDensityString());
-      } else {
-        builder.add("--densities", resourceFilterFactory.getDensityString());
-      }
-    }
     ImmutableList<String> filteredResources =
         resourceFilterFactory.getResourcesToIgnoreInExecution();
     if (!filteredResources.isEmpty()) {
@@ -494,11 +481,18 @@ public class AndroidResourcesProcessorBuilder {
       builder.addExecPath("--packagePath", apkOut);
       outs.add(apkOut);
     }
+
+    // Always pass density and resource configuration filter strings to execution, even when
+    // filtering in analysis. Filtering in analysis cannot remove resources from Filesets, and, in
+    // addition, aapt needs access to resource filters to generate pseudolocalized resources and
+    // because its resource filtering is somewhat stricter for locales, and
+    // resource processing needs access to densities to add them to the manifest.
     if (resourceFilterFactory.hasConfigurationFilters()) {
-      // Always pass filters to aapt, even if we filtered in analysis, since aapt is stricter and
-      // might remove resources that we previously accepted.
       builder.add("--resourceConfigs", resourceFilterFactory.getConfigurationFilterString());
     }
+    if (resourceFilterFactory.hasDensities()) {
+      builder.add("--densities", resourceFilterFactory.getDensityString());
+    }
     if (!uncompressedExtensions.isEmpty()) {
       builder.addAll("--uncompressedExtensions", VectorArg.join(",").each(uncompressedExtensions));
     }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java
index 7f2b6ae570..419c6042ac 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java
@@ -89,7 +89,9 @@ public class ResourceFilterFactory {
     /**
      * Resources will be filtered in analysis. In android_binary targets, all resources will be
      * filtered by this class, and only resources that are accepted will be passed to resource
-     * processing actions.
+     * processing actions. Resources may be filtered again in execution to handle some edge cases,
+     * including resources contained in Filesets, Pseudolocalized resource generation, and stricter
+     * filtering implementations within aapt.
      */
     FILTER_IN_ANALYSIS,