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: 195419781

9 files changed, 168 insertions, 33 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 49dd128c72..5ba8f806fb 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, RuleErrorException {
+      throws InterruptedException {
 
     // 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 678a5f67cd..f63ec0de8b 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,23 +230,12 @@ 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.
-          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;
+          return chooseTargetAaptVersion(
+              ruleContext, ruleContext.attributes().get("aapt_version", STRING));
         } else {
           // On rules can't choose, assume aapt2 if aapt2 is present in the sdk.
           return hasAapt2 ? AAPT2 : AAPT;
@@ -254,6 +243,29 @@ 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 7587421bb3..9125bf662e 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,6 +27,7 @@ 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;
@@ -187,7 +188,8 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
               ruleContext,
               resourceDeps,
               assetDeps,
-              StampedAndroidManifest.createEmpty(ruleContext, /* exported = */ false));
+              StampedAndroidManifest.createEmpty(ruleContext, /* exported = */ false),
+              AndroidAaptVersion.chooseTargetAaptVersion(ruleContext));
     }
 
     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 6f1d665be4..060b52373c 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,6 +53,20 @@ 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
    */
 
@@ -71,6 +85,7 @@ public class AndroidResourcesInfo extends NativeInfo {
       Label label,
       ProcessedAndroidManifest manifest,
       Artifact rTxt,
+      Artifact rClassJar,
       NestedSet<ValidatedAndroidData> transitiveAndroidResources,
       NestedSet<ValidatedAndroidData> directAndroidResources,
       NestedSet<Artifact> transitiveResources,
@@ -85,6 +100,7 @@ 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;
@@ -111,6 +127,17 @@ 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 3417f538c6..23d70394d4 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,6 +25,7 @@ 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;
@@ -227,11 +228,11 @@ public class AndroidResourcesProcessorBuilder {
    * @return a {@link ResourceApk} containing the processed resource, asset, and manifest
    *     information.
    */
-  public ResourceApk buildWithoutLocalResources(StampedAndroidManifest manifest) {
+  public ProcessedTransitiveData buildWithoutLocalResources(StampedAndroidManifest manifest) {
 
     build(AndroidResources.empty(), AndroidAssets.empty(), manifest);
 
-    return ResourceApk.fromTransitiveResources(
+    return new ProcessedTransitiveData(
         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 4b8574bda3..a4c0704d05 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,6 +20,7 @@ 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;
@@ -108,7 +109,16 @@ public class AndroidSkylarkData {
             named = true,
             doc =
                 "Defaults to False. If true, resources will not be exposed to targets that depend"
-                    + " on them.")
+                    + " 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."),
       },
       doc =
           "Creates an AndroidResourcesInfo from this target's resource dependencies, ignoring local"
@@ -117,14 +127,22 @@ 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)
+      SkylarkRuleContext ctx,
+      SkylarkList<AndroidResourcesInfo> deps,
+      boolean neverlink,
+      String aaptVersionString)
       throws EvalException, InterruptedException {
-    return ResourceApk.processFromTransitiveLibraryData(
-            ctx.getRuleContext(),
-            ResourceDependencies.fromProviders(deps, /* neverlink = */ neverlink),
-            AssetDependencies.empty(),
-            StampedAndroidManifest.createEmpty(ctx.getRuleContext(), /* exported = */ false))
-        .toResourceInfo(ctx.getLabel());
+    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);
+    }
   }
 
   /**
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 016aee5866..e93f0db376 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,6 +30,7 @@ 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;
@@ -76,6 +77,17 @@ 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 cc826a067c..16aa003284 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,6 +20,8 @@ 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;
 
 /**
@@ -48,6 +50,7 @@ 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;
 
@@ -71,6 +74,7 @@ public final class ResourceApk {
         resourceContainer.getAndroidAssets(),
         resourceContainer.getProcessedManifest(),
         resourceContainer.getRTxt(),
+        resourceContainer.getJavaClassJar(),
         resourceProguardConfig,
         mainDexProguardConfig);
   }
@@ -91,6 +95,7 @@ public final class ResourceApk {
         assets,
         resources.getProcessedManifest(),
         resources.getRTxt(),
+        resources.getClassJar(),
         resourceProguardConfig,
         mainDexProguardConfig);
   }
@@ -106,6 +111,7 @@ public final class ResourceApk {
       AndroidAssets primaryAssets,
       ProcessedAndroidManifest manifest,
       Artifact rTxt,
+      Artifact rClassJar,
       @Nullable Artifact resourceProguardConfig,
       @Nullable Artifact mainDexProguardConfig) {
     this.resourceApk = resourceApk;
@@ -118,6 +124,7 @@ public final class ResourceApk {
     this.primaryAssets = primaryAssets;
     this.manifest = manifest;
     this.rTxt = rTxt;
+    this.rClassJar = rClassJar;
     this.resourceProguardConfig = resourceProguardConfig;
     this.mainDexProguardConfig = mainDexProguardConfig;
   }
@@ -134,6 +141,7 @@ public final class ResourceApk {
         primaryAssets,
         manifest,
         rTxt,
+        rClassJar,
         resourceProguardConfig,
         mainDexProguardConfig);
   }
@@ -175,7 +183,8 @@ public final class ResourceApk {
       ResourceDependencies resourceDeps,
       AssetDependencies assetDeps,
       ProcessedAndroidManifest manifest,
-      Artifact rTxt) {
+      Artifact rTxt,
+      Artifact rClassJar) {
     return new ResourceApk(
         null,
         null,
@@ -187,6 +196,7 @@ public final class ResourceApk {
         AndroidAssets.empty(),
         manifest,
         rTxt,
+        rClassJar,
         null,
         null);
   }
@@ -219,7 +229,7 @@ public final class ResourceApk {
    */
   AndroidResourcesInfo toResourceInfo(Label label) {
     if (validatedResources == null) {
-      return resourceDeps.toInfo(label, manifest, rTxt);
+      return resourceDeps.toInfo(label, manifest, rTxt, rClassJar);
     }
     return resourceDeps.toInfo(validatedResources);
   }
@@ -270,8 +280,9 @@ public final class ResourceApk {
       RuleContext ruleContext,
       ResourceDependencies resourceDeps,
       AssetDependencies assetDeps,
-      StampedAndroidManifest manifest)
-      throws InterruptedException {
+      StampedAndroidManifest manifest,
+      AndroidAaptVersion aaptVersion)
+      throws InterruptedException, RuleErrorException {
 
     return new AndroidResourcesProcessorBuilder(ruleContext)
         .setLibrary(true)
@@ -286,6 +297,55 @@ public final class ResourceApk {
         .setDebug(ruleContext.getConfiguration().getCompilationMode() != CompilationMode.OPT)
         .setThrowOnResourceConflict(
             AndroidCommon.getAndroidConfig(ruleContext).throwOnResourceConflict())
-        .buildWithoutLocalResources(manifest);
+        .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;
+    }
   }
 }
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 09ac5afbb3..264804ef7a 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,12 +276,14 @@ public final class ResourceDependencies {
           .toInfo(
               newDirectResource.getLabel(),
               newDirectResource.getProcessedManifest(),
-              newDirectResource.getRTxt());
+              newDirectResource.getRTxt(),
+              newDirectResource.getJavaClassJar());
     }
     return new AndroidResourcesInfo(
         newDirectResource.getLabel(),
         newDirectResource.getProcessedManifest(),
         newDirectResource.getRTxt(),
+        newDirectResource.getJavaClassJar(),
         NestedSetBuilder.<ValidatedAndroidData>naiveLinkOrder()
             .addTransitive(transitiveResourceContainers)
             .addTransitive(directResourceContainers)
@@ -314,14 +316,15 @@ public final class ResourceDependencies {
    * @return A provider with the current resources and label.
    */
   public AndroidResourcesInfo toInfo(
-      Label label, ProcessedAndroidManifest manifest, Artifact rTxt) {
+      Label label, ProcessedAndroidManifest manifest, Artifact rTxt, Artifact rClassJar) {
     if (neverlink) {
-      return ResourceDependencies.empty().toInfo(label, manifest, rTxt);
+      return ResourceDependencies.empty().toInfo(label, manifest, rTxt, rClassJar);
     }
     return new AndroidResourcesInfo(
         label,
         manifest,
         rTxt,
+        rClassJar,
         transitiveResourceContainers,
         directResourceContainers,
         transitiveResources,