Create pipeline for monolithic data processing with decoupled classes

The AndroidResourceProcessingAction does all of asset parsing and merging and
all of resource parsing, merging, and validation except for R class generation,
all in one action. Add class to wrap the intermediate output of this action. It
can trigger R class generation to create a full ValidatedAndroidResources
object.

RELNOTES: none
PiperOrigin-RevId: 193350591

7 files changed, 222 insertions, 51 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataConverter.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataConverter.java
index c6f2d5cc26..de0f2b1d5c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataConverter.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidDataConverter.java
@@ -133,13 +133,7 @@ public class AndroidDataConverter<T> extends ParametrizedMapFn<T> {
     }
 
     Builder<T> withRoots(Function<T, ImmutableList<PathFragment>> rootsFunction) {
-      return with(
-          t ->
-              rootsFunction
-                  .apply(t)
-                  .stream()
-                  .map(PathFragment::toString)
-                  .collect(Collectors.joining("#")));
+      return with(t -> rootsToString(rootsFunction.apply(t)));
     }
 
     Builder<T> withArtifact(Function<T, Artifact> artifactFunction) {
@@ -160,4 +154,8 @@ public class AndroidDataConverter<T> extends ParametrizedMapFn<T> {
       return new AndroidDataConverter<>(inner.build(), joinerType);
     }
   }
+
+  static String rootsToString(ImmutableList<PathFragment> roots) {
+    return roots.stream().map(PathFragment::toString).collect(Collectors.joining("#"));
+  }
 }
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 0be002e9b8..2fe425728f 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
@@ -225,12 +225,11 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
                       AndroidRuleClasses.ANDROID_PROCESSED_MANIFEST))
               .setSourceJarOut(resourceContainer.getJavaSourceJar())
               .setJavaPackage(resourceContainer.getJavaPackage())
-              .withPrimary(resourceContainer)
               .withResourceDependencies(resourceApk.getResourceDependencies())
               .setDebug(ruleContext.getConfiguration().getCompilationMode() != CompilationMode.OPT)
               .setThrowOnResourceConflict(
                   ruleContext.getFragment(AndroidConfiguration.class).throwOnResourceConflict())
-              .build(ruleContext);
+              .build(resourceContainer);
     }
 
     new AarGeneratorBuilder(ruleContext)
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 b418cbc832..2747f304f2 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
@@ -19,7 +19,6 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ParamFileInfo;
 import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
 import com.google.devtools.build.lib.analysis.RuleContext;
-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.CustomCommandLine.Builder;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg;
@@ -56,13 +55,6 @@ public class AndroidResourcesProcessorBuilder {
           .withSeparator(ToArg.SeparatorType.COLON_COMMA)
           .toArgConverter();
 
-  private static final ResourceContainerConverter.ToArg RESOURCE_CONTAINER_TO_ARG =
-      ResourceContainerConverter.builder()
-          .include(Includes.ResourceRoots)
-          .include(Includes.Manifest)
-          .withSeparator(ToArg.SeparatorType.COLON_COMMA)
-          .toArgConverter();
-
   private static final ResourceContainerConverter.ToArg RESOURCE_DEP_TO_ARG =
       ResourceContainerConverter.builder()
           .include(Includes.ResourceRoots)
@@ -72,8 +64,6 @@ public class AndroidResourcesProcessorBuilder {
           .withSeparator(ToArg.SeparatorType.COLON_COMMA)
           .toArgConverter();
 
-  private ResourceContainer primary;
-
   private ResourceDependencies resourceDependencies;
   private AssetDependencies assetDependencies;
 
@@ -116,15 +106,6 @@ public class AndroidResourcesProcessorBuilder {
   }
 
   /**
-   * The primary resource for merging. This resource will overwrite any resource or data value in
-   * the transitive closure.
-   */
-  public AndroidResourcesProcessorBuilder withPrimary(ResourceContainer primary) {
-    this.primary = primary;
-    return this;
-  }
-
-  /**
    * The output zip for resource-processed data binding expressions (i.e. a zip of .xml files).
    *
    * <p>If null, data binding processing is skipped (and data binding expressions aren't allowed in
@@ -239,12 +220,11 @@ public class AndroidResourcesProcessorBuilder {
     return this;
   }
 
-  public ResourceContainer build(ActionConstructionContext context) {
-    if (aaptVersion == AndroidAaptVersion.AAPT2) {
-      createAapt2ApkAction(context);
-    } else {
-      createAaptAction(context);
-    }
+  public ResourceContainer build(ResourceContainer primary) {
+    build(
+        primary.getAndroidResources(),
+        primary.getAndroidAssets(),
+        ProcessedAndroidManifest.from(primary));
 
     ResourceContainer.Builder builder =
         primary.toBuilder().setJavaSourceJar(sourceJarOut).setRTxt(rTxtOut).setSymbols(symbols);
@@ -263,6 +243,50 @@ public class AndroidResourcesProcessorBuilder {
     return builder.build();
   }
 
+  public ProcessedAndroidData build(
+      AndroidResources primaryResources,
+      AndroidAssets primaryAssets,
+      StampedAndroidManifest primaryManifest) {
+
+    if (aaptVersion == AndroidAaptVersion.AAPT2) {
+      createAapt2ApkAction(primaryResources, primaryAssets, primaryManifest);
+    } else {
+      createAaptAction(primaryResources, primaryAssets, primaryManifest);
+    }
+
+    // Wrap the new manifest, if any
+    ProcessedAndroidManifest processedManifest =
+        new ProcessedAndroidManifest(
+            manifestOut == null ? primaryManifest.getManifest() : manifestOut,
+            primaryManifest.getPackage(),
+            primaryManifest.isExported());
+
+    // Wrap the parsed resources
+    ParsedAndroidResources parsedResources =
+        ParsedAndroidResources.of(
+            primaryResources,
+            symbols,
+            /* compiledSymbols = */ null,
+            ruleContext.getLabel(),
+            processedManifest);
+
+    // Wrap the parsed and merged assets
+    ParsedAndroidAssets parsedAssets =
+        ParsedAndroidAssets.of(primaryAssets, symbols, ruleContext.getLabel());
+    MergedAndroidAssets mergedAssets =
+        MergedAndroidAssets.of(parsedAssets, mergedResourcesOut, assetDependencies);
+
+    return ProcessedAndroidData.of(
+        parsedResources,
+        mergedAssets,
+        processedManifest,
+        rTxtOut,
+        sourceJarOut,
+        apkOut,
+        dataBindingInfoZip,
+        resourceDependencies);
+  }
+
   public AndroidResourcesProcessorBuilder setJavaPackage(String customJavaPackage) {
     this.customJavaPackage = customJavaPackage;
     return this;
@@ -301,7 +325,10 @@ public class AndroidResourcesProcessorBuilder {
     return this;
   }
 
-  private void createAapt2ApkAction(ActionConstructionContext context) {
+  private void createAapt2ApkAction(
+      AndroidResources primaryResources,
+      AndroidAssets primaryAssets,
+      StampedAndroidManifest primaryManifest) {
     List<Artifact> outs = new ArrayList<>();
     // TODO(corysmith): Convert to an immutable list builder, as there is no benefit to a NestedSet
     // here, as it will already have been flattened.
@@ -341,7 +368,7 @@ public class AndroidResourcesProcessorBuilder {
       builder.add("--conditionalKeepRules");
     }
 
-    configureCommonFlags(outs, inputs, builder);
+    configureCommonFlags(primaryResources, primaryAssets, primaryManifest, outs, inputs, builder);
 
     ParamFileInfo.Builder paramFileInfo = ParamFileInfo.builder(ParameterFileType.SHELL_QUOTED);
     // Some flags (e.g. --mainData) may specify lists (or lists of lists) separated by special
@@ -366,10 +393,13 @@ public class AndroidResourcesProcessorBuilder {
                 ruleContext.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST))
             .setProgressMessage("Processing Android resources for %s", ruleContext.getLabel())
             .setMnemonic("AndroidAapt2")
-            .build(context));
+            .build(ruleContext));
   }
 
-  private void createAaptAction(ActionConstructionContext context) {
+  private void createAaptAction(
+      AndroidResources primaryResources,
+      AndroidAssets primaryAssets,
+      StampedAndroidManifest primaryManifest) {
     List<Artifact> outs = new ArrayList<>();
     // TODO(corysmith): Convert to an immutable list builder, as there is no benefit to a NestedSet
     // here, as it will already have been flattened.
@@ -393,7 +423,7 @@ public class AndroidResourcesProcessorBuilder {
     addAssetDeps(builder, inputs);
 
     builder.addExecPath("--aapt", sdk.getAapt().getExecutable());
-    configureCommonFlags(outs, inputs, builder);
+    configureCommonFlags(primaryResources, primaryAssets, primaryManifest, outs, inputs, builder);
 
     ImmutableList<String> filteredResources =
         resourceFilterFactory.getResourcesToIgnoreInExecution();
@@ -424,7 +454,7 @@ public class AndroidResourcesProcessorBuilder {
                 ruleContext.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST))
             .setProgressMessage("Processing Android resources for %s", ruleContext.getLabel())
             .setMnemonic("AaptPackage")
-            .build(context));
+            .build(ruleContext));
   }
 
   private void addAssetDeps(CustomCommandLine.Builder builder, NestedSetBuilder<Artifact> inputs) {
@@ -447,12 +477,23 @@ public class AndroidResourcesProcessorBuilder {
   }
 
   private void configureCommonFlags(
-      List<Artifact> outs, NestedSetBuilder<Artifact> inputs, Builder builder) {
+      AndroidResources primaryResources,
+      AndroidAssets primaryAssets,
+      StampedAndroidManifest primaryManifest,
+      List<Artifact> outs,
+      NestedSetBuilder<Artifact> inputs,
+      Builder builder) {
 
     // Add data
-    builder.add("--primaryData", RESOURCE_CONTAINER_TO_ARG.map(primary));
-    inputs.addAll(primary.getArtifacts());
-    inputs.add(primary.getManifest());
+    builder.add(
+        "--primaryData",
+        String.format(
+            "%s:%s:%s",
+            AndroidDataConverter.rootsToString(primaryResources.getResourceRoots()),
+            AndroidDataConverter.rootsToString(primaryAssets.getAssetRoots()),
+            primaryManifest.getManifest().getExecPathString()));
+    inputs.addAll(primaryResources.getResources());
+    inputs.add(primaryManifest.getManifest());
 
     if (!Strings.isNullOrEmpty(sdk.getBuildToolsVersion())) {
       builder.add("--buildToolsVersion", sdk.getBuildToolsVersion());
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 8b6775b44b..3bf61dbbec 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
@@ -325,7 +325,6 @@ public final class ApplicationManifest {
             .setCrunchPng(true)
             .setJavaPackage(resourceContainer.getJavaPackage())
             .setDebug(ruleContext.getConfiguration().getCompilationMode() != CompilationMode.OPT)
-            .withPrimary(resourceContainer)
             .withResourceDependencies(resourceDeps)
             .setProguardOut(proguardCfg)
             .setMainDexProguardOut(mainDexProguardCfg)
@@ -346,7 +345,7 @@ public final class ApplicationManifest {
           .setSymbols(resourceContainer.getSymbols())
           .setSourceJarOut(resourceContainer.getJavaSourceJar());
     }
-    ResourceContainer processed = builder.build(ruleContext);
+    ResourceContainer processed = builder.build(resourceContainer);
 
     ResourceContainer finalContainer =
         new RClassGeneratorActionBuilder(ruleContext)
@@ -462,7 +461,6 @@ public final class ApplicationManifest {
             .setCrunchPng(crunchPng)
             .setJavaPackage(resourceContainer.getJavaPackage())
             .setDebug(ruleContext.getConfiguration().getCompilationMode() != CompilationMode.OPT)
-            .withPrimary(resourceContainer)
             .withResourceDependencies(resourceDeps)
             .setProguardOut(proguardCfg)
             .setApplicationId(manifestValues.get("applicationId"))
@@ -474,7 +472,7 @@ public final class ApplicationManifest {
                     .getFragment(AndroidConfiguration.class)
                     .throwOnResourceConflict())
             .setPackageUnderTest(null)
-            .build(ruleContext);
+            .build(resourceContainer);
 
     // Intentionally skip building an R class JAR - incremental binaries handle this separately.
 
@@ -540,7 +538,6 @@ public final class ApplicationManifest {
             .setDebug(ruleContext.getConfiguration().getCompilationMode() != CompilationMode.OPT)
             .setManifestOut(manifestOut)
             .setMergedResourcesOut(mergedResources)
-            .withPrimary(resourceContainer)
             .withResourceDependencies(resourceDeps)
             .setProguardOut(proguardCfg)
             .setMainDexProguardOut(mainDexProguardCfg)
@@ -557,7 +554,7 @@ public final class ApplicationManifest {
             .setRTxtOut(resourceContainer.getRTxt())
             .setSymbols(resourceContainer.getSymbols())
             .setSourceJarOut(resourceContainer.getJavaSourceJar())
-            .build(ruleContext);
+            .build(resourceContainer);
 
     ResourceContainer finalContainer =
         new RClassGeneratorActionBuilder(ruleContext)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidAssets.java b/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidAssets.java
index ccffbc3d25..fcda8e5912 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidAssets.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/MergedAndroidAssets.java
@@ -63,7 +63,12 @@ public class MergedAndroidAssets extends ParsedAndroidAssets {
         .addTransitiveInputValues(deps.getTransitiveSymbols())
         .buildAndRegister("Merging Android assets", "AndroidAssetMerger");
 
-    return new MergedAndroidAssets(parsed, mergedAssets, deps);
+    return of(parsed, mergedAssets, deps);
+  }
+
+  static MergedAndroidAssets of(
+      ParsedAndroidAssets parsed, Artifact mergedAssets, AssetDependencies assetDependencies) {
+    return new MergedAndroidAssets(parsed, mergedAssets, assetDependencies);
   }
 
   private MergedAndroidAssets(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java b/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java
new file mode 100644
index 0000000000..f7f86f9b75
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ProcessedAndroidData.java
@@ -0,0 +1,125 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.rules.android;
+
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
+import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion;
+import javax.annotation.Nullable;
+
+/**
+ * Processed Android data (assets, resources, and manifest) returned from resource processing.
+ *
+ * <p>In libraries, data is parsed, merged, and then validated. For top-level targets like
+ * android_binary, however, most of these steps happen in a single monolithic action. The only thing
+ * the monolithic action doesn't do is generate an R.class file for the resources. When combined
+ * with such a file, this object should contain the same data as the results of the individual
+ * actions.
+ *
+ * <p>In general, the individual actions should be used, as they are decoupled and designed to allow
+ * parallelized processing of a dependency tree of android_library targets. The monolithic action
+ * should only be used as part of building the data into a final APK that can become part of a
+ * produce android_binary or other top-level APK.
+ */
+public class ProcessedAndroidData {
+  private final ParsedAndroidResources resources;
+  private final MergedAndroidAssets assets;
+  private final ProcessedAndroidManifest manifest;
+  private final Artifact rTxt;
+  private final Artifact sourceJar;
+  private final Artifact apk;
+  @Nullable private final Artifact dataBindingInfoZip;
+  private final ResourceDependencies resourceDeps;
+
+  static ProcessedAndroidData of(
+      ParsedAndroidResources resources,
+      MergedAndroidAssets assets,
+      ProcessedAndroidManifest manifest,
+      Artifact rTxt,
+      Artifact sourceJar,
+      Artifact apk,
+      @Nullable Artifact dataBindingInfoZip,
+      ResourceDependencies resourceDeps) {
+    return new ProcessedAndroidData(
+        resources, assets, manifest, rTxt, sourceJar, apk, dataBindingInfoZip, resourceDeps);
+  }
+
+  private ProcessedAndroidData(
+      ParsedAndroidResources resources,
+      MergedAndroidAssets assets,
+      ProcessedAndroidManifest manifest,
+      Artifact rTxt,
+      Artifact sourceJar,
+      Artifact apk,
+      @Nullable Artifact dataBindingInfoZip,
+      ResourceDependencies resourceDeps) {
+    this.resources = resources;
+    this.assets = assets;
+    this.manifest = manifest;
+    this.rTxt = rTxt;
+    this.sourceJar = sourceJar;
+    this.apk = apk;
+    this.dataBindingInfoZip = dataBindingInfoZip;
+    this.resourceDeps = resourceDeps;
+  }
+
+  /**
+   * Gets the fully processed resources from this class.
+   *
+   * <p>Registers an action to run R class generation, the last step needed in resource processing.
+   * Returns the fully processed resources.
+   */
+  public ValidatedAndroidResources generateRClass(RuleContext ruleContext)
+      throws RuleErrorException, InterruptedException {
+    return new RClassGeneratorActionBuilder(ruleContext)
+        .targetAaptVersion(AndroidAaptVersion.chooseTargetAaptVersion(ruleContext))
+        .withDependencies(resourceDeps)
+        .setClassJarOut(
+            ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR))
+        .build(this);
+  }
+
+  /**
+   * Returns fully processed resources. The R class generator action will not be registered.
+   *
+   * @param rClassJar an artifact containing the resource class jar for these resources. An action
+   *     to generate it must be registered elsewhere.
+   */
+  ValidatedAndroidResources toValidatedResources(Artifact rClassJar) {
+    // When assets and resources are processed together, they are both merged into the same zip
+    Artifact mergedResources = assets.getMergedAssets();
+
+    // Since parts of both merging and validation were already done in combined resource processing,
+    // we need to build containers for both here.
+    MergedAndroidResources merged =
+        MergedAndroidResources.of(
+            resources, mergedResources, rClassJar, dataBindingInfoZip, resourceDeps, manifest);
+
+    // Combined resource processing does not produce aapt2 artifacts; they're nulled out
+    return ValidatedAndroidResources.of(merged, rTxt, sourceJar, apk, null, null, null);
+  }
+
+  public MergedAndroidAssets getAssets() {
+    return assets;
+  }
+
+  public ProcessedAndroidManifest getManifest() {
+    return manifest;
+  }
+
+  public Artifact getRTxt() {
+    return rTxt;
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
index 975fbd5be6..42d30d4867 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/RClassGeneratorActionBuilder.java
@@ -70,6 +70,12 @@ public class RClassGeneratorActionBuilder {
     return primary.toBuilder().setJavaClassJar(classJarOut).build();
   }
 
+  public ValidatedAndroidResources build(ProcessedAndroidData data) {
+    build(data.getRTxt(), data.getManifest());
+
+    return data.toValidatedResources(classJarOut);
+  }
+
   private void build(Artifact rTxt, ProcessedAndroidManifest manifest) {
     CustomCommandLine.Builder builder = new CustomCommandLine.Builder();