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: 150602545
MOS_MIGRATED_REVID=150602545

6 files changed, 64 insertions, 196 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 516db565c0..7c340694cb 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,18 +99,17 @@ 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_LOCAL_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_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 c70b10780a..f194aa559c 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
@@ -90,26 +90,23 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
     if (definesLocalResources) {
       ApplicationManifest applicationManifest = androidSemantics.getManifestForRule(ruleContext)
           .renamePackage(ruleContext, AndroidCommon.getJavaPackage(ruleContext));
-      resourceApk =
-          applicationManifest.packWithDataAndResources(
-              null, /* resourceApk -- not needed for library */
-              ruleContext,
-              true, /* isLibrary */
-              ResourceDependencies.fromRuleDeps(ruleContext, JavaCommon.isNeverLink(ruleContext)),
-              ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
-              ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_LOCAL_SYMBOLS),
-              ResourceConfigurationFilter.empty(ruleContext),
-              ImmutableList.<String>of(), /* uncompressedExtensions */
-              false, /* crunchPng */
-              ImmutableList.<String>of(), /* densities */
-              false, /* incremental */
-              null, /* proguardCfgOut */
-              null, /* mainDexProguardCfg */
-              ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_PROCESSED_MANIFEST),
-              ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_ZIP),
-              DataBinding.isEnabled(ruleContext)
-                  ? DataBinding.getLayoutInfoFile(ruleContext)
-                  : null);
+      resourceApk = applicationManifest.packWithDataAndResources(
+          null, /* resourceApk -- not needed for library */
+          ruleContext,
+          true, /* isLibrary */
+          ResourceDependencies.fromRuleDeps(ruleContext, JavaCommon.isNeverLink(ruleContext)),
+          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
+          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_SYMBOLS),
+          ResourceConfigurationFilter.empty(ruleContext),
+          ImmutableList.<String>of(), /* uncompressedExtensions */
+          false, /* crunchPng */
+          ImmutableList.<String>of(), /* densities */
+          false, /* incremental */
+          null, /* proguardCfgOut */
+          null, /* mainDexProguardCfg */
+          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_PROCESSED_MANIFEST),
+          ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_ZIP),
+          DataBinding.isEnabled(ruleContext) ? DataBinding.getLayoutInfoFile(ruleContext) : null);
       if (ruleContext.hasErrors()) {
         return null;
       }
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 dc84ee5dc5..3d7886d368 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,11 +144,10 @@ public class AndroidResourceMergingActionBuilder {
     ResourceContainerConverter.convertDependencies(
         dependencies, builder, inputs, RESOURCE_CONTAINER_TO_ARG, RESOURCE_CONTAINER_TO_ARTIFACTS);
 
+    Preconditions.checkNotNull(classJarOut);
     List<Artifact> outs = new ArrayList<>();
-    if (classJarOut != null) {
-      builder.addExecPath("--classJarOutput", classJarOut);
-      outs.add(classJarOut);
-    }
+    builder.addExecPath("--classJarOutput", classJarOut);
+    outs.add(classJarOut);
 
     if (mergedResourcesOut != null) {
       builder.addExecPath("--resourcesOutput", mergedResourcesOut);
@@ -188,11 +187,8 @@ public class AndroidResourceMergingActionBuilder {
             .build(context));
 
     // Return the full set of processed transitive dependencies.
-    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);
-    }
+    ResourceContainer.Builder result = primary.toBuilder()
+        .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 8ab8794eb9..36eb19c3aa 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_MERGED_SYMBOLS =
-      fromTemplates("%{name}_symbols/merged.bin");
+  public static final SafeImplicitOutputsFunction ANDROID_SYMBOLS =
+      fromTemplates("%{name}_symbols/symbols.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 5f08ee6c88..4467c823c7 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
@@ -453,43 +453,38 @@ 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);
-
-      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);
+      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);
+      }
 
-      ResourceContainer merged =
+      AndroidResourceMergingActionBuilder resourcesMergerBuilder =
           new AndroidResourceMergingActionBuilder(ruleContext)
-              .setJavaPackage(generated.getJavaPackage())
-              .withPrimary(generated)
+              .setJavaPackage(resourceContainer.getJavaPackage())
+              .withPrimary(resourceContainer)
               .withDependencies(resourceDeps)
               .setMergedResourcesOut(mergedResources)
               .setManifestOut(manifestOut)
-              .setDataBindingInfoZip(dataBindingInfoZip)
-              .build(ruleContext);
+              .setClassJarOut(rJavaClassJar)
+              .setDataBindingInfoZip(dataBindingInfoZip);
+      ResourceContainer merged = resourcesMergerBuilder.build(ruleContext);
 
-      processed =
+      AndroidResourceValidatorActionBuilder validatorBuilder =
           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())
-              .build(ruleContext);
+              .setRTxtOut(merged.getRTxt());
+      processed = validatorBuilder.build(ruleContext);
     } else {
       AndroidResourcesProcessorBuilder builder =
           new AndroidResourcesProcessorBuilder(ruleContext)
@@ -635,16 +630,14 @@ 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
deleted file mode 100644
index 361a70a803..0000000000
--- a/src/main/java/com/google/devtools/build/lib/rules/android/LibraryRGeneratorActionBuilder.java
+++ /dev/null
@@ -1,117 +0,0 @@
-// 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();
-  }
-}