When filtering resources in analysis, also filter resource roots

If all resources from a directory are filtered out, the resource processing
action should not receive a reference to that directory.

Filtering the entire contents of a directory is uncommon but not impossible. It
may be indicative of an error (if the user filters on a language, but only
provided resources in other languages - in this case, the build should and will
fail during Java compilation when the requested resource does not exist) or of
weird but acceptable behavior (placing resources for some default language in
one resource directory and resources for all other languages in some other
directory, then inheriting both directories and filtering on language) that
should be correctly handled.

RELNOTES: none
PiperOrigin-RevId: 157499745

2 files changed, 86 insertions, 2 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java
index d66961b48d..758eca0019 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java
@@ -126,10 +126,32 @@ public abstract class ResourceContainer {
   public abstract Builder toBuilder();
 
   /**
-   * Returns a copy of this container with filtered resources. The original container is unchanged.
+   * Returns a copy of this container with filtered resources, or the original if no resources
+   * should be filtered. The original container is unchanged.
    */
   public ResourceContainer filter(RuleContext ruleContext, ResourceFilter filter) {
-    return toBuilder().setResources(filter.filter(ruleContext, getResources())).build();
+    ImmutableList<Artifact> filteredResources = filter.filter(ruleContext, getResources());
+
+    if (filteredResources.size() == getResources().size()) {
+      // No filtering was done; return this container
+      return this;
+    }
+
+    // If the resources were filtered, also filter the resource roots
+    ImmutableList.Builder<PathFragment> filteredResourcesRootsBuilder = ImmutableList.builder();
+    for (PathFragment resourceRoot : getResourcesRoots()) {
+      for (Artifact resource : filteredResources) {
+        if (resource.getRootRelativePath().startsWith(resourceRoot)) {
+          filteredResourcesRootsBuilder.add(resourceRoot);
+          break;
+        }
+      }
+    }
+
+    return toBuilder()
+        .setResources(filteredResources)
+        .setResourcesRoots(filteredResourcesRootsBuilder.build())
+        .build();
   }
 
   /** Creates a new builder with default values. */
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
index 9afaf68798..1f3767221e 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
@@ -1504,6 +1504,68 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
         .containsAllOf(matchingResource, unqualifiedResource);
   }
 
+  @Test
+  public void testFilteredResourcesAllFilteredOut() throws Exception {
+    String dir = "java/r/android";
+
+    useConfiguration("--experimental_android_resource_filtering_method", "filter_in_analysis");
+
+    final String keptBaseDir = "partly_filtered_dir";
+    String removedLibraryDir = "fully_filtered_library_dir";
+    String removedBinaryDir = "fully_filtered_binary_dir";
+
+    ConfiguredTarget binary =
+        scratchConfiguredTarget(
+            dir,
+            "bin",
+            "android_library(name = 'not_fully_filtered_lib',",
+            "  manifest = 'AndroidManifest.xml',",
+            "  resource_files = [",
+            // Some of the resources in this directory are kept:
+            "    '" + keptBaseDir + "/values-es/foo.xml',",
+            "    '" + keptBaseDir + "/values-fr/foo.xml',",
+            "    '" + keptBaseDir + "/values-en/foo.xml',",
+            "  ])",
+            "android_library(name = 'fully_filtered_lib',",
+            "  manifest = 'AndroidManifest.xml',",
+            "  resource_files = [",
+            // All of the resources in this directory are filtered out:
+            "    '" + removedLibraryDir + "/values-es/foo.xml',",
+            "    '" + removedLibraryDir + "/values-fr/foo.xml',",
+            "  ])",
+            "android_binary(name = 'bin',",
+            "  manifest = 'AndroidManifest.xml',",
+            "  resource_configuration_filters = ['en'],",
+            "  resource_files = [",
+            // All of the resources in this directory are filtered out:
+            "    '" + removedBinaryDir + "/values-es/foo.xml',",
+            "    '" + removedBinaryDir + "/values-fr/foo.xml',",
+            "  ],",
+            "  deps = [",
+            "    ':fully_filtered_lib',",
+            "    ':not_fully_filtered_lib',",
+            "  ])");
+
+    List<String> resourceProcessingArgs =
+        getGeneratingSpawnAction(getResourceContainer(binary).getRTxt()).getArguments();
+
+    assertThat(
+            Iterables.filter(
+                resourceProcessingArgs,
+                new Predicate<String>() {
+                  @Override
+                  public boolean apply(String input) {
+                    return input.contains(keptBaseDir);
+                  }
+                }))
+        .isNotEmpty();
+
+    for (String arg : resourceProcessingArgs) {
+      assertThat(arg).doesNotContain(removedLibraryDir);
+      assertThat(arg).doesNotContain(removedBinaryDir);
+    }
+  }
+
   /**
    * Gets the paths of matching artifacts contained within a resource container
    *