Filter LocalResourceContainer and use it to get a filtered ResourceContainer

Dynamically Configured Resource Filtering change 2/6

We need to filter LocalResourceContainer for dynamically configured resource
filtering - it is an input to resource parsing for android_library targets.
Filter it appropriately.

Once it's being filtered, use it to build a filtered ResourceContainer, rather
than building an unfiltered ResourceContainer and then filtering it. This means
filtering gets done sooner, saving a bit of time and work, and isn't done
twice, which would just be bad.

To do this, we pass an unbuilt ResourceContainer.Builder into createApk, rather than a fully built one. Move the setAssetsAndResourcesFrom and setManifest calls to that builder, called every time, into createApk so that we only call them once.

Also, initialize the ResourceFilter object when it's needed, rather than pass
it down from the binary or library. This makes the code a bit cleaner, plus
means library targets will pick up the dynamically configured information once
that exists.

RELNOTES: none
PiperOrigin-RevId: 162007885

3 files changed, 29 insertions, 51 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
index 6fb4a3c54c..c8ba00d1c8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
@@ -235,7 +235,6 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
               ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_APK),
               resourceDeps,
               ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
-              ResourceFilter.fromRuleContext(ruleContext),
               ruleContext.getTokenizedStringListAttr("nocompress_extensions"),
               ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
               ProguardHelper.getProguardConfigArtifact(ruleContext, ""),
@@ -257,7 +256,6 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
                   ruleContext.getImplicitOutputArtifact(
                       AndroidRuleClasses.ANDROID_INCREMENTAL_RESOURCES_APK),
                   resourceDeps,
-                  ResourceFilter.fromRuleContext(ruleContext),
                   ruleContext.getTokenizedStringListAttr("nocompress_extensions"),
                   ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
                   ProguardHelper.getProguardConfigArtifact(ruleContext, "incremental"));
@@ -270,7 +268,6 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
                   ruleContext,
                   getDxArtifact(ruleContext, "android_instant_run.ap_"),
                   resourceDeps,
-                  ResourceFilter.fromRuleContext(ruleContext),
                   ruleContext.getTokenizedStringListAttr("nocompress_extensions"),
                   ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
                   ProguardHelper.getProguardConfigArtifact(ruleContext, "instant_run"));
@@ -283,7 +280,6 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
                   ruleContext,
                   getDxArtifact(ruleContext, "android_resources.ap_"),
                   resourceDeps,
-                  ResourceFilter.fromRuleContext(ruleContext),
                   ruleContext.getTokenizedStringListAttr("nocompress_extensions"),
                   ruleContext.attributes().get("crunch_png", Type.BOOLEAN),
                   ProguardHelper.getProguardConfigArtifact(ruleContext, "incremental_split"));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
index 83cc88ec6c..5e5454200b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLibrary.java
@@ -98,7 +98,6 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
               ResourceDependencies.fromRuleDeps(ruleContext, JavaCommon.isNeverLink(ruleContext)),
               ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
               ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_MERGED_SYMBOLS),
-              ResourceFilter.empty(ruleContext),
               ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_PROCESSED_MANIFEST),
               ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_ZIP),
               DataBinding.isEnabled(ruleContext)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
index 1ef2406f50..36ef672877 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
@@ -302,8 +302,7 @@ public final class ApplicationManifest {
       ResourceDependencies resourceDeps,
       Artifact rTxt,
       boolean incremental,
-      Artifact proguardCfg)
-      throws InterruptedException {
+      Artifact proguardCfg) throws InterruptedException, RuleErrorException {
     LocalResourceContainer data = new LocalResourceContainer.Builder(ruleContext)
         .withAssets(
             AndroidCommon.getAssetDir(ruleContext),
@@ -317,16 +316,12 @@ public final class ApplicationManifest {
         ruleContext,
         false, /* isLibrary */
         resourceDeps,
-        ResourceFilter.empty(ruleContext),
         ImmutableList.<String>of(), /* uncompressedExtensions */
         true, /* crunchPng */
         incremental,
         ResourceContainer.builderFromRule(ruleContext)
-            .setAssetsAndResourcesFrom(data)
-            .setManifest(getManifest())
             .setRTxt(rTxt)
-            .setApk(resourceApk)
-            .build(),
+            .setApk(resourceApk),
         data,
         proguardCfg,
         null, /* Artifact mainDexProguardCfg */
@@ -346,15 +341,12 @@ public final class ApplicationManifest {
       Artifact symbols,
       Artifact manifestOut,
       Artifact mergedResources,
-      boolean alwaysExportManifest)
-      throws InterruptedException {
+      boolean alwaysExportManifest) throws InterruptedException, RuleErrorException {
     if (ruleContext.hasErrors()) {
       return null;
     }
     ResourceContainer.Builder resourceContainer =
         ResourceContainer.builderFromRule(ruleContext)
-            .setAssetsAndResourcesFrom(data)
-            .setManifest(getManifest())
             .setRTxt(rTxt)
             .setSymbols(symbols)
             .setJavaPackageFrom(JavaPackageSource.MANIFEST);
@@ -365,11 +357,10 @@ public final class ApplicationManifest {
         ruleContext,
         true, /* isLibrary */
         resourceDeps,
-        ResourceFilter.empty(ruleContext),
         ImmutableList.<String>of(), /* List<String> uncompressedExtensions */
         false, /* crunchPng */
         false, /* incremental */
-        resourceContainer.build(),
+        resourceContainer,
         data,
         null, /* Artifact proguardCfg */
         null, /* Artifact mainDexProguardCfg */
@@ -385,7 +376,6 @@ public final class ApplicationManifest {
       RuleContext ruleContext,
       Artifact resourceApk,
       ResourceDependencies resourceDeps,
-      ResourceFilter resourceFilter,
       List<String> uncompressedExtensions,
       boolean crunchPng,
       Artifact proguardCfg)
@@ -407,15 +397,11 @@ public final class ApplicationManifest {
         ruleContext,
         false /* isLibrary */,
         resourceDeps,
-        resourceFilter,
         uncompressedExtensions,
         crunchPng,
         true,
         ResourceContainer.builderFromRule(ruleContext)
-            .setAssetsAndResourcesFrom(data)
-            .setManifest(getManifest())
-            .setApk(resourceApk)
-            .build(),
+            .setApk(resourceApk),
         data,
         proguardCfg,
         null, /* mainDexProguardCfg */
@@ -433,7 +419,6 @@ public final class ApplicationManifest {
       Artifact resourceApk,
       ResourceDependencies resourceDeps,
       Artifact rTxt,
-      ResourceFilter resourceFilter,
       List<String> uncompressedExtensions,
       boolean crunchPng,
       Artifact proguardCfg,
@@ -464,16 +449,12 @@ public final class ApplicationManifest {
         ruleContext,
         false /* isLibrary */,
         resourceDeps,
-        resourceFilter,
         uncompressedExtensions,
         crunchPng,
         false /* incremental */,
         ResourceContainer.builderFromRule(ruleContext)
-            .setAssetsAndResourcesFrom(data)
-            .setManifest(getManifest())
             .setRTxt(rTxt)
-            .setApk(resourceApk)
-            .build(),
+            .setApk(resourceApk),
         data,
         proguardCfg,
         mainDexProguardCfg,
@@ -490,7 +471,6 @@ public final class ApplicationManifest {
       ResourceDependencies resourceDeps,
       Artifact rTxt,
       Artifact symbols,
-      ResourceFilter resourceFilter,
       Artifact manifestOut,
       Artifact mergedResources,
       Artifact dataBindingInfoZip)
@@ -512,17 +492,13 @@ public final class ApplicationManifest {
         ruleContext,
         true /* isLibrary */,
         resourceDeps,
-        resourceFilter,
         ImmutableList.<String>of() /* uncompressedExtensions */,
         false /* crunchPng */,
         false /* incremental */,
         ResourceContainer.builderFromRule(ruleContext)
-            .setAssetsAndResourcesFrom(data)
-            .setManifest(getManifest())
             .setRTxt(rTxt)
             .setSymbols(symbols)
-            .setApk(null)
-            .build(),
+            .setApk(null),
         data,
         null /* proguardCfg */,
         null /* mainDexProguardCfg */,
@@ -537,11 +513,10 @@ public final class ApplicationManifest {
       RuleContext ruleContext,
       boolean isLibrary,
       ResourceDependencies resourceDeps,
-      ResourceFilter resourceFilter,
       List<String> uncompressedExtensions,
       boolean crunchPng,
       boolean incremental,
-      ResourceContainer maybeInlinedResourceContainer,
+      ResourceContainer.Builder maybeInlinedResourceContainerBuilder,
       LocalResourceContainer data,
       Artifact proguardCfg,
       @Nullable Artifact mainDexProguardCfg,
@@ -549,23 +524,31 @@ public final class ApplicationManifest {
       Artifact mergedResources,
       Artifact dataBindingInfoZip,
       @Nullable Artifact featureOf,
-      @Nullable Artifact featureAfter) throws InterruptedException {
-    ResourceContainer resourceContainer = checkForInlinedResources(
-        maybeInlinedResourceContainer,
-        resourceDeps.getResources(),  // TODO(bazel-team): Figure out if we really need to check
-        // the ENTIRE transitive closure, or just the direct dependencies. Given that each rule with
-        // resources would check for inline resources, we can rely on the previous rule to have
-        // checked its dependencies.
-        ruleContext);
-    if (ruleContext.hasErrors()) {
-      return null;
-    }
-
+      @Nullable Artifact featureAfter)
+      throws InterruptedException, RuleErrorException {
     // Filter the resources during analysis to prevent processing of and dependencies on unwanted
     // resources during execution.
-    resourceContainer = resourceContainer.filter(ruleContext, resourceFilter);
+    ResourceFilter resourceFilter = ResourceFilter.fromRuleContext(ruleContext);
+    data = data.filter(ruleContext, resourceFilter);
     resourceDeps = resourceDeps.filter(ruleContext, resourceFilter);
 
+    // Now that the LocalResourceContainer has been filtered, we can build a filtered resource
+    // container from it.
+    ResourceContainer resourceContainer =
+        checkForInlinedResources(
+            maybeInlinedResourceContainerBuilder
+                .setManifest(getManifest())
+                .setAssetsAndResourcesFrom(data)
+                .build(),
+            resourceDeps.getResources(), // TODO(bazel-team): Figure out if we really need to check
+            // the ENTIRE transitive closure, or just the direct dependencies. Given that each rule
+            // with resources would check for inline resources, we can rely on the previous rule to
+            // have checked its dependencies.
+            ruleContext);
+    if (ruleContext.hasErrors()) {
+      return null;
+    }
+
     ResourceContainer processed;
     if (isLibrary && AndroidCommon.getAndroidConfig(ruleContext).useParallelResourceProcessing()) {
       // android_library should only build the APK one way (!incremental).