Automated rollback of commit f384ab5855ba230e7767b7b5350bd72ef01b9a1b.

*** Reason for rollback ***

Memory regression (b/79331026)

*** Original change description ***

Some fixup for R.class files

Expose R.class file in AndroidResourcesInfo, to allow Java compilation.

Include documentation that the R.class should not be used in other targets.

Actually generate the R.class file in the case of an android_library with no
manifest, resources, or assets, rather than compiling it from the R.java

RELNOTES: none
PiperOrigin-RevId: 195659672

9 files changed, 33 insertions, 168 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
index 5ba8f806fb..49dd128c72 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
@@ -398,7 +398,7 @@ public class AndroidCommon {
       JavaCompilationArtifacts.Builder artifactsBuilder,
       JavaTargetAttributes.Builder attributes,
       NestedSetBuilder<Artifact> filesBuilder)
-      throws InterruptedException {
+      throws InterruptedException, RuleErrorException {
 
     // The resource class JAR should already have been generated.
     Preconditions.checkArgument(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
index f63ec0de8b..678a5f67cd 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
@@ -230,12 +230,23 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
         throws RuleErrorException {
       if (ruleContext.isLegalFragment(AndroidConfiguration.class)) {
         boolean hasAapt2 = AndroidSdkProvider.fromRuleContext(ruleContext).getAapt2() != null;
+        AndroidAaptVersion flag =
+            ruleContext.getFragment(AndroidConfiguration.class).getAndroidAaptVersion();
 
         if (ruleContext.getRule().isAttrDefined("aapt_version", STRING)) {
           // On rules that can choose a version, test attribute then flag choose the aapt version
           // target.
-          return chooseTargetAaptVersion(
-              ruleContext, ruleContext.attributes().get("aapt_version", STRING));
+          AndroidAaptVersion version =
+              fromString(ruleContext.attributes().get("aapt_version", STRING));
+          // version is null if the value is "auto"
+          version = version == AndroidAaptVersion.AUTO ? flag : version;
+
+          if (version == AAPT2 && !hasAapt2) {
+            ruleContext.throwWithRuleError(
+                "aapt2 processing requested but not available on the android_sdk");
+            return null;
+          }
+          return version == AndroidAaptVersion.AUTO ? AAPT : version;
         } else {
           // On rules can't choose, assume aapt2 if aapt2 is present in the sdk.
           return hasAapt2 ? AAPT2 : AAPT;
@@ -243,29 +254,6 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
       }
       return null;
     }
-
-    @Nullable
-    public static AndroidAaptVersion chooseTargetAaptVersion(
-        RuleContext ruleContext, @Nullable String versionString) throws RuleErrorException {
-
-      if (ruleContext.isLegalFragment(AndroidConfiguration.class)) {
-        boolean hasAapt2 = AndroidSdkProvider.fromRuleContext(ruleContext).getAapt2() != null;
-        AndroidAaptVersion flag =
-            ruleContext.getFragment(AndroidConfiguration.class).getAndroidAaptVersion();
-
-        AndroidAaptVersion version = fromString(versionString);
-        // version is null if the value is "auto"
-        version = version == AndroidAaptVersion.AUTO ? flag : version;
-
-        if (version == AAPT2 && !hasAapt2) {
-          ruleContext.throwWithRuleError(
-              "aapt2 processing requested but not available on the android_sdk");
-          return null;
-        }
-        return version == AndroidAaptVersion.AUTO ? AAPT : version;
-      }
-      return null;
-    }
   }
 
   /** Android configuration options. */
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 9125bf662e..7587421bb3 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
@@ -27,7 +27,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.TriState;
-import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion;
 import com.google.devtools.build.lib.rules.android.AndroidLibraryAarProvider.Aar;
 import com.google.devtools.build.lib.rules.java.JavaCommon;
 import com.google.devtools.build.lib.rules.java.JavaSemantics;
@@ -188,8 +187,7 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
               ruleContext,
               resourceDeps,
               assetDeps,
-              StampedAndroidManifest.createEmpty(ruleContext, /* exported = */ false),
-              AndroidAaptVersion.chooseTargetAaptVersion(ruleContext));
+              StampedAndroidManifest.createEmpty(ruleContext, /* exported = */ false));
     }
 
     JavaTargetAttributes javaTargetAttributes =
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesInfo.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesInfo.java
index 060b52373c..6f1d665be4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesInfo.java
@@ -53,20 +53,6 @@ public class AndroidResourcesInfo extends NativeInfo {
   private final Artifact rTxt;
 
   /*
-   * An R.class file for this target that can be used to build Java source.
-   *
-   * This class file should not be used for any targets besides the one that exposes this provider
-   * instance - even similar targets can have very different R.class files. In particular, final
-   * R.class files (for android_binary and similar rules) are generated very differently than
-   * R.class files for android_library - the latter are meant to be thrown away after building local
-   * Java classes.
-   *
-   * An R.class file is exposed rather than an R.java file because generating the R.class from the
-   * R.txt file is quicker than compiling the R.java file.
-   */
-  private final Artifact rClassJar;
-
-  /*
    * Transitive information used for resource processing
    */
 
@@ -85,7 +71,6 @@ public class AndroidResourcesInfo extends NativeInfo {
       Label label,
       ProcessedAndroidManifest manifest,
       Artifact rTxt,
-      Artifact rClassJar,
       NestedSet<ValidatedAndroidData> transitiveAndroidResources,
       NestedSet<ValidatedAndroidData> directAndroidResources,
       NestedSet<Artifact> transitiveResources,
@@ -100,7 +85,6 @@ public class AndroidResourcesInfo extends NativeInfo {
     this.label = label;
     this.manifest = manifest;
     this.rTxt = rTxt;
-    this.rClassJar = rClassJar;
     this.transitiveAndroidResources = transitiveAndroidResources;
     this.directAndroidResources = directAndroidResources;
     this.transitiveResources = transitiveResources;
@@ -127,17 +111,6 @@ public class AndroidResourcesInfo extends NativeInfo {
     return rTxt;
   }
 
-  @SkylarkCallable(
-      name = "r_class_jar",
-      structField = true,
-      doc =
-          "Returns a JAR of R.class files for this target, for use in compiling this target's Java"
-              + " code. This JAR is only accurate for this target, and should not be used in any"
-              + " way for other targets.")
-  public Artifact getRClassJar() {
-    return rClassJar;
-  }
-
   /** Returns the transitive ResourceContainers for the label. */
   @SkylarkCallable(
     name = "transitive_android_resources",
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 23d70394d4..3417f538c6 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
@@ -25,7 +25,6 @@ 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.NestedSetBuilder;
 import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion;
-import com.google.devtools.build.lib.rules.android.ResourceApk.ProcessedTransitiveData;
 import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.ToArg;
 import com.google.devtools.build.lib.rules.android.ResourceContainerConverter.ToArg.Includes;
 import com.google.devtools.build.lib.util.OS;
@@ -228,11 +227,11 @@ public class AndroidResourcesProcessorBuilder {
    * @return a {@link ResourceApk} containing the processed resource, asset, and manifest
    *     information.
    */
-  public ProcessedTransitiveData buildWithoutLocalResources(StampedAndroidManifest manifest) {
+  public ResourceApk buildWithoutLocalResources(StampedAndroidManifest manifest) {
 
     build(AndroidResources.empty(), AndroidAssets.empty(), manifest);
 
-    return new ProcessedTransitiveData(
+    return ResourceApk.fromTransitiveResources(
         resourceDependencies,
         assetDependencies,
         manifest.withProcessedManifest(manifestOut == null ? manifest.getManifest() : manifestOut),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java
index a4c0704d05..4b8574bda3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidSkylarkData.java
@@ -20,7 +20,6 @@ import com.google.devtools.build.lib.analysis.FileProvider;
 import com.google.devtools.build.lib.analysis.skylark.SkylarkRuleContext;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
-import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion;
 import com.google.devtools.build.lib.skylarkinterface.Param;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
@@ -109,16 +108,7 @@ public class AndroidSkylarkData {
             named = true,
             doc =
                 "Defaults to False. If true, resources will not be exposed to targets that depend"
-                    + " on them."),
-        @Param(
-            name = "aapt_version",
-            positional = false,
-            defaultValue = "'auto'",
-            type = String.class,
-            named = true,
-            doc =
-                "The version of aapt to use. Defaults to 'auto'. 'aapt' and 'aapt2' are also"
-                    + " supported."),
+                    + " on them.")
       },
       doc =
           "Creates an AndroidResourcesInfo from this target's resource dependencies, ignoring local"
@@ -127,22 +117,14 @@ public class AndroidSkylarkData {
               + " manifest will be generated and included in the provider - this path should not"
               + " be used when an explicit manifest is specified.")
   public static AndroidResourcesInfo resourcesFromDeps(
-      SkylarkRuleContext ctx,
-      SkylarkList<AndroidResourcesInfo> deps,
-      boolean neverlink,
-      String aaptVersionString)
+      SkylarkRuleContext ctx, SkylarkList<AndroidResourcesInfo> deps, boolean neverlink)
       throws EvalException, InterruptedException {
-    try {
-      return ResourceApk.processFromTransitiveLibraryData(
-              ctx.getRuleContext(),
-              ResourceDependencies.fromProviders(deps, /* neverlink = */ neverlink),
-              AssetDependencies.empty(),
-              StampedAndroidManifest.createEmpty(ctx.getRuleContext(), /* exported = */ false),
-              AndroidAaptVersion.chooseTargetAaptVersion(ctx.getRuleContext(), aaptVersionString))
-          .toResourceInfo(ctx.getLabel());
-    } catch (RuleErrorException e) {
-      throw new EvalException(Location.BUILTIN, e);
-    }
+    return ResourceApk.processFromTransitiveLibraryData(
+            ctx.getRuleContext(),
+            ResourceDependencies.fromProviders(deps, /* neverlink = */ neverlink),
+            AssetDependencies.empty(),
+            StampedAndroidManifest.createEmpty(ctx.getRuleContext(), /* exported = */ false))
+        .toResourceInfo(ctx.getLabel());
   }
 
   /**
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 e93f0db376..016aee5866 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
@@ -30,7 +30,6 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion;
-import com.google.devtools.build.lib.rules.android.ResourceApk.ProcessedTransitiveData;
 import java.util.ArrayList;
 import java.util.List;
 import javax.annotation.Nullable;
@@ -77,17 +76,6 @@ public class RClassGeneratorActionBuilder {
     return data.withValidatedResources(classJarOut);
   }
 
-  public ResourceApk build(ProcessedTransitiveData data) {
-    build(data.getRTxt(), data.getManifest());
-
-    return ResourceApk.fromTransitiveResources(
-        data.getResourceDeps(),
-        data.getAssetDeps(),
-        data.getManifest(),
-        data.getRTxt(),
-        classJarOut);
-  }
-
   private void build(Artifact rTxt, ProcessedAndroidManifest manifest) {
     CustomCommandLine.Builder builder = new CustomCommandLine.Builder();
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java
index 16aa003284..cc826a067c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java
@@ -20,8 +20,6 @@ import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.config.CompilationMode;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
-import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion;
 import javax.annotation.Nullable;
 
 /**
@@ -50,7 +48,6 @@ public final class ResourceApk {
   // The non-binary XML version of AndroidManifest.xml
   private final ProcessedAndroidManifest manifest;
   private final Artifact rTxt;
-  private final Artifact rClassJar;
   @Nullable private final Artifact resourceProguardConfig;
   @Nullable private final Artifact mainDexProguardConfig;
 
@@ -74,7 +71,6 @@ public final class ResourceApk {
         resourceContainer.getAndroidAssets(),
         resourceContainer.getProcessedManifest(),
         resourceContainer.getRTxt(),
-        resourceContainer.getJavaClassJar(),
         resourceProguardConfig,
         mainDexProguardConfig);
   }
@@ -95,7 +91,6 @@ public final class ResourceApk {
         assets,
         resources.getProcessedManifest(),
         resources.getRTxt(),
-        resources.getClassJar(),
         resourceProguardConfig,
         mainDexProguardConfig);
   }
@@ -111,7 +106,6 @@ public final class ResourceApk {
       AndroidAssets primaryAssets,
       ProcessedAndroidManifest manifest,
       Artifact rTxt,
-      Artifact rClassJar,
       @Nullable Artifact resourceProguardConfig,
       @Nullable Artifact mainDexProguardConfig) {
     this.resourceApk = resourceApk;
@@ -124,7 +118,6 @@ public final class ResourceApk {
     this.primaryAssets = primaryAssets;
     this.manifest = manifest;
     this.rTxt = rTxt;
-    this.rClassJar = rClassJar;
     this.resourceProguardConfig = resourceProguardConfig;
     this.mainDexProguardConfig = mainDexProguardConfig;
   }
@@ -141,7 +134,6 @@ public final class ResourceApk {
         primaryAssets,
         manifest,
         rTxt,
-        rClassJar,
         resourceProguardConfig,
         mainDexProguardConfig);
   }
@@ -183,8 +175,7 @@ public final class ResourceApk {
       ResourceDependencies resourceDeps,
       AssetDependencies assetDeps,
       ProcessedAndroidManifest manifest,
-      Artifact rTxt,
-      Artifact rClassJar) {
+      Artifact rTxt) {
     return new ResourceApk(
         null,
         null,
@@ -196,7 +187,6 @@ public final class ResourceApk {
         AndroidAssets.empty(),
         manifest,
         rTxt,
-        rClassJar,
         null,
         null);
   }
@@ -229,7 +219,7 @@ public final class ResourceApk {
    */
   AndroidResourcesInfo toResourceInfo(Label label) {
     if (validatedResources == null) {
-      return resourceDeps.toInfo(label, manifest, rTxt, rClassJar);
+      return resourceDeps.toInfo(label, manifest, rTxt);
     }
     return resourceDeps.toInfo(validatedResources);
   }
@@ -280,9 +270,8 @@ public final class ResourceApk {
       RuleContext ruleContext,
       ResourceDependencies resourceDeps,
       AssetDependencies assetDeps,
-      StampedAndroidManifest manifest,
-      AndroidAaptVersion aaptVersion)
-      throws InterruptedException, RuleErrorException {
+      StampedAndroidManifest manifest)
+      throws InterruptedException {
 
     return new AndroidResourcesProcessorBuilder(ruleContext)
         .setLibrary(true)
@@ -297,55 +286,6 @@ public final class ResourceApk {
         .setDebug(ruleContext.getConfiguration().getCompilationMode() != CompilationMode.OPT)
         .setThrowOnResourceConflict(
             AndroidCommon.getAndroidConfig(ruleContext).throwOnResourceConflict())
-        .buildWithoutLocalResources(manifest)
-        .generateRClass(ruleContext, aaptVersion);
-  }
-
-  /**
-   * Intermediate class representing processed data from transitive deps of a library, without a
-   * generated R class
-   */
-  static final class ProcessedTransitiveData {
-    private final ResourceDependencies resourceDeps;
-    private final AssetDependencies assetDeps;
-    private final ProcessedAndroidManifest manifest;
-    private final Artifact rTxt;
-
-    ProcessedTransitiveData(
-        ResourceDependencies resourceDeps,
-        AssetDependencies assetDeps,
-        ProcessedAndroidManifest manifest,
-        Artifact rTxt) {
-      this.resourceDeps = resourceDeps;
-      this.assetDeps = assetDeps;
-      this.manifest = manifest;
-      this.rTxt = rTxt;
-    }
-
-    public ResourceApk generateRClass(RuleContext ruleContext, AndroidAaptVersion aaptVersion)
-        throws InterruptedException {
-      return new RClassGeneratorActionBuilder(ruleContext)
-          .targetAaptVersion(aaptVersion)
-          .withDependencies(resourceDeps)
-          .setClassJarOut(
-              ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_CLASS_JAR))
-          .build(this);
-    }
-
-    public ResourceDependencies getResourceDeps() {
-      return resourceDeps;
-    }
-
-    public AssetDependencies getAssetDeps() {
-      return assetDeps;
-    }
-
-    public ProcessedAndroidManifest getManifest() {
-      return manifest;
-    }
-
-    public Artifact getRTxt() {
-      return rTxt;
-    }
+        .buildWithoutLocalResources(manifest);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java
index 264804ef7a..09ac5afbb3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java
@@ -276,14 +276,12 @@ public final class ResourceDependencies {
           .toInfo(
               newDirectResource.getLabel(),
               newDirectResource.getProcessedManifest(),
-              newDirectResource.getRTxt(),
-              newDirectResource.getJavaClassJar());
+              newDirectResource.getRTxt());
     }
     return new AndroidResourcesInfo(
         newDirectResource.getLabel(),
         newDirectResource.getProcessedManifest(),
         newDirectResource.getRTxt(),
-        newDirectResource.getJavaClassJar(),
         NestedSetBuilder.<ValidatedAndroidData>naiveLinkOrder()
             .addTransitive(transitiveResourceContainers)
             .addTransitive(directResourceContainers)
@@ -316,15 +314,14 @@ public final class ResourceDependencies {
    * @return A provider with the current resources and label.
    */
   public AndroidResourcesInfo toInfo(
-      Label label, ProcessedAndroidManifest manifest, Artifact rTxt, Artifact rClassJar) {
+      Label label, ProcessedAndroidManifest manifest, Artifact rTxt) {
     if (neverlink) {
-      return ResourceDependencies.empty().toInfo(label, manifest, rTxt, rClassJar);
+      return ResourceDependencies.empty().toInfo(label, manifest, rTxt);
     }
     return new AndroidResourcesInfo(
         label,
         manifest,
         rTxt,
-        rClassJar,
         transitiveResourceContainers,
         directResourceContainers,
         transitiveResources,