In android_library targets, R.class files should not be runtime dependencies

android_binary targets have their own R.java files (built from merging
dependencies and any resources that belong directly to the target). As such,
they don't need inherited R.java files at runtime. Taking these out makes for
smaller APKs and less inheritance from the target's dependencies.

Add a flag to control this behavior. Have it default to continue to include
R.class files as runtime dependencies so we can control rollout of this
behavior.

Add tests of android_binary to ensure the JAR is filtered out as appropriate,
and of android_robolectrictest to ensure that those tests still have access to
the JARs.

RELNOTES: none
PiperOrigin-RevId: 153177074

5 files changed, 117 insertions, 11 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
index 29dad5109b..52c1b52a56 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
@@ -391,7 +391,8 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
         resourceApk,
         ruleContext.getConfiguration().isCodeCoverageEnabled(),
         true /* collectJavaCompilationArgs */,
-        true /* isBinary */);
+        true, /* isBinary */
+        androidConfig.includeLibraryResourceJars());
     ruleContext.assertNoErrors();
 
     Function<Artifact, Artifact> derivedJarFunction =
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 e3f2417cd7..ab6878c824 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
@@ -378,8 +378,8 @@ public class AndroidCommon {
       JavaCompilationArtifacts.Builder artifactsBuilder,
       JavaTargetAttributes.Builder attributes,
       NestedSetBuilder<Artifact> filesBuilder,
-      NestedSetBuilder<Artifact> jarsProducedForRuntime,
-      boolean useRClassGenerator) throws InterruptedException {
+      boolean useRClassGenerator)
+      throws InterruptedException {
     compileResourceJar(javaSemantics, resourceApk, resourcesJar, useRClassGenerator);
     // Add the compiled resource jar to the classpath of the main compilation.
     attributes.addDirectJars(NestedSetBuilder.create(Order.STABLE_ORDER, resourceClassJar));
@@ -388,10 +388,7 @@ public class AndroidCommon {
     // except for <clinit>, but it takes time to build and waiting for that to build would
     // just delay building the rest of the library.
     artifactsBuilder.addCompileTimeJar(resourceClassJar);
-    // Combined resource constants needs to come even before our own classes that may contain
-    // local resource constants.
-    artifactsBuilder.addRuntimeJar(resourceClassJar);
-    jarsProducedForRuntime.add(resourceClassJar);
+
     // Add the compiled resource jar as a declared output of the rule.
     filesBuilder.add(resourceSourceJar);
     filesBuilder.add(resourceClassJar);
@@ -503,7 +500,8 @@ public class AndroidCommon {
       ResourceApk resourceApk,
       boolean addCoverageSupport,
       boolean collectJavaCompilationArgs,
-      boolean isBinary)
+      boolean isBinary,
+      boolean includeLibraryResourceJars)
       throws InterruptedException {
 
     classJar = ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_LIBRARY_CLASS_JAR);
@@ -544,7 +542,17 @@ public class AndroidCommon {
       // (the legacy srcjar requires the createJarJar step below).
       boolean useRClassGenerator = isBinary && !resourceApk.isLegacy();
       compileResources(javaSemantics, resourceApk, resourcesJar, artifactsBuilder, attributes,
-          filesBuilder, jarsProducedForRuntime, useRClassGenerator);
+          filesBuilder, useRClassGenerator);
+
+      // In binary targets, add the resource jar as a runtime dependency. In libraries, the resource
+      // jar from the appropriate binary will be used, but add this jar anyway if requested.
+      if (isBinary || includeLibraryResourceJars) {
+        // Combined resource constants needs to come even before our own classes that may contain
+        // local resource constants.
+        artifactsBuilder.addRuntimeJar(resourceClassJar);
+        jarsProducedForRuntime.add(resourceClassJar);
+      }
+
       if (resourceApk.isLegacy()) {
         // Repackages the R.java for each dependency package and places the resultant jars before
         // the dependency libraries to ensure that the generated resource ids are correct.
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 cf9c618d07..b7e543e83a 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
@@ -444,6 +444,15 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
         help = "Compress Java resources in APKs")
     public boolean compressJavaResources;
 
+    @Option(name = "experimental_android_include_library_resource_jars",
+        defaultValue = "true",
+        category = "undocumented",
+        help = "Specifies whether resource JAR files for android_library targets should be included"
+            + " as runtime dependencies. Defaults to the old behavior, including them. These JARs"
+            + " are not nessecary for normal use as all required resources are included in the"
+            + " top-level android_binary resource JAR.")
+    public boolean includeLibraryResourceJars;
+
     @Override
     public void addAllLabels(Multimap<String, Label> labelMap) {
       if (androidCrosstoolTop != null) {
@@ -528,6 +537,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
   private final ResourceFilter resourceFilter;
   private final boolean useSingleJarForProguardLibraryJars;
   private final boolean compressJavaResources;
+  private final boolean includeLibraryResourceJars;
 
   AndroidConfiguration(Options options, Label androidSdk) throws InvalidConfigurationException {
     this.sdk = androidSdk;
@@ -559,6 +569,7 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     this.useRexToCompressDexFiles = options.useRexToCompressDexFiles;
     this.resourceFilter = options.resourceFilter;
     this.compressJavaResources = options.compressJavaResources;
+    this.includeLibraryResourceJars = options.includeLibraryResourceJars;
 
     if (!dexoptsSupportedInIncrementalDexing.contains("--no-locals")) {
       // TODO(bazel-team): Still needed? See DexArchiveAspect
@@ -671,6 +682,10 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     return compressJavaResources;
   }
 
+  public boolean includeLibraryResourceJars() {
+    return includeLibraryResourceJars;
+  }
+
   @Override
   public void addGlobalMakeVariables(ImmutableMap.Builder<String, String> globalMakeEnvBuilder) {
     globalMakeEnvBuilder.put("ANDROID_CPU", cpu);
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 1f5e1a62b7..5f0da5fbc8 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
@@ -116,7 +116,8 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
           ResourceDependencies.fromRuleResourceAndDeps(ruleContext, false /* neverlink */));
     }
 
-    if (!ruleContext.getFragment(AndroidConfiguration.class).allowSrcsLessAndroidLibraryDeps()
+    AndroidConfiguration androidConfig = ruleContext.getFragment(AndroidConfiguration.class);
+    if (!androidConfig.allowSrcsLessAndroidLibraryDeps()
         && !definesLocalResources
         && ruleContext.attributes().get("srcs", BuildType.LABEL_LIST).isEmpty()
         && ruleContext.attributes().get("idl_srcs", BuildType.LABEL_LIST).isEmpty()
@@ -130,7 +131,8 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
         resourceApk,
         false /* addCoverageSupport */,
         true /* collectJavaCompilationArgs */,
-        false /* isBinary */);
+        false /* isBinary */,
+        androidConfig.includeLibraryResourceJars());
     if (javaTargetAttributes == null) {
       return null;
     }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
index 9a54de5bdb..6b366b418b 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
@@ -1539,6 +1539,86 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase {
   }
 
   @Test
+  public void testInheritedRNotInRuntimeJars() throws Exception {
+    String dir = "java/r/android/";
+    scratch.file(
+        dir + "BUILD",
+        "android_library(name = 'sublib',",
+        "                manifest = 'AndroidManifest.xml',",
+        "                resource_files = glob(['res3/**']),",
+        "                srcs =['sublib.java'],",
+        "                )",
+        "android_library(name = 'lib',",
+        "                manifest = 'AndroidManifest.xml',",
+        "                resource_files = glob(['res2/**']),",
+        "                deps = [':sublib'],",
+        "                srcs =['lib.java'],",
+        "                )",
+        "android_binary(name = 'bin',",
+        "               manifest = 'AndroidManifest.xml',",
+        "               resource_files = glob(['res/**']),",
+        "               deps = [':lib'],",
+        "               srcs =['bin.java'],",
+        "               )");
+
+    // TODO(b/37087277): Remove this once this behavior is the default
+    useConfiguration("--noexperimental_android_include_library_resource_jars");
+
+    Action deployJarAction =
+        getGeneratingAction(
+            getFileConfiguredTarget("//java/r/android:bin_deploy.jar").getArtifact());
+    List<String> inputs = ActionsTestUtil.prettyArtifactNames(deployJarAction.getInputs());
+
+    assertThat(inputs)
+        .containsAllOf(
+            dir + "libsublib.jar",
+            dir + "liblib.jar",
+            dir + "libbin.jar",
+            dir + "bin_resources.jar");
+    assertThat(inputs).containsNoneOf(dir + "lib_resources.jar", dir + "sublib_resources.jar");
+  }
+
+  @Test
+  public void testInheritedRInRuntimeJarsWhenSpecified() throws Exception {
+    String dir = "java/r/android/";
+    scratch.file(
+        dir + "BUILD",
+        "android_library(name = 'sublib',",
+        "                manifest = 'AndroidManifest.xml',",
+        "                resource_files = glob(['res3/**']),",
+        "                srcs =['sublib.java'],",
+        "                )",
+        "android_library(name = 'lib',",
+        "                manifest = 'AndroidManifest.xml',",
+        "                resource_files = glob(['res2/**']),",
+        "                deps = [':sublib'],",
+        "                srcs =['lib.java'],",
+        "                )",
+        "android_binary(name = 'bin',",
+        "               manifest = 'AndroidManifest.xml',",
+        "               resource_files = glob(['res/**']),",
+        "               deps = [':lib'],",
+        "               srcs =['bin.java'],",
+        "               )");
+
+    // TODO(b/37087277): Add a configuration flag once this behavior is no longer the default.
+
+    Action deployJarAction =
+        getGeneratingAction(
+            getFileConfiguredTarget("//java/r/android:bin_deploy.jar").getArtifact());
+    List<String> inputs = ActionsTestUtil.prettyArtifactNames(deployJarAction.getInputs());
+
+    assertThat(inputs)
+        .containsAllOf(
+            dir + "libsublib.jar",
+            dir + "liblib.jar",
+            dir + "libbin.jar",
+            dir + "bin_resources.jar",
+            dir + "lib_resources.jar",
+            dir + "sublib_resources.jar");
+  }
+
+  @Test
   public void testLocalResourcesUseRClassGenerator() throws Exception {
     scratch.file("java/r/android/BUILD",
         "android_library(name = 'lib',",