Read android resource package from the manifest instead of using the …

…`AndroidIdeInfo.java_package` provider API.

Since the API `AndroidIdeInfo.java_package` cannot always know the actual package that the `R` class is generated in, switch to reading that informatiob from the manifest file. We rely on `AndroidIdeInfo` to provide us with the manifest file location and then parse the package name from that in the IDE.

PiperOrigin-RevId: 590919016

aspect/build_dependencies.bzl

@@ -24,6 +24,7 @@ def _package_dependencies_impl(target, ctx):
         artifact_info_file = java_info_file,
         qsync_aars = target[DependenciesInfo].aars.to_list(),
         qsync_gensrcs = target[DependenciesInfo].gensrcs.to_list(),
+        qsync_android_manifests = target[DependenciesInfo].android_manifest_files.to_list(),
         cc_headers = target[DependenciesInfo].cc_headers.to_list(),
         cc_info_file = cc_info_file + [target[DependenciesInfo].cc_toolchain_info.file] if target[DependenciesInfo].cc_toolchain_info else [],
     )]
@@ -57,6 +58,7 @@ DependenciesInfo = provider(
         "target_to_artifacts": "a map between a target and all its artifacts",
         "aars": "a list of aars with resource files",
         "gensrcs": "a list of sources generated by project targets",
+        "android_manifest_files": "a list of files containing android resources package names",
         "expand_sources": "boolean, true if the sources for this target should be expanded when it appears inside another rules srcs list",
         "cc_info": "a structure containing info required to compile cc sources",
         "cc_headers": "a depset of generated headers required to compile cc sources",
@@ -71,6 +73,7 @@ def create_dependencies_info(
         target_to_artifacts = {},
         aars = depset(),
         gensrcs = depset(),
+        android_manifest_files = depset(),
         expand_sources = False,
         cc_info = None,
         cc_headers = depset(),
@@ -87,6 +90,7 @@ def create_dependencies_info(
         cc_info = cc_info,
         cc_headers = cc_headers,
         cc_toolchain_info = cc_toolchain_info,
+        android_manifest_files = android_manifest_files,
         test_mode_own_files = test_mode_own_files,
         test_mode_cc_src_deps = test_mode_cc_src_deps,
     )
@@ -102,7 +106,7 @@ def _encode_target_info_proto(target_to_artifacts):
                 gen_srcs = target_info["gen_srcs"],
                 srcs = target_info["srcs"],
                 srcjars = target_info["srcjars"],
-                android_resources_package = target_info["android_resources_package"],
+                android_manifest_file = target_info["android_manifest_file"],
             ),
         )
     return proto.encode_text(struct(artifacts = contents))
@@ -234,7 +238,7 @@ def _collect_own_java_artifacts(
     own_gensrc_files = []
     own_src_files = []
     own_srcjar_files = []
-    resource_package = None
+    own_android_manifest = None
 
     if must_build_main_artifacts:
         # For rules that we do not follow dependencies of (either because they don't
@@ -259,7 +263,17 @@ def _collect_own_java_artifacts(
 
     else:
         if AndroidIdeInfo in target:
-            resource_package = target[AndroidIdeInfo].java_package
+            android_sdk_info = None
+            if hasattr(rule.attr, "_android_sdk"):
+                android_sdk_info = rule.attr._android_sdk[AndroidSdkInfo]
+
+            # Export the manifest file to the IDE can read the package name from it
+            # Note that while AndroidIdeInfo has a `java_package` API, it cannot always
+            # determine the package name, since the it may only appear inside the
+            # AndroidManifest.xml file which cannot be read by an aspect. So instead of
+            # using that, we extract the use the manifest directly
+            if target[AndroidIdeInfo].defines_android_resources and android_sdk_info:
+                own_android_manifest = target[AndroidIdeInfo].manifest
 
             if generate_aidl_classes:
                 add_base_idl_jar = False
@@ -274,8 +288,7 @@ def _collect_own_java_artifacts(
                     add_base_idl_jar = True
 
                 # An AIDL base jar needed for resolving base classes for aidl generated stubs.
-                if add_base_idl_jar and hasattr(rule.attr, "_android_sdk"):
-                    android_sdk_info = getattr(rule.attr, "_android_sdk")[AndroidSdkInfo]
+                if add_base_idl_jar and android_sdk_info:
                     own_jar_depsets.append(android_sdk_info.aidl_lib.files)
 
         # Add generated java_outputs (e.g. from annotation processing)
@@ -331,7 +344,7 @@ def _collect_own_java_artifacts(
         gensrcs = own_gensrc_files,
         srcs = own_src_files,
         srcjars = own_srcjar_files,
-        android_resources_package = resource_package,
+        android_manifest_file = own_android_manifest,
     )
 
 def _collect_own_and_dependency_java_artifacts(
@@ -357,7 +370,7 @@ def _collect_own_and_dependency_java_artifacts(
         len(own_files.gensrcs) +
         len(own_files.srcs) +
         len(own_files.srcjars) +
-        (1 if own_files.android_resources_package else 0)
+        (1 if own_files.android_manifest_file else 0)
     ) > 0
 
     target_to_artifacts = {}
@@ -373,24 +386,26 @@ def _collect_own_and_dependency_java_artifacts(
             "gen_srcs": [_output_relative_path(file.path) for file in gen_srcs],
             "srcs": own_files.srcs,
             "srcjars": own_files.srcjars,
-            "android_resources_package": own_files.android_resources_package,
+            "android_manifest_file": own_files.android_manifest_file.path if own_files.android_manifest_file else None,
         }
 
     own_and_transitive_jar_depsets = list(own_files.jar_depsets)  # Copy to prevent changes to own_jar_depsets.
     own_and_transitive_ide_aar_depsets = []
     own_and_transitive_gensrc_depsets = []
+    own_and_transitive_android_manifest_files = []
 
     for info in dependency_infos.values():
         target_to_artifacts.update(info.target_to_artifacts)
         own_and_transitive_jar_depsets.append(info.compile_time_jars)
         own_and_transitive_ide_aar_depsets.append(info.aars)
-        own_and_transitive_gensrc_depsets.append(info.gensrcs)
+        own_and_transitive_android_manifest_files.append(info.android_manifest_files)
 
     return (
         target_to_artifacts,
         depset(own_files.jars, transitive = own_and_transitive_jar_depsets),
         depset(own_files.ide_aars, transitive = own_and_transitive_ide_aar_depsets),
         depset(own_files.gensrcs, transitive = own_and_transitive_gensrc_depsets),
+        depset([own_files.android_manifest_file] if own_files.android_manifest_file else [], transitive = own_and_transitive_android_manifest_files),
     )
 
 def _get_followed_cc_dependency_info(rule):
@@ -470,7 +485,7 @@ def _collect_java_dependencies_core_impl(
     target_is_within_project_scope = _target_within_project_scope(str(target.label), include, exclude) and not test_mode
     dependency_infos = _get_followed_java_dependency_infos(ctx.rule)
 
-    target_to_artifacts, compile_jars, aars, gensrcs = _collect_own_and_dependency_java_artifacts(
+    target_to_artifacts, compile_jars, aars, gensrcs, android_manifest_files = _collect_own_and_dependency_java_artifacts(
         target,
         ctx,
         dependency_infos,
@@ -493,6 +508,7 @@ def _collect_java_dependencies_core_impl(
             test_mode_within_scope_own_jar_files = depset(within_scope_own_files.jars, transitive = within_scope_own_files.jar_depsets).to_list(),
             test_mode_within_scope_own_ide_aar_files = within_scope_own_files.ide_aars,
             test_mode_within_scope_own_gensrc_files = within_scope_own_files.gensrcs,
+            test_mode_within_scope_own_android_pname_files = within_scope_own_files.android_manifest_file,
         )
 
     expand_sources = False
@@ -508,6 +524,7 @@ def _collect_java_dependencies_core_impl(
             gensrcs = gensrcs,
             expand_sources = expand_sources,
             test_mode_own_files = test_mode_own_files,
+            android_manifest_files = android_manifest_files,
         ),
     ]
 

base/src/com/google/idea/blaze/base/qsync/BazelDependencyBuilder.java

@@ -121,6 +121,7 @@ public BazelDependencyBuilder(
               OutputGroup.JARS,
               OutputGroup.AARS,
               OutputGroup.GENSRCS,
+              OutputGroup.ANDROID_MANIFESTS,
               OutputGroup.ARTIFACT_INFO_FILE)
           .putAll(QuerySyncLanguage.CC, OutputGroup.CC_HEADERS, OutputGroup.CC_INFO_FILE)
           .build();

base/src/com/google/idea/blaze/base/qsync/OutputGroup.java

@@ -20,6 +20,7 @@ public enum OutputGroup {
   JARS("qsync_jars"),
   AARS("qsync_aars"),
   GENSRCS("qsync_gensrcs"),
+  ANDROID_MANIFESTS("qsync_android_manifests"),
   ARTIFACT_INFO_FILE("artifact_info_file"),
   CC_HEADERS("cc_headers"),
   CC_INFO_FILE("cc_info_file");

base/src/com/google/idea/blaze/base/qsync/cache/ArtifactTrackerImpl.java

@@ -186,6 +186,8 @@ public class ArtifactTrackerImpl
   private final FileCache generatedHeadersCache;
   private final Path appInspectorCacheDirectory;
   private final FileCache appInspectorCache;
+  private final Path androidManifestCacheDirectory;
+  private final FileCache androidManifestCache;
   private final Path persistentFile;
   private final Path ideProjectBasePath;
 
@@ -228,6 +230,10 @@ public ArtifactTrackerImpl(
     appInspectorCache =
         fileCacheCreator.createFileCache(
             new DefaultCacheLayout(appInspectorCacheDirectory, ImmutableSet.of("aar")));
+    androidManifestCacheDirectory = projectDirectory.resolve("androidmanifests");
+    androidManifestCache =
+        fileCacheCreator.createFileCache(
+            new ArtifactPathCacheLayout(androidManifestCacheDirectory));
     cacheDirectoryManager =
         new CacheDirectoryManager(
             projectDirectory.resolve(DIGESTS_DIRECTORY_NAME),
@@ -556,6 +562,9 @@ private ImmutableMap<OutputArtifact, OutputArtifactDestinationAndLayout> outputI
         .putAll(
             generatedHeadersCache.prepareDestinationPathsAndDirectories(
                 outputInfo.get(OutputGroup.CC_HEADERS)))
+        .putAll(
+            androidManifestCache.prepareDestinationPathsAndDirectories(
+                outputInfo.get(OutputGroup.ANDROID_MANIFESTS)))
         .buildOrThrow();
   }
 
@@ -714,7 +723,8 @@ private ProjectProto.Project updateProjectProtoForJavaDeps(ProjectProto.Project
 
     if (QuerySync.EXTRACT_RES_PACKAGES_AT_BUILD_TIME.getValue()) {
       AndroidResPackagesProjectUpdater resPackagesUpdater =
-          new AndroidResPackagesProjectUpdater(projectProto, javaArtifacts.values());
+          new AndroidResPackagesProjectUpdater(
+              projectProto, javaArtifacts.values(), androidManifestCache, projectPathResolver);
       projectProto = resPackagesUpdater.addAndroidResPackages();
     }
 

base/src/com/google/idea/blaze/base/qsync/cache/FileCache.java

@@ -22,6 +22,7 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.idea.blaze.base.command.buildresult.OutputArtifact;
 import com.google.idea.blaze.base.command.buildresult.OutputArtifactInfo;
+import com.google.idea.blaze.qsync.artifacts.BuildArtifactProvider;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -32,7 +33,7 @@
 
 /** Local cache of the .jar, .aars and other artifacts referenced by the project. */
 @SuppressWarnings("InvalidBlockTag")
-public class FileCache {
+public class FileCache implements BuildArtifactProvider {
 
   /**
    * An interface that defines the layout of an IDE artifact cache directory.
@@ -120,6 +121,11 @@ public Optional<Path> getCacheFile(Path artifactPath) {
     return Optional.ofNullable(Files.exists(path) ? path : null);
   }
 
+  @Override
+  public Optional<Path> getCachedArtifact(Path buildOutputPathRelativePath) {
+    return getCacheFile(buildOutputPathRelativePath);
+  }
+
   /**
    * Builds a map describing where artifact files should be copied to and where their content should
    * be extracted to.

querysync/BUILD

@@ -14,6 +14,7 @@ java_library(
     name = "querysync",
     exports = [
         "//querysync/java/com/google/idea/blaze/qsync",
+        "//querysync/java/com/google/idea/blaze/qsync/artifacts",
         "//querysync/java/com/google/idea/blaze/qsync/cc",
         "//querysync/java/com/google/idea/blaze/qsync/cc:cc_compilation_info_java_proto",
         "//querysync/java/com/google/idea/blaze/qsync/java",

querysync/java/com/google/idea/blaze/qsync/artifacts/BUILD

@@ -0,0 +1,9 @@
+package(default_visibility = [
+    "//javatests/com/google/devtools/intellij/blaze/plugin/aswb:__pkg__",
+    "//querysync:__subpackages__",
+])
+
+java_library(
+    name = "artifacts",
+    srcs = glob(["*.java"]),
+)

querysync/java/com/google/idea/blaze/qsync/artifacts/BuildArtifactProvider.java

@@ -0,0 +1,16 @@
+package com.google.idea.blaze.qsync.artifacts;
+
+import java.nio.file.Path;
+import java.util.Optional;
+
+/**
+ * An interface to provide absolute paths for build artifacts specified as paths relative the the
+ * build output path.
+ *
+ * <p>The build output path is that reported by a {@code bazel info} invocation with key {@code
+ * output_path}.
+ */
+public interface BuildArtifactProvider {
+
+  Optional<Path> getCachedArtifact(Path buildOutputPathRelativePath);
+}

querysync/java/com/google/idea/blaze/qsync/java/AndroidResPackagesProjectUpdater.java

@@ -20,27 +20,106 @@
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
+import com.google.common.io.Closer;
+import com.google.idea.blaze.qsync.artifacts.BuildArtifactProvider;
+import com.google.idea.blaze.qsync.project.ProjectPath;
 import com.google.idea.blaze.qsync.project.ProjectProto.Project;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Optional;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import javax.xml.namespace.QName;
+import javax.xml.stream.XMLEventReader;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.events.Attribute;
+import javax.xml.stream.events.StartElement;
+import javax.xml.stream.events.XMLEvent;
 
 /**
  * Updates the project proto with the android resources packages extracted by the aspect in a
  * dependencies build.
  */
 public class AndroidResPackagesProjectUpdater {
 
+  private final Logger logger =
+      Logger.getLogger(AndroidResPackagesProjectUpdater.class.getSimpleName());
+
   private final Project project;
   private final ImmutableList<JavaArtifactInfo> javaArtifacts;
+  private final BuildArtifactProvider artifactProvider;
+  private final ProjectPath.Resolver pathResolver;
 
   public AndroidResPackagesProjectUpdater(
-      Project project, Iterable<JavaArtifactInfo> javaArtifacts) {
+      Project project,
+      Iterable<JavaArtifactInfo> javaArtifacts,
+      BuildArtifactProvider artifactProvider,
+      ProjectPath.Resolver pathResolver) {
     this.project = project;
     this.javaArtifacts = ImmutableList.copyOf(javaArtifacts);
+    this.artifactProvider = artifactProvider;
+    this.pathResolver = pathResolver;
+  }
+
+  /**
+   * Resolves a path from a Java target info proto message into an absolute path.
+   *
+   * @param relative The relative path. This may refer to a build artifact, in which case it will
+   *     start with {@code (blaze|bazel)-out}, or a workspace relative source path.
+   * @return An absolute path for the source file or build artifact.
+   */
+  private Optional<Path> resolveManifestPath(Path relative) {
+    if (relative.getName(0).toString().endsWith("-out")) {
+      return artifactProvider.getCachedArtifact(relative.subpath(1, relative.getNameCount()));
+    } else {
+      return Optional.of(pathResolver.resolve(ProjectPath.workspaceRelative(relative)));
+    }
+  }
+
+  private String readPackageFromManifest(Path manifestPath) {
+    return resolveManifestPath(manifestPath).map(this::parseProjectFromXmlFile).orElse("");
+  }
+
+  private String parseProjectFromXmlFile(Path absolutePath) {
+    XMLInputFactory xmlInputFactory = XMLInputFactory.newInstance();
+    try (Closer closer = Closer.create()) {
+      XMLEventReader reader =
+          xmlInputFactory.createXMLEventReader(Files.newInputStream(absolutePath));
+      closer.register(
+          () -> {
+            try {
+              reader.close();
+            } catch (XMLStreamException e) {
+              throw new IOException(e);
+            }
+          });
+      while (reader.hasNext()) {
+        XMLEvent event = reader.nextEvent();
+        if (event.isStartElement()) {
+          StartElement startElement = event.asStartElement();
+          if (startElement.getName().getLocalPart().equals("manifest")) {
+            Attribute pname = startElement.getAttributeByName(new QName("package"));
+            if (pname == null) {
+              return "";
+            }
+            return pname.getValue();
+          }
+        }
+      }
+    } catch (IOException | XMLStreamException e) {
+      logger.log(Level.WARNING, "Failed to read package name from " + absolutePath, e);
+    }
+    return "";
   }
 
   public Project addAndroidResPackages() {
     ImmutableList<String> packages =
         javaArtifacts.stream()
-            .map(JavaArtifactInfo::androidResourcesPackage)
+            .map(JavaArtifactInfo::androidManifestFile)
+            .filter(p -> !p.toString().isEmpty())
+            .map(this::readPackageFromManifest)
             .filter(not(Strings::isNullOrEmpty))
             .distinct()
             .collect(ImmutableList.toImmutableList());

querysync/java/com/google/idea/blaze/qsync/java/BUILD

@@ -21,6 +21,7 @@ java_library(
     srcs = glob(["*.java"]),
     deps = [
         ":java_target_info_java_proto",
+        "//querysync/java/com/google/idea/blaze/qsync/artifacts",
         "//querysync/java/com/google/idea/blaze/qsync/project",
         "//querysync/java/com/google/idea/blaze/qsync/project:project_java_proto",
         "//shared",