Support adding additional raw xml files to R8 resource shrinking
To ensure that we can parse all xml keep rules we introduce a new Kind for passing in rules. Rules are explicitly not output again, and only used for extracting keep rules.
This will ensure that we are compatible with the old resource shrinker
and additionally the setup can ensure that we will not hit cases where
aapt have removed duplicates (AGP can pass all files from the
individual res folders, even when there are name clashes)
Bug: b/287398085
Change-Id: I167c75e3b244d96870524e8a67141a22dd463d68
diff --git a/src/main/java/com/android/tools/r8/AndroidResourceInput.java b/src/main/java/com/android/tools/r8/AndroidResourceInput.java
index 9403c75..dc2602e 100644
--- a/src/main/java/com/android/tools/r8/AndroidResourceInput.java
+++ b/src/main/java/com/android/tools/r8/AndroidResourceInput.java
@@ -23,11 +23,13 @@
MANIFEST,
// The resource table, in proto format.
RESOURCE_TABLE,
- // An xml file within the res folder, in proto format if not inside res/raw, otherwise
- // in UTF-8 format.
+ // An xml file within the res folder, in proto format if not inside res/raw.
XML_FILE,
// Any other binary file withing the res folder.
RES_FOLDER_FILE,
+ // A xml file in text format which should be parsed for keep/discard rules. The files is not
+ // required to include rules. The files will NOT be copied to the output provider.
+ KEEP_RULE_FILE,
// Other files are ignored, but copied through
UNKNOWN
}
diff --git a/src/main/java/com/android/tools/r8/ArchiveProtoAndroidResourceProvider.java b/src/main/java/com/android/tools/r8/ArchiveProtoAndroidResourceProvider.java
index 7af6e19..e67c735 100644
--- a/src/main/java/com/android/tools/r8/ArchiveProtoAndroidResourceProvider.java
+++ b/src/main/java/com/android/tools/r8/ArchiveProtoAndroidResourceProvider.java
@@ -35,6 +35,7 @@
private static final String MANIFEST_NAME = "AndroidManifest.xml";
private static final String RESOURCE_TABLE = "resources.pb";
private static final String RES_FOLDER = "res/";
+ private static final String RES_RAW_FOLDER = RES_FOLDER + "raw/";
private static final String XML_SUFFIX = ".xml";
/**
@@ -56,13 +57,24 @@
while (entries.hasMoreElements()) {
ZipEntry entry = entries.nextElement();
String name = entry.getName();
+ Kind kind = getKindFromName(name);
ByteAndroidResourceInput resource =
new ByteAndroidResourceInput(
name,
- getKindFromName(name),
+ kind,
ByteStreams.toByteArray(zipFile.getInputStream(entry)),
new ArchiveEntryOrigin(name, origin));
resources.add(resource);
+ // We explicitly add any res/raw folder xml file also as a keep rule file.
+ if (kind == Kind.XML_FILE && name.startsWith(RES_RAW_FOLDER)) {
+ ByteAndroidResourceInput keepResource =
+ new ByteAndroidResourceInput(
+ name,
+ Kind.KEEP_RULE_FILE,
+ ByteStreams.toByteArray(zipFile.getInputStream(entry)),
+ new ArchiveEntryOrigin(name, origin));
+ resources.add(keepResource);
+ }
}
return resources;
} catch (IOException e) {
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 088b1ee..da49f2e 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -1028,6 +1028,9 @@
shrinkerResult.getResourceTableInProtoFormat(featureSplit)),
reporter);
break;
+ case KEEP_RULE_FILE:
+ // Intentionally not written
+ break;
case RES_FOLDER_FILE:
case XML_FILE:
if (toKeep.contains(androidResource.getPath().location())) {
@@ -1063,6 +1066,9 @@
case XML_FILE:
resourceShrinkerBuilder.addXmlInput(path, bytes);
break;
+ case KEEP_RULE_FILE:
+ resourceShrinkerBuilder.addKeepRuleInput(bytes);
+ break;
case UNKNOWN:
break;
}
diff --git a/src/main/java/com/android/tools/r8/utils/ResourceShrinkerUtils.java b/src/main/java/com/android/tools/r8/utils/ResourceShrinkerUtils.java
index 74fbb46..47742f0 100644
--- a/src/main/java/com/android/tools/r8/utils/ResourceShrinkerUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/ResourceShrinkerUtils.java
@@ -7,13 +7,13 @@
import com.android.build.shrinker.ShrinkerDebugReporter;
import com.android.build.shrinker.r8integration.R8ResourceShrinkerState;
import com.android.tools.r8.AndroidResourceInput;
+import com.android.tools.r8.AndroidResourceProvider;
import com.android.tools.r8.DiagnosticsHandler;
import com.android.tools.r8.FeatureSplit;
import com.android.tools.r8.ResourceException;
import com.android.tools.r8.StringConsumer;
import com.android.tools.r8.graph.AppView;
import java.io.InputStream;
-import java.util.Collection;
import java.util.function.Supplier;
public class ResourceShrinkerUtils {
@@ -28,20 +28,12 @@
if (options.resourceShrinkerConfiguration.isOptimizedShrinking()
&& options.androidResourceProvider != null) {
try {
- addResources(
- appView,
- state,
- options.androidResourceProvider.getAndroidResources(),
- FeatureSplit.BASE);
+ addResources(appView, state, options.androidResourceProvider, FeatureSplit.BASE);
if (options.hasFeatureSplitConfiguration()) {
for (FeatureSplit featureSplit :
options.getFeatureSplitConfiguration().getFeatureSplits()) {
if (featureSplit.getAndroidResourceProvider() != null) {
- addResources(
- appView,
- state,
- featureSplit.getAndroidResourceProvider().getAndroidResources(),
- featureSplit);
+ addResources(appView, state, featureSplit.getAndroidResourceProvider(), featureSplit);
}
}
}
@@ -56,10 +48,10 @@
private static void addResources(
AppView<?> appView,
R8ResourceShrinkerState state,
- Collection<AndroidResourceInput> androidResources,
+ AndroidResourceProvider androidResourceProvider,
FeatureSplit featureSplit)
throws ResourceException {
- for (AndroidResourceInput androidResource : androidResources) {
+ for (AndroidResourceInput androidResource : androidResourceProvider.getAndroidResources()) {
switch (androidResource.getKind()) {
case MANIFEST:
state.addManifestProvider(
@@ -73,6 +65,10 @@
() -> wrapThrowingInputStreamResource(appView, androidResource),
androidResource.getPath().location());
break;
+ case KEEP_RULE_FILE:
+ state.addKeepRuleRileProvider(
+ () -> wrapThrowingInputStreamResource(appView, androidResource));
+ break;
case RES_FOLDER_FILE:
state.addResFileProvider(
() -> wrapThrowingInputStreamResource(appView, androidResource),
diff --git a/src/resourceshrinker/java/com/android/build/shrinker/r8integration/LegacyResourceShrinker.java b/src/resourceshrinker/java/com/android/build/shrinker/r8integration/LegacyResourceShrinker.java
index f1ba15e..019a123 100644
--- a/src/resourceshrinker/java/com/android/build/shrinker/r8integration/LegacyResourceShrinker.java
+++ b/src/resourceshrinker/java/com/android/build/shrinker/r8integration/LegacyResourceShrinker.java
@@ -52,6 +52,7 @@
private final Map<String, byte[]> dexInputs;
private final Collection<PathAndBytes> resFolderInputs;
private final Collection<PathAndBytes> xmlInputs;
+ private final List<byte[]> keepRuleInput;
private List<String> proguardMapStrings;
private final ShrinkerDebugReporter debugReporter;
private final List<PathAndBytes> manifest;
@@ -62,6 +63,7 @@
private final Map<String, byte[]> dexInputs = new HashMap<>();
private final Map<Path, PathAndBytes> resFolderInputs = new HashMap<>();
private final Map<Path, PathAndBytes> xmlInputs = new HashMap<>();
+ private final List<byte[]> keepRuleInput = new ArrayList<>();
private final List<PathAndBytes> manifests = new ArrayList<>();
private final Map<PathAndBytes, FeatureSplit> resourceTables = new HashMap<>();
@@ -91,6 +93,11 @@
return this;
}
+ public Builder addKeepRuleInput(byte[] bytes) {
+ keepRuleInput.add(bytes);
+ return this;
+ }
+
public Builder addResFolderInput(Path path, byte[] bytes) {
PathAndBytes existing = resFolderInputs.get(path);
if (existing != null) {
@@ -119,6 +126,7 @@
manifests,
resourceTables,
xmlInputs.values(),
+ keepRuleInput,
proguardMapStrings,
debugReporter);
}
@@ -139,6 +147,7 @@
List<PathAndBytes> manifests,
Map<PathAndBytes, FeatureSplit> resourceTables,
Collection<PathAndBytes> xmlInputs,
+ List<byte[]> additionalRawXmlInputs,
List<String> proguardMapStrings,
ShrinkerDebugReporter debugReporter) {
this.dexInputs = dexInputs;
@@ -146,6 +155,7 @@
this.manifest = manifests;
this.resourceTables = resourceTables;
this.xmlInputs = xmlInputs;
+ this.keepRuleInput = additionalRawXmlInputs;
this.proguardMapStrings = proguardMapStrings;
this.debugReporter = debugReporter;
}
@@ -174,10 +184,8 @@
ProtoAndroidManifestUsageRecorderKt.recordUsagesFromNode(
XmlNode.parseFrom(pathAndBytes.bytes), model);
}
- for (PathAndBytes xmlInput : xmlInputs) {
- if (xmlInput.path.startsWith("res/raw")) {
- ToolsAttributeUsageRecorderKt.processRawXml(getUtfReader(xmlInput.getBytes()), model);
- }
+ for (byte[] keepBytes : keepRuleInput) {
+ ToolsAttributeUsageRecorderKt.processRawXml(getUtfReader(keepBytes), model);
}
ImmutableMap<String, PathAndBytes> resFolderMappings =
diff --git a/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java b/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java
index 2dd02e8..5ac9755 100644
--- a/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java
+++ b/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java
@@ -53,6 +53,7 @@
private final Function<Exception, RuntimeException> errorHandler;
private final R8ResourceShrinkerModel r8ResourceShrinkerModel;
private final Map<String, Supplier<InputStream>> xmlFileProviders = new HashMap<>();
+ private final List<Supplier<InputStream>> keepRuleFileProviders = new ArrayList<>();
private final List<Supplier<InputStream>> manifestProviders = new ArrayList<>();
private final Map<String, Supplier<InputStream>> resfileProviders = new HashMap<>();
@@ -141,6 +142,10 @@
this.xmlFileProviders.put(location, inputStreamSupplier);
}
+ public void addKeepRuleRileProvider(Supplier<InputStream> inputStreamSupplier) {
+ this.keepRuleFileProviders.add(inputStreamSupplier);
+ }
+
public void addResFileProvider(Supplier<InputStream> inputStreamSupplier, String location) {
this.resfileProviders.put(location, inputStreamSupplier);
}
@@ -308,11 +313,9 @@
}
public void updateModelWithKeepXmlReferences() throws IOException {
- for (Map.Entry<String, Supplier<InputStream>> entry : xmlFileProviders.entrySet()) {
- if (entry.getKey().startsWith("res/raw")) {
- ToolsAttributeUsageRecorderKt.processRawXml(
- getUtfReader(entry.getValue().get().readAllBytes()), r8ResourceShrinkerModel);
- }
+ for (Supplier<InputStream> keepRuleFileProvider : keepRuleFileProviders) {
+ ToolsAttributeUsageRecorderKt.processRawXml(
+ getUtfReader(keepRuleFileProvider.get().readAllBytes()), r8ResourceShrinkerModel);
}
}
diff --git a/src/test/java/com/android/tools/r8/androidresources/KeepXmlFilesTest.java b/src/test/java/com/android/tools/r8/androidresources/KeepXmlFilesTest.java
index 5d19a6a..3676fd8 100644
--- a/src/test/java/com/android/tools/r8/androidresources/KeepXmlFilesTest.java
+++ b/src/test/java/com/android/tools/r8/androidresources/KeepXmlFilesTest.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.androidresources;
+import com.android.tools.r8.R8TestBuilder;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResource;
@@ -36,6 +37,7 @@
return new AndroidTestResourceBuilder()
.withSimpleManifestAndAppNameString()
.addKeepXmlFor("@string/bar", "@drawable/foobar")
+ .addExplicitKeepRuleFileFor("@drawable/barfoo")
.addRClassInitializeWithDefaultValues(R.string.class, R.drawable.class)
.build(temp);
}
@@ -47,7 +49,7 @@
.addProgramClasses(FooBar.class)
.addAndroidResources(getTestResources(temp))
.addKeepMainRule(FooBar.class)
- .applyIf(optimized, b -> b.enableOptimizedShrinking())
+ .applyIf(optimized, R8TestBuilder::enableOptimizedShrinking)
.compile()
.inspectShrunkenResources(
resourceTableInspector -> {
@@ -57,6 +59,8 @@
resourceTableInspector.assertContainsResourceWithName("string", "foo");
// Referenced from keep.xml
resourceTableInspector.assertContainsResourceWithName("drawable", "foobar");
+ // Referenced from additional keep xml files
+ resourceTableInspector.assertContainsResourceWithName("drawable", "barfoo");
resourceTableInspector.assertDoesNotContainResourceWithName(
"string", "unused_string");
resourceTableInspector.assertDoesNotContainResourceWithName(
@@ -86,6 +90,7 @@
public static class drawable {
public static int foobar;
+ public static int barfoo;
public static int unused_drawable;
}
}
diff --git a/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java b/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java
index bf3998d..3412d54 100644
--- a/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java
+++ b/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java
@@ -47,8 +47,11 @@
import com.android.tools.r8.utils.SemanticVersion;
import com.android.tools.r8.utils.SourceFileTemplateProvider;
import com.android.tools.r8.utils.codeinspector.Matchers;
+import java.io.ByteArrayInputStream;
import java.io.IOException;
+import java.io.InputStream;
import java.lang.annotation.Annotation;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
@@ -982,15 +985,58 @@
AndroidTestResource testResource, Path output, List<byte[]> classFileData) {
Path resources = testResource.getResourceZip();
resourceShrinkerOutput = output;
- getBuilder()
- .setAndroidResourceProvider(
- new ArchiveProtoAndroidResourceProvider(resources, new PathOrigin(resources)));
+ ArchiveProtoAndroidResourceProvider provider = getResourceProvider(testResource);
+ getBuilder().setAndroidResourceProvider(provider);
getBuilder()
.setAndroidResourceConsumer(new ArchiveProtoAndroidResourceConsumer(output, resources));
return addProgramClassFileData(classFileData);
}
+ private static ArchiveProtoAndroidResourceProvider getResourceProvider(
+ AndroidTestResource testResource) {
+ Path resources = testResource.getResourceZip();
+ if (testResource.getAdditionalKeepRuleFiles().isEmpty()) {
+ return new ArchiveProtoAndroidResourceProvider(resources, new PathOrigin(resources));
+ }
+ ArchiveProtoAndroidResourceProvider provider =
+ new ArchiveProtoAndroidResourceProvider(resources, new PathOrigin(resources)) {
+ @Override
+ public Collection<AndroidResourceInput> getAndroidResources() throws ResourceException {
+ ArrayList<AndroidResourceInput> resourceInputs =
+ new ArrayList<>(super.getAndroidResources());
+ resourceInputs.addAll(
+ testResource.getAdditionalKeepRuleFiles().stream()
+ .map(
+ s ->
+ new AndroidResourceInput() {
+ @Override
+ public Origin getOrigin() {
+ return Origin.unknown();
+ }
+
+ @Override
+ public ResourcePath getPath() {
+ return () -> "keep/rule/path";
+ }
+
+ @Override
+ public Kind getKind() {
+ return Kind.KEEP_RULE_FILE;
+ }
+
+ @Override
+ public InputStream getByteStream() throws ResourceException {
+ return new ByteArrayInputStream(s.getBytes(StandardCharsets.UTF_8));
+ }
+ })
+ .collect(Collectors.toList()));
+ return resourceInputs;
+ }
+ };
+ return provider;
+ }
+
public T setAndroidResourcesFromPath(Path input) throws IOException {
return setAndroidResourcesFromPath(
input, getState().getNewTempFile("resourceshrinkeroutput.zip"));
diff --git a/src/test/testbase/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java b/src/test/testbase/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java
index 80333c3..2f9a852 100644
--- a/src/test/testbase/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java
+++ b/src/test/testbase/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java
@@ -173,10 +173,13 @@
public static class AndroidTestResource {
private final AndroidTestRClass rClass;
private final Path resourceZip;
+ private final List<String> additionalKeepRuleFiles;
- AndroidTestResource(AndroidTestRClass rClass, Path resourceZip) {
+ AndroidTestResource(
+ AndroidTestRClass rClass, Path resourceZip, List<String> additionalRawXmlFiles) {
this.rClass = rClass;
this.resourceZip = resourceZip;
+ this.additionalKeepRuleFiles = additionalRawXmlFiles;
}
public AndroidTestRClass getRClass() {
@@ -186,6 +189,10 @@
public Path getResourceZip() {
return resourceZip;
}
+
+ public List<String> getAdditionalKeepRuleFiles() {
+ return additionalKeepRuleFiles;
+ }
}
// Easy traversable resource table.
@@ -296,6 +303,7 @@
// Used for xml files that we hard code to -v24
private final Map<String, String> apiSpecificXmlFiles = new TreeMap<>();
private final Map<String, String> rawXmlFiles = new TreeMap<>();
+ private final List<String> keepRuleFiles = new ArrayList<>();
private final List<Class<?>> classesToRemap = new ArrayList<>();
private int packageId = 0x7f;
private String packageName;
@@ -379,7 +387,16 @@
}
public AndroidTestResourceBuilder addKeepXmlFor(String... resourceReferences) {
- addRawXml("keep.xml", String.format(KEEP_XML, String.join(",", resourceReferences)));
+ addRawXml("keep.xml", getKeepXmlContent(resourceReferences));
+ return this;
+ }
+
+ private static String getKeepXmlContent(String[] resourceReferences) {
+ return String.format(KEEP_XML, String.join(",", resourceReferences));
+ }
+
+ public AndroidTestResourceBuilder addExplicitKeepRuleFileFor(String... resourceReferences) {
+ keepRuleFiles.add(getKeepXmlContent(resourceReferences));
return this;
}
@@ -546,7 +563,7 @@
}
});
return new AndroidTestResource(
- new AndroidTestRClass(rClassJavaFile, rewrittenRClassFiles), output);
+ new AndroidTestRClass(rClassJavaFile, rewrittenRClassFiles), output, keepRuleFiles);
}
private String createOverlayableXml() {
