Pass resources and assets seperately into parse and compile actions

This will eventually allow us to parse or compile assets and resources
separately.

Also, remove a no-op null check that was making the code confusing, and some
unneeded resource filtering that was doing the same.

RELNOTES: none
PiperOrigin-RevId: 190951667

2 files changed, 83 insertions, 98 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceParsingActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceParsingActionBuilder.java
index 67076096b7..1b7125be7d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceParsingActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceParsingActionBuilder.java
@@ -15,7 +15,6 @@ package com.google.devtools.build.lib.rules.android;
 
 import static java.util.stream.Collectors.joining;
 
-import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
@@ -28,7 +27,6 @@ import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
-import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -39,15 +37,15 @@ import javax.annotation.Nullable;
 /** Builder for creating $android_resource_parser action. */
 public class AndroidResourceParsingActionBuilder {
 
-  private static final ResourceContainerToArtifacts RESOURCE_CONTAINER_TO_ARTIFACTS =
-      new ResourceContainerToArtifacts();
-
-  private static final ResourceContainerToArg RESOURCE_CONTAINER_TO_ARG =
-      new ResourceContainerToArg();
-
   private final RuleContext ruleContext;
   private final AndroidSdkProvider sdk;
-  private ResourceContainer primary;
+
+  // These are only needed when parsing resources with data binding
+  @Nullable private Artifact manifest;
+  @Nullable private String javaPackage;
+
+  private AndroidResources resources = AndroidResources.empty();
+  private AndroidAssets assets = AndroidAssets.empty();
   private Artifact output;
 
   private Artifact compiledSymbols;
@@ -65,54 +63,44 @@ public class AndroidResourceParsingActionBuilder {
     return this;
   }
 
-  /** Set the primary resources. */
-  public AndroidResourceParsingActionBuilder setPrimary(ResourceContainer primary) {
-    this.primary = primary;
+  /** Sets the manifest. Will be ignored except when parsing resources with data binding. */
+  public AndroidResourceParsingActionBuilder setManifest(@Nullable Artifact manifest) {
+    this.manifest = manifest;
     return this;
   }
 
-  public AndroidResourceParsingActionBuilder setCompiledSymbolsOutput(
-      @Nullable Artifact compiledSymbols) {
-    this.compiledSymbols = compiledSymbols;
+  /** Sets the Java package. Will be ignored except when parsing resources with data binding. */
+  public AndroidResourceParsingActionBuilder setJavaPackage(@Nullable String javaPackage) {
+    this.javaPackage = javaPackage;
     return this;
   }
 
-  public AndroidResourceParsingActionBuilder setDataBindingInfoZip(Artifact dataBindingInfoZip) {
-    this.dataBindingInfoZip = dataBindingInfoZip;
+  public AndroidResourceParsingActionBuilder setResources(AndroidResources resources) {
+    this.resources = resources;
     return this;
   }
 
-  private static class ResourceContainerToArg implements Function<ResourceContainer, String> {
-
-    public ResourceContainerToArg() {}
-
-    @Override
-    public String apply(ResourceContainer container) {
-      return convertRoots(container.getResources().getResourceRoots())
-          + ":"
-          + convertRoots(container.getAssets().getAssetRoots());
-    }
+  public AndroidResourceParsingActionBuilder setAssets(AndroidAssets assets) {
+    this.assets = assets;
+    return this;
   }
 
-  private static class ResourceContainerToArtifacts
-      implements Function<ResourceContainer, NestedSet<Artifact>> {
-
-    public ResourceContainerToArtifacts() {}
+  public AndroidResourceParsingActionBuilder setCompiledSymbolsOutput(
+      @Nullable Artifact compiledSymbols) {
+    this.compiledSymbols = compiledSymbols;
+    return this;
+  }
 
-    @Override
-    public NestedSet<Artifact> apply(ResourceContainer container) {
-      NestedSetBuilder<Artifact> artifacts = NestedSetBuilder.naiveLinkOrder();
-      artifacts.addAll(container.getAssets().getAssets());
-      artifacts.addAll(container.getResources().getResources());
-      return artifacts.build();
-    }
+  public AndroidResourceParsingActionBuilder setDataBindingInfoZip(Artifact dataBindingInfoZip) {
+    this.dataBindingInfoZip = dataBindingInfoZip;
+    return this;
   }
 
   private static String convertRoots(Iterable<PathFragment> roots) {
     return Streams.stream(roots).map(Object::toString).collect(joining("#"));
   }
 
-  public ResourceContainer build(ActionConstructionContext context) {
+  private void build(ActionConstructionContext context) {
     CustomCommandLine.Builder builder = new CustomCommandLine.Builder();
 
     // Set the busybox tool.
@@ -120,10 +108,14 @@ public class AndroidResourceParsingActionBuilder {
 
     NestedSetBuilder<Artifact> inputs = NestedSetBuilder.naiveLinkOrder();
 
-    Preconditions.checkNotNull(primary);
-    String resourceDirectories = RESOURCE_CONTAINER_TO_ARG.apply(primary);
+    String resourceDirectories =
+        convertRoots(resources.getResourceRoots()) + ":" + convertRoots(assets.getAssetRoots());
     builder.add("--primaryData", resourceDirectories);
-    inputs.addTransitive(RESOURCE_CONTAINER_TO_ARTIFACTS.apply(primary));
+    inputs.addTransitive(
+        NestedSetBuilder.<Artifact>naiveLinkOrder()
+            .addAll(assets.getAssets())
+            .addAll(resources.getResources())
+            .build());
 
     Preconditions.checkNotNull(output);
     builder.addExecPath("--output", output);
@@ -168,10 +160,10 @@ public class AndroidResourceParsingActionBuilder {
 
       // The databinding needs to be processed before compilation, so the stripping happens here.
       if (dataBindingInfoZip != null) {
-        flatFileBuilder.addExecPath("--manifest", primary.getManifest());
-        inputs.add(primary.getManifest());
-        if (!Strings.isNullOrEmpty(primary.getJavaPackage())) {
-          flatFileBuilder.add("--packagePath", primary.getJavaPackage());
+        flatFileBuilder.addExecPath("--manifest", manifest);
+        inputs.add(manifest);
+        if (!Strings.isNullOrEmpty(javaPackage)) {
+          flatFileBuilder.add("--packagePath", javaPackage);
         }
         flatFileBuilder.addExecPath("--dataBindingInfoOut", dataBindingInfoZip);
         outs.add(dataBindingInfoZip);
@@ -188,9 +180,24 @@ public class AndroidResourceParsingActionBuilder {
               .setProgressMessage("Compiling Android resources for %s", ruleContext.getLabel())
               .setMnemonic("AndroidResourceCompiler")
               .build(context));
-      return primary.toBuilder().setCompiledSymbols(compiledSymbols).setSymbols(output).build();
-    } else {
-      return primary.toBuilder().setSymbols(output).build();
     }
   }
+
+  /**
+   * Builds and registers the action, and updates the given resourceContainer with the output
+   * symbols.
+   */
+  public ResourceContainer buildAndUpdate(
+      RuleContext ruleContext, ResourceContainer resourceContainer) {
+    build(ruleContext);
+
+    ResourceContainer.Builder builder =
+        resourceContainer.toBuilder().setSymbols(output).setAssets(assets).setResources(resources);
+
+    if (compiledSymbols != null) {
+      builder.setCompiledSymbols(compiledSymbols);
+    }
+
+    return builder.build();
+  }
 }
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 0b22c36e5d..c4f098ac6b 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
@@ -356,41 +356,25 @@ public final class ApplicationManifest {
       Artifact symbols,
       Artifact manifestOut,
       Artifact mergedResources)
-      throws InterruptedException, RuleErrorException {
-    // Filter the resources during analysis to prevent processing of dependencies on unwanted
-    // resources during execution.
-    ResourceFilter resourceFilter =
-        ResourceFilterFactory.fromRuleContext(ruleContext)
-            .getResourceFilter(ruleContext, resourceDeps, resources);
-    resources = resources.filterLocalResources(resourceFilter);
-    resourceDeps = resourceDeps.filter(resourceFilter);
-
-    // Now that the LocalResourceContainer has been filtered, we can build a filtered resource
-    // container from it.
+      throws InterruptedException {
     ResourceContainer resourceContainer =
         ResourceContainer.builderFromRule(ruleContext)
             .setRTxt(rTxt)
-            .setSymbols(symbols)
             .setJavaPackageFrom(JavaPackageSource.MANIFEST)
             .setManifestExported(true)
             .setManifest(getManifest())
-            .setAssets(assets)
-            .setResources(resources)
             .build();
 
     // android_library should only build the APK one way (!incremental).
     Artifact rJavaClassJar =
         ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR);
 
-    if (resourceContainer.getSymbols() != null) {
-      AndroidResourceParsingActionBuilder parsingBuilder =
-          new AndroidResourceParsingActionBuilder(ruleContext)
-              .setPrimary(resourceContainer)
-              .setOutput(resourceContainer.getSymbols())
-              .setCompiledSymbolsOutput(resourceContainer.getCompiledSymbols());
-
-      resourceContainer = parsingBuilder.build(ruleContext);
-    }
+    resourceContainer =
+        new AndroidResourceParsingActionBuilder(ruleContext)
+            .setAssets(assets)
+            .setResources(resources)
+            .setOutput(symbols)
+            .buildAndUpdate(ruleContext, resourceContainer);
 
     ResourceContainer merged =
         new AndroidResourceMergingActionBuilder(ruleContext)
@@ -595,21 +579,13 @@ public final class ApplicationManifest {
       Artifact symbols,
       Artifact manifestOut,
       Artifact mergedResources,
-      Artifact dataBindingInfoZip)
+      @Nullable Artifact dataBindingInfoZip)
       throws InterruptedException, RuleErrorException {
-    // Filter the resources during analysis to prevent processing of dependencies on unwanted
-    // resources during execution.
     AndroidResources resources = AndroidResources.from(ruleContext, "resource_files");
-    ResourceFilter resourceFilter =
-        ResourceFilterFactory.fromRuleContext(ruleContext)
-            .getResourceFilter(ruleContext, resourceDeps, resources);
-    resources = resources.filterLocalResources(resourceFilter);
-    resourceDeps = resourceDeps.filter(resourceFilter);
+    AndroidAssets assets = AndroidAssets.from(ruleContext);
 
     ResourceContainer.Builder builder =
         ResourceContainer.builderFromRule(ruleContext)
-            .setAssets(AndroidAssets.from(ruleContext))
-            .setResources(resources)
             .setManifest(getManifest())
             .setSymbols(symbols)
             .setRTxt(rTxt)
@@ -644,25 +620,27 @@ public final class ApplicationManifest {
     boolean skipParsingAction =
         targetAaptVersion == AndroidAaptVersion.AAPT2 && androidConfiguration.skipParsingAction();
 
-    if (resourceContainer.getSymbols() != null) {
-      AndroidResourceParsingActionBuilder parsingBuilder =
-          new AndroidResourceParsingActionBuilder(ruleContext)
-              .setPrimary(resourceContainer)
-              .setOutput(resourceContainer.getSymbols())
-              .setCompiledSymbolsOutput(resourceContainer.getCompiledSymbols());
+    AndroidResourceParsingActionBuilder parsingBuilder =
+        new AndroidResourceParsingActionBuilder(ruleContext)
+            .setAssets(assets)
+            .setResources(resources)
+            .setOutput(resourceContainer.getSymbols())
+            .setCompiledSymbolsOutput(resourceContainer.getCompiledSymbols());
 
-      if (dataBindingInfoZip != null && resourceContainer.getCompiledSymbols() != null) {
+    if (dataBindingInfoZip != null && resourceContainer.getCompiledSymbols() != null) {
         PathFragment unusedInfo = dataBindingInfoZip.getRootRelativePath();
-        // TODO(corysmith): Centralize the data binding processing and zipping into a single
-        // action. Data binding processing needs to be triggered here as well as the merger to
-        // avoid aapt2 from throwing an error during compilation.
-        parsingBuilder.setDataBindingInfoZip(
-            ruleContext.getDerivedArtifact(
-                unusedInfo.replaceName(unusedInfo.getBaseName() + "_unused.zip"),
-                dataBindingInfoZip.getRoot()));
-      }
-      resourceContainer = parsingBuilder.build(ruleContext);
+      // TODO(corysmith): Centralize the data binding processing and zipping into a single
+      // action. Data binding processing needs to be triggered here as well as the merger to
+      // avoid aapt2 from throwing an error during compilation.
+      parsingBuilder
+          .setDataBindingInfoZip(
+              ruleContext.getDerivedArtifact(
+                  unusedInfo.replaceName(unusedInfo.getBaseName() + "_unused.zip"),
+                  dataBindingInfoZip.getRoot()))
+          .setManifest(resourceContainer.getManifest())
+          .setJavaPackage(resourceContainer.getJavaPackage());
     }
+    resourceContainer = parsingBuilder.buildAndUpdate(ruleContext, resourceContainer);
 
     ResourceContainer merged =
         new AndroidResourceMergingActionBuilder(ruleContext)