Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Use package and resource name when computing referenced resource names in dex #830

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/com/facebook/buck/android/TrimUberRDotJava.java
Expand Up @@ -58,6 +58,9 @@ class TrimUberRDotJava extends AbstractBuildRule {
private static final Pattern R_DOT_JAVA_LINE_PATTERN = Pattern.compile(
"^ *public static final int(?:\\[\\])? (\\w+)=");

private static final Pattern R_DOT_JAVA_PACKAGE_NAME_PATTERN = Pattern.compile(
"^ *package ([\\w.]+);");

TrimUberRDotJava(
BuildRuleParams buildRuleParams,
SourcePathResolver resolver,
Expand Down Expand Up @@ -176,12 +179,22 @@ private static void filterRDotJava(
OutputStream output,
ImmutableSet<String> allReferencedResources)
throws IOException {
String packageName = null;
Matcher m;
for (String line : rDotJavaLines) {
Matcher m = R_DOT_JAVA_LINE_PATTERN.matcher(line);
// We ignore the containing nested class and just match on the resource name.
if (packageName == null) {
m = R_DOT_JAVA_PACKAGE_NAME_PATTERN.matcher(line);
if (m.find()) {
packageName = m.group(1);
} else {
continue;
}
}
m = R_DOT_JAVA_LINE_PATTERN.matcher(line);
// We match on the package name + resource name.
// This can cause us to keep (for example) R.layout.foo when only R.string.foo
// is referenced. That is a very rare case, though, and not worth the complexity to fix.
if (m.find() && !allReferencedResources.contains(m.group(1))) {
if (m.find() && !allReferencedResources.contains(packageName + "." + m.group(1))) {
continue;
}
output.write(line.getBytes(Charsets.UTF_8));
Expand Down
Expand Up @@ -243,7 +243,7 @@ public void testDxFindsReferencedResources() throws IOException {
buildInfo.getValues(DexProducedFromJavaLibrary.REFERENCED_RESOURCES);
assertTrue(resourcesFromMetadata.isPresent());
assertEquals(
ImmutableSet.of("title", "top_layout"),
ImmutableSet.of("com.sample.top_layout", "com.sample2.title"),
ImmutableSet.copyOf(resourcesFromMetadata.get()));
}

Expand Down
2 changes: 1 addition & 1 deletion test/com/facebook/buck/android/TrimUberRDotJavaTest.java
Expand Up @@ -99,7 +99,7 @@ public void testTrimming() throws IOException, InterruptedException {
.putMetadata(DexProducedFromJavaLibrary.CLASSNAMES_TO_HASHES, "{}")
.putMetadata(
DexProducedFromJavaLibrary.REFERENCED_RESOURCES,
ImmutableList.of("my_first_resource"))));
ImmutableList.of("com.test.my_first_resource"))));

TrimUberRDotJava trimUberRDotJava = new TrimUberRDotJava(
new FakeBuildRuleParamsBuilder(BuildTargetFactory.newInstance("//:trim"))
Expand Down
Expand Up @@ -1654,8 +1654,8 @@ private void computeReferencedResources() {
CstType fieldClass = item.getDefiningClass();
CstString fieldName = item.getRef().getNat().getName();
if (fieldClass.getClassType().getDescriptor().contains("/R$")) {
// We ignore the name of the containing class for simplicity.
resourceNames.add(fieldName.getString());
// Add the packageName of the class for better accuracy.
resourceNames.add(fieldClass.getPackageName() + "." + fieldName.getString());
}
}
}
Expand Down