Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read android resource package from the manifest instead of using the AndroidIdeInfo.java_package provider API. #5849

Open
wants to merge 1 commit into
base: google
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 27 additions & 10 deletions aspect/build_dependencies.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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 [],
)]
Expand Down Expand Up @@ -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",
Expand All @@ -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(),
Expand All @@ -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,
)
Expand All @@ -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))
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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(
Expand All @@ -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 = {}
Expand All @@ -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):
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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,
),
]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions querysync/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
9 changes: 9 additions & 0 deletions querysync/java/com/google/idea/blaze/qsync/artifacts/BUILD
Original file line number Diff line number Diff line change
@@ -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"]),
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright 2023 The Bazel Authors. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Loading