Use dynamically configured resource filtering

Actually hook up the resource filtering configuration transitions to
AndroidConfiguration's topLevelConfigurationHook.

Dynamic configuration is inefficient when multiple configurations are used.
Multiple configurations result in splitting the build graph and duplicating
work.

To avoid using multiple configurations as much as possible, dynamically
configured resource filtering will only be applied for top-level android_binary
targets. When android_binary targets are included as dependencies, it's very
likely that multiple binaries with many shared dependencies but different
configurations would be used. Only applying dynamic filtering to top-level
binaries removes this concern.

It is still possible to build multiple top-level binary targets with different
configurations at once. Previous versions of this code considered not using
dynamic configuration in that case as well, but that raised some unanswered
questions about whether Bazel's invariants should allow modifying one target's
configuration based solely on targets that happen to be building in parallel.
As a result, that optimization is not included for now; we assume that
developers manually building multiple similar binaries at once is relatively
rare (plus, dynamically configured resource filtering in general is not turned
on by default and will not be turned on by default until we're confident the
benefits outweigh the costs).

RELNOTES: none
PiperOrigin-RevId: 163464415

2 files changed, 71 insertions, 26 deletions

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 a64f633f2b..62c5057d8e 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
@@ -24,6 +24,7 @@ import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
+import com.google.common.eventbus.EventBus;
 import com.google.common.truth.Truth;
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -1513,6 +1514,10 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
 
     assertThat(resourceInputPaths(dir, directResources))
         .containsAllOf(matchingResource, unqualifiedResource);
+
+    String[] flagValues =
+        flagValue("--prefilteredResources", resourceArguments(directResources)).split(",");
+    assertThat(flagValues).asList().containsExactly("values-fr/foo.xml");
   }
 
   @Test
@@ -1595,6 +1600,65 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
     assertThat(resourceProcessingArgs).contains("--throwOnResourceConflict");
   }
 
+  @Test
+  public void testFilteredTransitiveResourcesDynamicConfiguration() throws Exception {
+    String enResource = "res/values-en/foo.xml";
+    String unqualifiedResource = "res/values/foo.xml";
+    String frResource = "res/values-fr/foo.xml";
+
+    String dir = "java/r/android";
+    scratch.file(
+        dir + "/BUILD",
+        "android_library(name = 'lib',",
+        "  manifest = 'AndroidManifest.xml',",
+        "  resource_files = [",
+        "    '" + enResource + "',",
+        "    '" + frResource + "',",
+        "    '" + unqualifiedResource + "',",
+        "])",
+        "android_binary(name = 'en',",
+        "  manifest = 'AndroidManifest.xml',",
+        "  resource_configuration_filters = ['en'],",
+        "  deps = [':lib'])");
+
+    useConfiguration(
+        "--experimental_android_resource_filtering_method",
+        "filter_in_analysis_with_dynamic_configuration");
+
+    ConfiguredTarget binary =
+        Iterables.getOnlyElement(
+            update(
+                    ImmutableList.of("//" + dir + ":en"),
+                    /* keepGoing= */ false,
+                    /* loadingPhaseThreads= */ 1,
+                    /* doAnalysis= */ true,
+                    new EventBus())
+                .getTargetsToBuild());
+
+    // Assert the resources were still filtered in analysis in the binary.
+    String expectedQualifiedResource =
+        binary.getLabel().toString().endsWith("en") ? enResource : frResource;
+
+    assertThat(resourceContentsPaths(dir, getResourceContainer(binary, /* transitive=*/ true)))
+        .containsExactly(expectedQualifiedResource, unqualifiedResource);
+
+    ConfiguredTarget library = getDirectPrerequisite(binary, "//" + dir + ":lib");
+
+    // Assert the resources were filtered in the library.
+    // This is only possible if the filters are correctly being passed using dynamic
+    // configuration.
+    assertThat(resourceContentsPaths(dir, getResourceContainer(library)))
+        .containsExactly(expectedQualifiedResource, unqualifiedResource);
+
+    // assert the correct prefix is used for library outputs
+    assertThat(
+            library
+                .getConfiguration()
+                .getFragment(AndroidConfiguration.class)
+                .getOutputDirectoryName())
+        .contains("en_");
+  }
+
   /**
    * Gets the paths of matching artifacts contained within a resource container
    *
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java
index e2d7b9f6f2..2fc4e372db 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceFilterTest.java
@@ -569,8 +569,7 @@ public class ResourceFilterTest extends ResourceTestBase {
                 ImmutableList.of("en"),
                 ImmutableList.of("hdpi"),
                 FilterBehavior.FILTER_IN_EXECUTION,
-                true,
-                1))
+                true))
         .isNull();
   }
 
@@ -581,32 +580,18 @@ public class ResourceFilterTest extends ResourceTestBase {
                 ImmutableList.of("en"),
                 ImmutableList.of("hdpi"),
                 FilterBehavior.FILTER_IN_ANALYSIS,
-                true,
-                1))
+                true))
         .isNull();
   }
 
   @Test
-  public void testGetTopLevelTransitionMultipleTargets() throws Exception {
-    assertThat(
-            getTopLevelTransition(
-                ImmutableList.of("en"),
-                ImmutableList.of("hdpi"),
-                FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION,
-                true,
-                2))
-        .isSameAs(ResourceFilter.REMOVE_DYNAMICALLY_CONFIGURED_RESOURCE_FILTERING_TRANSITION);
-  }
-
-  @Test
   public void testGetTopLevelTransitionNotBinary() throws Exception {
     assertThat(
             getTopLevelTransition(
                 ImmutableList.of("en"),
                 ImmutableList.of("hdpi"),
                 FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION,
-                false,
-                1))
+                false))
         .isSameAs(ResourceFilter.REMOVE_DYNAMICALLY_CONFIGURED_RESOURCE_FILTERING_TRANSITION);
   }
 
@@ -617,8 +602,7 @@ public class ResourceFilterTest extends ResourceTestBase {
                 ImmutableList.<String>of(),
                 ImmutableList.<String>of(),
                 FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION,
-                true,
-                1))
+                true))
         .isSameAs(ResourceFilter.REMOVE_DYNAMICALLY_CONFIGURED_RESOURCE_FILTERING_TRANSITION);
   }
 
@@ -631,8 +615,7 @@ public class ResourceFilterTest extends ResourceTestBase {
             resourceConfigurationFilters,
             densities,
             FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION,
-            true,
-            1);
+            true);
 
     assertThat(transition).isInstanceOf(AddDynamicallyConfiguredResourceFilteringTransition.class);
 
@@ -655,13 +638,11 @@ public class ResourceFilterTest extends ResourceTestBase {
       ImmutableList<String> resourceConfigurationFilters,
       ImmutableList<String> densities,
       FilterBehavior behavior,
-      boolean isBinary,
-      int topLevelTargetCount)
+      boolean isBinary)
       throws Exception {
     AttributeMap attrs = getAttributeMap(resourceConfigurationFilters, densities);
     return makeResourceFilter("", "", behavior)
-        .getTopLevelPatchTransition(
-            isBinary ? "android_binary" : "android_library", topLevelTargetCount, attrs);
+        .getTopLevelPatchTransition(isBinary ? "android_binary" : "android_library", attrs);
   }
 
   @Test