Include library R classes in runtime classpath for library but not binary

In general, R classes for Android libraries are used for compilation of those
libraries and then thrown away. They are replaced by the R classes from the
binary. Removing the unused android_library R classes will make binaries
smaller.

RELNOTES: none
PiperOrigin-RevId: 169244068

3 files changed, 34 insertions, 23 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 0bb79d7e22..1385b16a7b 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
@@ -311,6 +311,16 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
 
     boolean shrinkResources = shouldShrinkResources(ruleContext);
 
+    NestedSet<Artifact> excludedRuntimeArtifacts = null;
+    if (!androidConfig.includeLibraryResourceJars()) {
+      // Remove the library resource JARs from the binary's runtime classpath.
+      // Resource classes from android_library dependencies are replaced by the binary's resource
+      // class. We remove them only at the top level so that resources included by a library that is
+      // a dependency of a java_library are still included, since these resources are propagated via
+      // android-specific providers and won't show up when we collect the library resource JARs.
+      excludedRuntimeArtifacts = getLibraryResourceJars(ruleContext);
+    }
+
     JavaTargetAttributes resourceClasses = androidCommon.init(
         javaSemantics,
         androidSemantics,
@@ -318,7 +328,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
         ruleContext.getConfiguration().isCodeCoverageEnabled(),
         true /* collectJavaCompilationArgs */,
         true, /* isBinary */
-        androidConfig.includeLibraryResourceJars());
+        excludedRuntimeArtifacts);
     ruleContext.assertNoErrors();
 
     Function<Artifact, Artifact> derivedJarFunction =
@@ -689,19 +699,9 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
 
     // The resources jars from android_library rules contain stub ids, so filter those out of the
     // transitive jars.
-    Iterable<AndroidLibraryResourceClassJarProvider> libraryResourceJarProviders =
-        AndroidCommon.getTransitivePrerequisites(
-            ruleContext, Mode.TARGET, AndroidLibraryResourceClassJarProvider.class);
-
-    NestedSetBuilder<Artifact> libraryResourceJarsBuilder = NestedSetBuilder.naiveLinkOrder();
-    for (AndroidLibraryResourceClassJarProvider provider : libraryResourceJarProviders) {
-      libraryResourceJarsBuilder.addTransitive(provider.getResourceClassJars());
-    }
-    NestedSet<Artifact> libraryResourceJars = libraryResourceJarsBuilder.build();
-
     Iterable<Artifact> filteredJars =
         Streams.stream(jars)
-            .filter(not(in(libraryResourceJars.toSet())))
+            .filter(not(in(getLibraryResourceJars(ruleContext).toSet())))
             .collect(toImmutableList());
 
     AndroidSdkProvider sdk = AndroidSdkProvider.fromRuleContext(ruleContext);
@@ -720,6 +720,18 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
     ruleContext.registerAction(manifestAction);
   }
 
+  private static NestedSet<Artifact> getLibraryResourceJars(RuleContext ruleContext) {
+    Iterable<AndroidLibraryResourceClassJarProvider> libraryResourceJarProviders =
+        AndroidCommon.getTransitivePrerequisites(
+            ruleContext, Mode.TARGET, AndroidLibraryResourceClassJarProvider.class);
+
+    NestedSetBuilder<Artifact> libraryResourceJarsBuilder = NestedSetBuilder.naiveLinkOrder();
+    for (AndroidLibraryResourceClassJarProvider provider : libraryResourceJarProviders) {
+      libraryResourceJarsBuilder.addTransitive(provider.getResourceClassJars());
+    }
+    return libraryResourceJarsBuilder.build();
+  }
+
   /** Generates an uncompressed _deploy.jar of all the runtime jars. */
   public static Artifact createDeployJar(
       RuleContext ruleContext,
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 1fd3e9aa63..0796ae2db6 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
@@ -541,7 +541,7 @@ public class AndroidCommon {
       boolean addCoverageSupport,
       boolean collectJavaCompilationArgs,
       boolean isBinary,
-      boolean includeLibraryResourceJars)
+      NestedSet<Artifact> excludedRuntimeArtifacts)
       throws InterruptedException, RuleErrorException {
 
     classJar = ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_LIBRARY_CLASS_JAR);
@@ -572,6 +572,10 @@ public class AndroidCommon {
       DataBinding.addAnnotationProcessor(ruleContext, attributes);
     }
 
+    if (excludedRuntimeArtifacts != null) {
+      attributes.addExcludedArtifacts(excludedRuntimeArtifacts);
+    }
+
     JavaCompilationArtifacts.Builder artifactsBuilder = new JavaCompilationArtifacts.Builder();
     NestedSetBuilder<Artifact> jarsProducedForRuntime = NestedSetBuilder.<Artifact>stableOrder();
     NestedSetBuilder<Artifact> filesBuilder = NestedSetBuilder.<Artifact>stableOrder();
@@ -586,14 +590,10 @@ public class AndroidCommon {
       compileResources(javaSemantics, resourceApk, resourcesJar, artifactsBuilder, attributes,
           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);
-      }
+      // 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
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 f00baedc7f..011c7830e8 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
@@ -222,7 +222,6 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
           ResourceDependencies.fromRuleResourceAndDeps(ruleContext, false /* neverlink */));
     }
 
-    AndroidConfiguration androidConfig = ruleContext.getFragment(AndroidConfiguration.class);
 
     JavaTargetAttributes javaTargetAttributes = androidCommon.init(
         javaSemantics,
@@ -231,7 +230,7 @@ public abstract class AndroidLibrary implements RuleConfiguredTargetFactory {
         false /* addCoverageSupport */,
         true /* collectJavaCompilationArgs */,
         false /* isBinary */,
-        androidConfig.includeLibraryResourceJars());
+        null /* excludedRuntimeArtifacts */);
     if (javaTargetAttributes == null) {
       return null;
     }