Make aar_import use the java package from the manifest for all aapt resource rules.

The AndroidResourcesProcessorBuilder, AndroidResourceValidatorActionBuilder and AndroidResourceMergingActionBuilder that use the java package specified by the AndroidResourceProvider check if it is not null or empty before using it and default to using the package from the manifest. This CL just allows us to pass null java package from AndroidSdkProvider and does so with aar_import.

--
MOS_MIGRATED_REVID=133883286

4 files changed, 50 insertions, 30 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AarImport.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AarImport.java
index 95bdbf4e94..a6277301c6 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AarImport.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AarImport.java
@@ -27,8 +27,8 @@ 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;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
-import com.google.devtools.build.lib.rules.android.AndroidCommon;
 import com.google.devtools.build.lib.rules.android.AndroidResourcesProvider;
+import com.google.devtools.build.lib.rules.android.AndroidRuleClasses;
 import com.google.devtools.build.lib.rules.android.ApplicationManifest;
 import com.google.devtools.build.lib.rules.android.LocalResourceContainer;
 import com.google.devtools.build.lib.rules.android.ResourceApk;
@@ -75,8 +75,7 @@ public class AarImport implements RuleConfiguredTargetFactory {
     ruleContext.registerAction(createTreePopulater(ruleContext, aar, resourcesManifest, resources));
 
     ApplicationManifest androidManifest =
-        ApplicationManifest.fromExplicitManifest(ruleContext, androidManifestArtifact)
-            .renamePackage(ruleContext, AndroidCommon.getJavaPackage(ruleContext));
+        ApplicationManifest.fromExplicitManifest(ruleContext, androidManifestArtifact);
 
     FileProvider resourcesProvider = new FileProvider(
         new NestedSetBuilder<Artifact>(Order.NAIVE_LINK_ORDER).add(resources).build());
@@ -86,7 +85,11 @@ public class AarImport implements RuleConfiguredTargetFactory {
         new LocalResourceContainer.Builder(ruleContext)
             .withResources(ImmutableList.of(resourcesProvider))
             .build(),
-        ResourceDependencies.fromRuleDeps(ruleContext, JavaCommon.isNeverLink(ruleContext)));
+        ResourceDependencies.fromRuleDeps(ruleContext, JavaCommon.isNeverLink(ruleContext)),
+        ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
+        ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_SYMBOLS_TXT),
+        ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_PROCESSED_MANIFEST),
+        ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_ZIP));
 
     return ruleBuilder
         .setFilesToBuild(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceContainerBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceContainerBuilder.java
index aa7e32312b..70d91f2b3a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceContainerBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourceContainerBuilder.java
@@ -23,7 +23,7 @@ import com.google.devtools.build.lib.util.Preconditions;
 import javax.annotation.Nullable;
 
 /**
- * Encapsulates the process of building the AndroidResourceContainer.
+ * Encapsulates the process of building {@link ResourceContainer}.
  */
 public final class AndroidResourceContainerBuilder {
   private LocalResourceContainer data;
@@ -31,6 +31,7 @@ public final class AndroidResourceContainerBuilder {
   private Artifact rOutput;
   private boolean inlineConstants = false;
   private Artifact symbolsFile;
+  private boolean useJavaPackageFromManifest = false;
 
   /** Provides the resources and assets for the ResourceContainer. */
   public AndroidResourceContainerBuilder withData(LocalResourceContainer data) {
@@ -53,6 +54,12 @@ public final class AndroidResourceContainerBuilder {
     return this;
   }
 
+  public AndroidResourceContainerBuilder useJavaPackageFromManifest(
+      boolean useJavaPackageFromManifest) {
+    this.useJavaPackageFromManifest = useJavaPackageFromManifest;
+    return this;
+  }
+
   /** Creates a {@link ResourceContainer} from a {@link RuleContext}.
    * @throws InterruptedException */
   public ResourceContainer buildFromRule(RuleContext ruleContext, @Nullable Artifact apk)
@@ -80,6 +87,9 @@ public final class AndroidResourceContainerBuilder {
   }
 
   private String getJavaPackage(RuleContext ruleContext, Artifact rJavaSrcJar) {
+    if (useJavaPackageFromManifest) {
+      return null;
+    }
     if (hasCustomPackage(ruleContext)) {
       return ruleContext.attributes().get("custom_package", Type.STRING);
     }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java
index 999584015d..dd117d9697 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java
@@ -77,7 +77,7 @@ public abstract class AndroidResourcesProvider implements TransitiveInfoProvider
 
     public static ResourceContainer create(
         Label label,
-        String javaPackage,
+        @Nullable String javaPackage,
         @Nullable String renameManifestPackage,
         boolean constantsInlined,
         @Nullable Artifact apk,
@@ -110,7 +110,7 @@ public abstract class AndroidResourcesProvider implements TransitiveInfoProvider
     }
 
     public abstract Label getLabel();
-    public abstract String getJavaPackage();
+    @Nullable public abstract String getJavaPackage();
     @Nullable public abstract String getRenameManifestPackage();
     public abstract boolean getConstantsInlined();
     @Nullable public abstract Artifact getApk();
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 29b0091bef..8db19e0ff2 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
@@ -292,17 +292,19 @@ public final class ApplicationManifest {
                 FileProvider.class)).build();
 
     return createApk(
-        resourceApk,
         ruleContext,
         false, /* isLibrary */
         resourceDeps,
-        rTxt,
-        null, /* Artifact symbolsTxt */
         ImmutableList.<String>of(), /* configurationFilters */
         ImmutableList.<String>of(), /* uncompressedExtensions */
         true, /* crunchPng */
         ImmutableList.<String>of(), /* densities */
         incremental,
+        new AndroidResourceContainerBuilder()
+            .withData(data)
+            .withManifest(getManifest())
+            .withROutput(rTxt)
+            .buildFromRule(ruleContext, resourceApk),
         data,
         proguardCfg,
         null, /* Artifact mainDexProguardCfg */
@@ -312,28 +314,37 @@ public final class ApplicationManifest {
 
   /** Packages up the manifest with resource and assets from the LocalResourceContainer. */
   public ResourceApk packWithDataAndResources(
-      RuleContext ruleContext, LocalResourceContainer data, ResourceDependencies resourceDeps)
-      throws InterruptedException {
+      RuleContext ruleContext,
+      LocalResourceContainer data,
+      ResourceDependencies resourceDeps,
+      Artifact rTxt,
+      Artifact symbolsTxt,
+      Artifact manifestOut,
+      Artifact mergedResources) throws InterruptedException {
     if (ruleContext.hasErrors()) {
       return null;
     }
     return createApk(
-        null, /* Artifact resourceApk */
         ruleContext,
         true, /* isLibrary */
         resourceDeps,
-        ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_R_TXT),
-        ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_SYMBOLS_TXT),
         ImmutableList.<String>of(), /* List<String> configurationFilters */
         ImmutableList.<String>of(), /* List<String> uncompressedExtensions */
         false, /* crunchPng */
         ImmutableList.<String>of(), /* List<String> densities */
         false, /* incremental */
+        new AndroidResourceContainerBuilder()
+            .withData(data)
+            .withManifest(getManifest())
+            .withROutput(rTxt)
+            .withSymbolsFile(symbolsTxt)
+            .useJavaPackageFromManifest(true)
+            .buildFromRule(ruleContext, null),
         data,
         null, /* Artifact proguardCfg */
         null, /* Artifact mainDexProguardCfg */
-        ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_PROCESSED_MANIFEST),
-        ruleContext.getImplicitOutputArtifact(AndroidRuleClasses.ANDROID_RESOURCES_ZIP));
+        manifestOut,
+        mergedResources);
   }
 
   /** Packages up the manifest with resource and assets from the rule and dependent resources. */
@@ -370,17 +381,20 @@ public final class ApplicationManifest {
       return null;
     }
     return createApk(
-        resourceApk,
         ruleContext,
         isLibrary,
         resourceDeps,
-        rTxt,
-        symbolsTxt,
         configurationFilters,
         uncompressedExtensions,
         crunchPng,
         densities,
         incremental,
+        new AndroidResourceContainerBuilder()
+            .withData(data)
+            .withManifest(getManifest())
+            .withROutput(rTxt)
+            .withSymbolsFile(symbolsTxt)
+            .buildFromRule(ruleContext, resourceApk),
         data,
         proguardCfg,
         mainDexProguardCfg,
@@ -389,29 +403,22 @@ public final class ApplicationManifest {
   }
 
   private ResourceApk createApk(
-      @Nullable Artifact resourceApk,
       RuleContext ruleContext,
       boolean isLibrary,
       ResourceDependencies resourceDeps,
-      Artifact rTxt,
-      Artifact symbolsTxt,
       List<String> configurationFilters,
       List<String> uncompressedExtensions,
       boolean crunchPng,
       List<String> densities,
       boolean incremental,
+      ResourceContainer maybeInlinedResourceContainer,
       LocalResourceContainer data,
       Artifact proguardCfg,
       @Nullable Artifact mainDexProguardCfg,
       Artifact manifestOut,
       Artifact mergedResources) throws InterruptedException {
     ResourceContainer resourceContainer = checkForInlinedResources(
-        new AndroidResourceContainerBuilder()
-            .withData(data)
-            .withManifest(getManifest())
-            .withROutput(rTxt)
-            .withSymbolsFile(symbolsTxt)
-            .buildFromRule(ruleContext, resourceApk),
+        maybeInlinedResourceContainer,
         resourceDeps.getResources(),  // TODO(bazel-team): Figure out if we really need to check
         // the ENTIRE transitive closure, or just the direct dependencies. Given that each rule with
         // resources would check for inline resources, we can rely on the previous rule to have
@@ -485,7 +492,7 @@ public final class ApplicationManifest {
     }
 
     return new ResourceApk(
-        resourceApk, processed.getJavaSourceJar(), processed.getJavaClassJar(),
+        resourceContainer.getApk(), processed.getJavaSourceJar(), processed.getJavaClassJar(),
         resourceDeps, processed, processed.getManifest(),
         proguardCfg, mainDexProguardCfg, false);
   }