Rollback of commit bd40871283a54268945dcb0c47c0326645ffda18.

*** Reason for rollback ***

Rolling forward with the correct changes to the AndroidResourceMergingAction. Tested manually.

*** Original change description ***

Automated [] rollback of commit a58f245a4b40c0ef961b1f30d96b16a9349711c3.

*** Reason for rollback ***

broke over 100k targets, in the depot, see []

*** Original change description ***

Move library R generation to a separate action, ensuring the merging happens
off the java critical path.

--
PiperOrigin-RevId: 151087737
MOS_MIGRATED_REVID=151087737

6 files changed, 178 insertions, 48 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java
index 7c340694cb..516db565c0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AarImport.java
@@ -99,17 +99,18 @@ public class AarImport implements RuleConfiguredTargetFactory {
     Artifact resourcesZip =
         ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_ZIP);
 
-    ResourceApk resourceApk = androidManifest.packWithDataAndResources(
-        ruleContext,
-        new LocalResourceContainer.Builder(ruleContext)
-            .withResources(ImmutableList.of(resourcesProvider))
-            .build(),
-        ResourceDependencies.fromRuleDeps(ruleContext, JavaCommon.isNeverLink(ruleContext)),
-        ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
-        ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_SYMBOLS),
-        ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_PROCESSED_MANIFEST),
-        resourcesZip,
-        /* alwaysExportManifest = */ true);
+    ResourceApk resourceApk =
+        androidManifest.packWithDataAndResources(
+            ruleContext,
+            new LocalResourceContainer.Builder(ruleContext)
+                .withResources(ImmutableList.of(resourcesProvider))
+                .build(),
+            ResourceDependencies.fromRuleDeps(ruleContext, JavaCommon.isNeverLink(ruleContext)),
+            ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
+            ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_LOCAL_SYMBOLS),
+            ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_PROCESSED_MANIFEST),
+            resourcesZip,
+            /* alwaysExportManifest = */ true);
 
     // There isn't really any use case for building an aar_import target on its own, so the files to
     // build could be empty. The resources zip and merged jars are added here as a sanity check for
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 bd57dffab5..f573132b03 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
@@ -96,7 +96,7 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
           true, /* isLibrary */
           ResourceDependencies.fromRuleDeps(ruleContext, JavaCommon.isNeverLink(ruleContext)),
           ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
-          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_SYMBOLS),
+          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_MERGED_SYMBOLS),
           ResourceConfigurationFilter.empty(ruleContext),
           ImmutableList.<String>of(), /* uncompressedExtensions */
           false, /* crunchPng */
@@ -270,3 +270,4 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
     return builder;
   }
 }
+
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
index 3d7886d368..dc84ee5dc5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceMergingActionBuilder.java
@@ -144,10 +144,11 @@ public class AndroidResourceMergingActionBuilder {
     ResourceContainerConverter.convertDependencies(
         dependencies, builder, inputs, RESOURCE_CONTAINER_TO_ARG, RESOURCE_CONTAINER_TO_ARTIFACTS);
 
-    Preconditions.checkNotNull(classJarOut);
     List<Artifact> outs = new ArrayList<>();
-    builder.addExecPath("--classJarOutput", classJarOut);
-    outs.add(classJarOut);
+    if (classJarOut != null) {
+      builder.addExecPath("--classJarOutput", classJarOut);
+      outs.add(classJarOut);
+    }
 
     if (mergedResourcesOut != null) {
       builder.addExecPath("--resourcesOutput", mergedResourcesOut);
@@ -187,8 +188,11 @@ public class AndroidResourceMergingActionBuilder {
             .build(context));
 
     // Return the full set of processed transitive dependencies.
-    ResourceContainer.Builder result = primary.toBuilder()
-        .setJavaClassJar(classJarOut);
+    ResourceContainer.Builder result = primary.toBuilder();
+    if (classJarOut != null) {
+      // ensure the classJar is propgated if it exists. Otherwise, AndroidCommon tries to make it.
+      result.setJavaClassJar(classJarOut);
+    }
     if (manifestOut != null) {
       result.setManifest(manifestOut);
     }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
index 63a25ef0a5..7b345a16af 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
@@ -116,8 +116,8 @@ public final class AndroidRuleClasses {
       fromTemplates("%{name}_symbols/R.txt");
   public static final SafeImplicitOutputsFunction ANDROID_LOCAL_SYMBOLS =
       fromTemplates("%{name}_symbols/local.bin");
-  public static final SafeImplicitOutputsFunction ANDROID_SYMBOLS =
-      fromTemplates("%{name}_symbols/symbols.bin");
+  public static final SafeImplicitOutputsFunction ANDROID_MERGED_SYMBOLS =
+      fromTemplates("%{name}_symbols/merged.bin");
   public static final ImplicitOutputsFunction ANDROID_PROCESSED_MANIFEST =
       fromTemplates("%{name}_processed_manifest/AndroidManifest.xml");
   public static final SafeImplicitOutputsFunction MOBILE_INSTALL_STUB_APPLICATION_MANIFEST =
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 f5fabfc623..32e78d86e4 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
@@ -465,38 +465,43 @@ public final class ApplicationManifest {
     if (isLibrary && AndroidCommon.getAndroidConfig(ruleContext).useParallelResourceProcessing()) {
       // android_library should only build the APK one way (!incremental).
       Preconditions.checkArgument(!incremental);
-      Artifact rJavaClassJar = ruleContext.getImplicitOutputArtifact(
-          AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR);
-
-      if (resourceContainer.getSymbols() != null) {
-        new AndroidResourceParsingActionBuilder(ruleContext)
-            .withPrimary(resourceContainer)
-            .setParse(data)
-            .setOutput(resourceContainer.getSymbols())
-            .build(ruleContext);
-      }
+      Artifact rJavaClassJar =
+          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR);
 
-      AndroidResourceMergingActionBuilder resourcesMergerBuilder =
-          new AndroidResourceMergingActionBuilder(ruleContext)
-              .setJavaPackage(resourceContainer.getJavaPackage())
+      ResourceContainer parsed =
+          new AndroidResourceParsingActionBuilder(ruleContext)
+              .setParse(data)
               .withPrimary(resourceContainer)
+              .setOutput(resourceContainer.getSymbols())
+              .build(ruleContext);
+
+      ResourceContainer generated =
+          new LibraryRGeneratorActionBuilder()
+              .setJavaPackage(resourceContainer.getJavaPackage())
+              .withPrimary(parsed)
+              .withDependencies(resourceDeps)
+              .setClassJarOut(rJavaClassJar)
+              .build(ruleContext);
+
+      ResourceContainer merged =
+          new AndroidResourceMergingActionBuilder(ruleContext)
+              .setJavaPackage(generated.getJavaPackage())
+              .withPrimary(generated)
               .withDependencies(resourceDeps)
               .setMergedResourcesOut(mergedResources)
               .setManifestOut(manifestOut)
-              .setClassJarOut(rJavaClassJar)
-              .setDataBindingInfoZip(dataBindingInfoZip);
-      ResourceContainer merged = resourcesMergerBuilder.build(ruleContext);
+              .setDataBindingInfoZip(dataBindingInfoZip)
+              .build(ruleContext);
 
-      AndroidResourceValidatorActionBuilder validatorBuilder =
+      processed =
           new AndroidResourceValidatorActionBuilder(ruleContext)
               .setJavaPackage(merged.getJavaPackage())
-              .setDebug(
-                  ruleContext.getConfiguration().getCompilationMode() != CompilationMode.OPT)
+              .setDebug(ruleContext.getConfiguration().getCompilationMode() != CompilationMode.OPT)
               .setMergedResources(mergedResources)
               .withPrimary(merged)
               .setSourceJarOut(merged.getJavaSourceJar())
-              .setRTxtOut(merged.getRTxt());
-      processed = validatorBuilder.build(ruleContext);
+              .setRTxtOut(merged.getRTxt())
+              .build(ruleContext);
     } else {
       AndroidResourcesProcessorBuilder builder =
           new AndroidResourcesProcessorBuilder(ruleContext)
@@ -644,14 +649,16 @@ public final class ApplicationManifest {
     aaptActionHelper.createGenerateApkAction(resourceApk,
         resourceContainer.getRenameManifestPackage(), additionalAaptOpts.build(), densities);
 
-    ResourceContainer updatedResources = resourceContainer.toBuilder()
-        .setLabel(ruleContext.getLabel())
-        .setApk(resourceApk)
-        .setManifest(getManifest())
-        .setJavaSourceJar(javaSourcesJar)
-        .setJavaClassJar(null)
-        .setSymbols(null)
-        .build();
+    ResourceContainer updatedResources =
+        resourceContainer
+            .toBuilder()
+            .setLabel(ruleContext.getLabel())
+            .setApk(resourceApk)
+            .setManifest(getManifest())
+            .setJavaSourceJar(javaSourcesJar)
+            .setJavaClassJar(null)
+            .setSymbols(null)
+            .build();
 
     aaptActionHelper.createGenerateProguardAction(proguardCfg, mainDexProguardCfg);
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java
new file mode 100644
index 0000000000..361a70a803
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java
@@ -0,0 +1,117 @@
+// Copyright 2017 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.common.base.Function;
+import com.google.common.base.Strings;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
+import com.google.devtools.build.lib.analysis.FilesToRunProvider;
+import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
+import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
+import com.google.devtools.build.lib.analysis.actions.SpawnAction;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.collect.nestedset.Order;
+
+/** Builder for the action that generates the R class for libraries. */
+public class LibraryRGeneratorActionBuilder {
+  static final Function<ResourceContainer, Artifact> TO_SYMBOL_ARTIFACT =
+      new Function<ResourceContainer, Artifact>() {
+        @Override
+        public Artifact apply(ResourceContainer input) {
+          return input.getSymbols();
+        }
+      };
+  static final Function<ResourceContainer, String> TO_SYMBOL_PATH =
+      new Function<ResourceContainer, String>() {
+        @Override
+        public String apply(ResourceContainer container) {
+          return container.getSymbols().getExecPathString();
+        }
+      };
+
+  private String javaPackage;
+  private Iterable<ResourceContainer> deps = ImmutableList.<ResourceContainer>of();
+  private ResourceContainer resourceContainer;
+  private Artifact rJavaClassJar;
+
+  public LibraryRGeneratorActionBuilder setJavaPackage(String javaPackage) {
+    this.javaPackage = javaPackage;
+    return this;
+  }
+
+  public LibraryRGeneratorActionBuilder withPrimary(ResourceContainer resourceContainer) {
+    this.resourceContainer = resourceContainer;
+    return this;
+  }
+
+  public LibraryRGeneratorActionBuilder withDependencies(ResourceDependencies resourceDeps) {
+    this.deps = resourceDeps.getResources();
+    return this;
+  }
+
+  public LibraryRGeneratorActionBuilder setClassJarOut(Artifact rJavaClassJar) {
+    this.rJavaClassJar = rJavaClassJar;
+    return this;
+  }
+
+  public ResourceContainer build(RuleContext ruleContext) {
+    AndroidSdkProvider sdk = AndroidSdkProvider.fromRuleContext(ruleContext);
+
+    CustomCommandLine.Builder builder = new CustomCommandLine.Builder();
+    NestedSetBuilder<Artifact> inputs = NestedSetBuilder.naiveLinkOrder();
+    FilesToRunProvider executable =
+        ruleContext.getExecutablePrerequisite("$android_resources_busybox", Mode.HOST);
+    inputs.addAll(executable.getRunfilesSupport().getRunfilesArtifactsWithoutMiddlemen());
+
+    builder.add("--tool").add("GENERATE_LIBRARY_R").add("--");
+
+    if (!Strings.isNullOrEmpty(javaPackage)) {
+      builder.add("--packageForR").add(javaPackage);
+    }
+
+    FluentIterable<ResourceContainer> symbolProviders =
+        FluentIterable.from(deps).append(resourceContainer);
+
+    builder.addJoinStrings(
+        "--symbols",
+        ruleContext.getConfiguration().getHostPathSeparator(),
+        symbolProviders.transform(TO_SYMBOL_PATH));
+    inputs.addTransitive(
+        NestedSetBuilder.wrap(
+            Order.NAIVE_LINK_ORDER, symbolProviders.transform(TO_SYMBOL_ARTIFACT)));
+
+    builder.addExecPath("--classJarOutput", rJavaClassJar);
+
+    builder.addExecPath("--androidJar", sdk.getAndroidJar());
+    inputs.add(sdk.getAndroidJar());
+
+    // Create the spawn action.
+    SpawnAction.Builder spawnActionBuilder = new SpawnAction.Builder();
+    ruleContext.registerAction(
+        spawnActionBuilder
+            .addTransitiveInputs(inputs.build())
+            .addOutputs(ImmutableList.<Artifact>of(rJavaClassJar))
+            .useParameterFile(ParameterFileType.UNQUOTED)
+            .setCommandLine(builder.build())
+            .setExecutable(executable)
+            .setProgressMessage("Generating Library R Classes: " + ruleContext.getLabel())
+            .setMnemonic("LibraryRClassGenerator")
+            .build(ruleContext));
+    return resourceContainer.toBuilder().setJavaClassJar(rJavaClassJar).build();
+  }
+}