Add easier control of whether data resources are processed or not by R8
This is only relevant for R8. D8 will always ignore data resources.
When the current BaseCompilerCommand.setOutput API is used to specify
the output, there is no way of controlling if data resources are
processed or not. Before this change the consumers created when this
API was used would ignore data resources for R8 as well. The only way
to process data resources with R8 was to use a custom consumer through
the API.
This adds an additional setOutput method to the R8Command API which
takes a boolean argument to control if data resources are processed or
not.
The default when the original setOutput is used is now to process data
resources for R8.
Added the option --no-data-resources to the command line to be able to
override this default behaviour.
Change-Id: I87d30c99133bfc365ec30eaacfa1c0755acf38e1
Bug:
diff --git a/src/main/java/com/android/tools/r8/BaseCompilerCommand.java b/src/main/java/com/android/tools/r8/BaseCompilerCommand.java
index b57ff61..6ae611c 100644
--- a/src/main/java/com/android/tools/r8/BaseCompilerCommand.java
+++ b/src/main/java/com/android/tools/r8/BaseCompilerCommand.java
@@ -235,11 +235,16 @@
* @param outputMode Mode in which to write the output.
*/
public B setOutput(Path outputPath, OutputMode outputMode) {
+ return setOutput(outputPath, outputMode, false);
+ }
+
+ // This is only public in R8Command.
+ protected B setOutput(Path outputPath, OutputMode outputMode, boolean includeDataResources) {
assert outputPath != null;
assert outputMode != null;
this.outputPath = outputPath;
this.outputMode = outputMode;
- programConsumer = createProgramOutputConsumer(outputPath, outputMode, false);
+ programConsumer = createProgramOutputConsumer(outputPath, outputMode, includeDataResources);
return self();
}
diff --git a/src/main/java/com/android/tools/r8/R8Command.java b/src/main/java/com/android/tools/r8/R8Command.java
index 5195a82..c53f367 100644
--- a/src/main/java/com/android/tools/r8/R8Command.java
+++ b/src/main/java/com/android/tools/r8/R8Command.java
@@ -239,6 +239,47 @@
return self();
}
+ /**
+ * Set the output path-and-mode.
+ *
+ * <p>Setting the output path-and-mode will override any previous set consumer or any previous
+ * output path-and-mode, and implicitly sets the appropriate program consumer to write the
+ * output.
+ *
+ * <p>By default data resources from the input will be included in the output. (see {@link
+ * #setOutput(Path, OutputMode, boolean) for details}
+ *
+ * @param outputPath Path to write the output to. Must be an archive or and existing directory.
+ * @param outputMode Mode in which to write the output.
+ */
+ @Override
+ public Builder setOutput(Path outputPath, OutputMode outputMode) {
+ setOutput(outputPath, outputMode, true);
+ return self();
+ }
+
+ /**
+ * Set the output path-and-mode and control if data resources are included.
+ *
+ * <p>In addition to setting the output path-and-mode (see {@link #setOutput(Path, OutputMode)})
+ * this can control if data resources should be included or not.
+ *
+ * <p>Data resources are non Java classfile items in the input.
+ *
+ * <p>If data resources are not included they are ignored in the input and will not produce
+ * anything in the output. If data resources are included they are processed according to the
+ * configuration and written to the output.
+ *
+ * @param outputPath Path to write the output to. Must be an archive or and existing directory.
+ * @param outputMode Mode in which to write the output.
+ * @param includeDataResources If data resources from the input should be included in the
+ * output.
+ */
+ @Override
+ public Builder setOutput(Path outputPath, OutputMode outputMode, boolean includeDataResources) {
+ return super.setOutput(outputPath, outputMode, includeDataResources);
+ }
+
@Override
public Builder addProgramResourceProvider(ProgramResourceProvider programProvider) {
return super.addProgramResourceProvider(
@@ -250,7 +291,7 @@
Path path,
OutputMode mode,
boolean consumeDataResources) {
- return super.createProgramOutputConsumer(path, mode, false);
+ return super.createProgramOutputConsumer(path, mode, consumeDataResources);
}
@Override
diff --git a/src/main/java/com/android/tools/r8/R8CommandParser.java b/src/main/java/com/android/tools/r8/R8CommandParser.java
index 3d33037..580b068 100644
--- a/src/main/java/com/android/tools/r8/R8CommandParser.java
+++ b/src/main/java/com/android/tools/r8/R8CommandParser.java
@@ -27,6 +27,7 @@
OutputMode outputMode = null;
Path outputPath = null;
boolean hasDefinedApiLevel = false;
+ private boolean includeDataResources = true;
}
static final String USAGE_MESSAGE =
@@ -49,6 +50,7 @@
" --pg-map-output <file> # Output the resulting name and line mapping to <file>.",
" --no-tree-shaking # Force disable tree shaking of unreachable classes.",
" --no-minification # Force disable minification of names.",
+ " --no-data-resources # Ignore all data resources.",
" --no-desugaring # Force disable desugaring.",
" --main-dex-rules <file> # Proguard keep rules for classes to place in the",
" # primary dex file.",
@@ -91,7 +93,7 @@
}
Path outputPath = state.outputPath != null ? state.outputPath : Paths.get(".");
OutputMode outputMode = state.outputMode != null ? state.outputMode : OutputMode.DexIndexed;
- builder.setOutput(outputPath, outputMode);
+ builder.setOutput(outputPath, outputMode, state.includeDataResources);
return builder;
}
@@ -175,6 +177,8 @@
builder.addProguardConfigurationFiles(Paths.get(expandedArgs[++i]));
} else if (arg.equals("--pg-map-output")) {
builder.setProguardMapOutputPath(Paths.get(expandedArgs[++i]));
+ } else if (arg.equals("--no-data-resources")) {
+ state.includeDataResources = false;
} else {
if (arg.startsWith("--")) {
builder.error(new StringDiagnostic("Unknown option: " + arg, argsOrigin));
diff --git a/src/main/java/com/android/tools/r8/compatproguard/CompatProguard.java b/src/main/java/com/android/tools/r8/compatproguard/CompatProguard.java
index dad3120..ec15120 100644
--- a/src/main/java/com/android/tools/r8/compatproguard/CompatProguard.java
+++ b/src/main/java/com/android/tools/r8/compatproguard/CompatProguard.java
@@ -34,6 +34,7 @@
public final String output;
public final int minApi;
public final boolean forceProguardCompatibility;
+ public final boolean includeDataResources;
public final boolean multiDex;
public final String mainDexList;
public final List<String> proguardConfig;
@@ -48,12 +49,14 @@
int minApi,
boolean multiDex,
boolean forceProguardCompatibility,
+ boolean includeDataResources,
String mainDexList,
boolean printHelpAndExit,
boolean verticalClassMerging) {
this.output = output;
this.minApi = minApi;
this.forceProguardCompatibility = forceProguardCompatibility;
+ this.includeDataResources = includeDataResources;
this.multiDex = multiDex;
this.mainDexList = mainDexList;
this.proguardConfig = proguardConfig;
@@ -65,6 +68,7 @@
String output = null;
int minApi = 1;
boolean forceProguardCompatibility = false;
+ boolean includeDataResources = true;
boolean multiDex = false;
String mainDexList = null;
boolean printHelpAndExit = false;
@@ -87,6 +91,8 @@
minApi = Integer.valueOf(args[++i]);
} else if (arg.equals("--force-proguard-compatibility")) {
forceProguardCompatibility = true;
+ } else if (arg.equals("--no-data-resources")) {
+ includeDataResources = false;
} else if (arg.equals("--output")) {
output = args[++i];
} else if (arg.equals("--multi-dex")) {
@@ -129,6 +135,7 @@
minApi,
multiDex,
forceProguardCompatibility,
+ includeDataResources,
mainDexList,
printHelpAndExit,
verticalClassMerging);
@@ -142,6 +149,8 @@
System.out.println("--multi-dex : ignored (provided for compatibility)");
System.out.println("--no-locals : ignored (provided for compatibility)");
System.out.println("--core-library : ignored (provided for compatibility)");
+ System.out.println("--force-proguard-compatibility : Proguard compatibility mode");
+ System.out.println("--no-data-resources : ignore all data resources");
}
}
@@ -170,7 +179,7 @@
new CompatProguardCommandBuilder(
options.forceProguardCompatibility, options.enableVerticalClassMerging);
builder
- .setOutput(Paths.get(options.output), OutputMode.DexIndexed)
+ .setOutput(Paths.get(options.output), OutputMode.DexIndexed, options.includeDataResources)
.addProguardConfiguration(options.proguardConfig, CommandLineOrigin.INSTANCE)
.setMinApiLevel(options.minApi);
if (options.mainDexList != null) {
diff --git a/src/test/java/com/android/tools/r8/R8CommandTest.java b/src/test/java/com/android/tools/r8/R8CommandTest.java
index 16bbae9..0f0bc2f 100644
--- a/src/test/java/com/android/tools/r8/R8CommandTest.java
+++ b/src/test/java/com/android/tools/r8/R8CommandTest.java
@@ -15,6 +15,7 @@
import com.android.tools.r8.dex.Marker.Tool;
import com.android.tools.r8.origin.EmbeddedOrigin;
import com.android.tools.r8.utils.FileUtils;
+import com.android.tools.r8.utils.ZipUtils;
import com.google.common.collect.ImmutableList;
import java.io.IOException;
import java.lang.reflect.Method;
@@ -22,10 +23,13 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
+import java.util.zip.ZipOutputStream;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
@@ -463,6 +467,116 @@
assertEquals(0, new ZipFile(emptyZip.toFile(), StandardCharsets.UTF_8).size());
}
+ private Path writeZipWithDataResource(String name) throws Exception {
+ Path dataResourceZip = temp.newFolder().toPath().resolve(name);
+ try (ZipOutputStream out =
+ new ZipOutputStream(
+ Files.newOutputStream(
+ dataResourceZip,
+ StandardOpenOption.CREATE,
+ StandardOpenOption.TRUNCATE_EXISTING))) {
+ // Write a directory entry and a normal entry.
+ ZipUtils.writeToZipStream(out, "org/", new byte[] {}, ZipEntry.STORED);
+ ZipUtils.writeToZipStream(
+ out, "org/resource.txt", "Hello world!".getBytes(), ZipEntry.STORED);
+ }
+ return dataResourceZip;
+ }
+
+ @Test
+ public void defaultResourceProcessing() throws Exception {
+ Path dataResourceZip = writeZipWithDataResource("dataResource.zip");
+ Path outputZip = temp.getRoot().toPath().resolve("output.zip");
+ R8.run(
+ R8Command.builder()
+ .addProgramFiles(dataResourceZip)
+ .setOutput(outputZip, OutputMode.ClassFile)
+ .build());
+ assertTrue(Files.exists(outputZip));
+ assertEquals(2, new ZipFile(outputZip.toFile(), StandardCharsets.UTF_8).size());
+ }
+
+ public void runCustomResourceProcessing(boolean includeDataResources, int expectedZipEntries)
+ throws Exception {
+ Path dataResourceZip = writeZipWithDataResource("dataResource.zip");
+ Path outputZip = temp.newFolder().toPath().resolve("output.zip");
+ R8.run(
+ R8Command.builder()
+ .addProgramFiles(dataResourceZip)
+ .setOutput(outputZip, OutputMode.ClassFile, includeDataResources)
+ .build());
+ assertTrue(Files.exists(outputZip));
+ assertEquals(
+ expectedZipEntries, new ZipFile(outputZip.toFile(), StandardCharsets.UTF_8).size());
+ }
+
+ private Path simpleProguardConfiguration() throws Exception {
+ Path proguardConfiguration = temp.newFile("printseedsandprintusage.txt").toPath();
+ FileUtils.writeTextFile(proguardConfiguration, ImmutableList.of("-keep class A { *; }"));
+ return proguardConfiguration;
+ }
+
+ @Test
+ public void noTreeShakingOption() throws Throwable {
+ // Default "keep all" rule implies no tree shaking.
+ assertFalse(parse().getEnableTreeShaking());
+ assertFalse(parse("--no-tree-shaking").getEnableTreeShaking());
+
+ // With a Proguard configuration --no-tree-shaking takes effect.
+ String proguardConfiguration = simpleProguardConfiguration().toAbsolutePath().toString();
+ assertTrue(parse("--pg-conf", proguardConfiguration).getEnableTreeShaking());
+ assertFalse(
+ parse("--no-tree-shaking", "--pg-conf", proguardConfiguration).getEnableTreeShaking());
+ }
+
+ @Test
+ public void noMinificationOption() throws Throwable {
+ // Default "keep all" rule implies no tree minification.
+ assertFalse(parse().getEnableMinification());
+ assertFalse(parse("--no-minification").getEnableMinification());
+
+ // With a Proguard configuration --no-tree-shaking takes effect.
+ String proguardConfiguration = simpleProguardConfiguration().toAbsolutePath().toString();
+ assertTrue(parse("--pg-conf", proguardConfiguration).getEnableMinification());
+ assertFalse(
+ parse("--no-minification", "--pg-conf", proguardConfiguration).getEnableMinification());
+ }
+
+ @Test
+ public void defaultDataResourcesOption() throws Throwable {
+ Path dataResourceZip = writeZipWithDataResource("dataResource.zip");
+ Path outputZip = temp.newFolder().toPath().resolve("output.zip");
+
+ R8.run(
+ parse(
+ dataResourceZip.toAbsolutePath().toString(),
+ "--output",
+ outputZip.toAbsolutePath().toString()));
+ assertTrue(Files.exists(outputZip));
+ assertEquals(2, new ZipFile(outputZip.toFile(), StandardCharsets.UTF_8).size());
+ }
+
+ @Test
+ public void noDataResourcesOption() throws Throwable {
+ Path dataResourceZip = writeZipWithDataResource("dataResource.zip");
+ Path outputZip = temp.newFolder().toPath().resolve("output.zip");
+
+ R8.run(
+ parse(
+ "--no-data-resources",
+ dataResourceZip.toAbsolutePath().toString(),
+ "--output",
+ outputZip.toAbsolutePath().toString()));
+ assertTrue(Files.exists(outputZip));
+ assertEquals(0, new ZipFile(outputZip.toFile(), StandardCharsets.UTF_8).size());
+ }
+
+ @Test
+ public void customResourceProcessing() throws Exception {
+ runCustomResourceProcessing(true, 2);
+ runCustomResourceProcessing(false, 0);
+ }
+
private R8Command parse(String... args) throws CompilationFailedException {
return R8Command.parse(args, EmbeddedOrigin.INSTANCE).build();
}
diff --git a/src/test/java/com/android/tools/r8/compatproguard/CompatProguardTest.java b/src/test/java/com/android/tools/r8/compatproguard/CompatProguardTest.java
index 8da93a5..cf95846 100644
--- a/src/test/java/com/android/tools/r8/compatproguard/CompatProguardTest.java
+++ b/src/test/java/com/android/tools/r8/compatproguard/CompatProguardTest.java
@@ -5,6 +5,9 @@
package com.android.tools.r8.compatproguard;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
import com.android.tools.r8.compatproguard.CompatProguard.CompatProguardOptions;
import org.junit.Test;
@@ -16,9 +19,20 @@
}
@Test
+ public void testDefaultDataResources() throws Exception {
+ CompatProguardOptions options = parseArgs();
+ assertNull(options.output);
+ assertEquals(1, options.minApi);
+ assertFalse(options.forceProguardCompatibility);
+ assertTrue(options.includeDataResources);
+ assertFalse(options.multiDex);
+ assertNull(options.mainDexList);
+ assertEquals(0, options.proguardConfig.size());
+ }
+
+ @Test
public void testShortLine() throws Exception {
- CompatProguardOptions options;
- options = parseArgs("-");
+ CompatProguardOptions options = parseArgs("-");
assertEquals(1, options.proguardConfig.size());
}
@@ -65,18 +79,20 @@
@Test
public void testInclude() throws Exception {
- CompatProguardOptions options;
-
- options = parseArgs("-include --my-include-file.txt");
+ CompatProguardOptions options = parseArgs("-include --my-include-file.txt");
assertEquals(1, options.proguardConfig.size());
assertEquals("-include --my-include-file.txt", options.proguardConfig.get(0));
}
@Test
public void testNoLocalsOption() throws Exception {
- CompatProguardOptions options;
-
- options = parseArgs("--no-locals");
+ CompatProguardOptions options = parseArgs("--no-locals");
assertEquals(0, options.proguardConfig.size());
}
+
+ @Test
+ public void testNoDataResources() throws Exception {
+ CompatProguardOptions options = parseArgs("--no-data-resources");
+ assertFalse(options.includeDataResources);
+ }
}
