Merge "Follow up with changes on Enqueuer"
diff --git a/src/main/java/com/android/tools/r8/DexIndexedConsumer.java b/src/main/java/com/android/tools/r8/DexIndexedConsumer.java
index e235e67..8731d2a 100644
--- a/src/main/java/com/android/tools/r8/DexIndexedConsumer.java
+++ b/src/main/java/com/android/tools/r8/DexIndexedConsumer.java
@@ -12,6 +12,7 @@
import com.android.tools.r8.utils.OutputBuilder;
import com.android.tools.r8.utils.StringDiagnostic;
import com.android.tools.r8.utils.ZipUtils;
+import com.google.common.collect.ImmutableList;
import com.google.common.io.ByteStreams;
import com.google.common.io.Closer;
import java.io.IOException;
@@ -181,6 +182,12 @@
public static void writeResources(Path archive, List<ProgramResource> resources)
throws IOException, ResourceException {
+ writeResources(archive, resources, ImmutableList.of());
+ }
+
+ public static void writeResources(
+ Path archive, List<ProgramResource> resources, List<DataEntryResource> dataResources)
+ throws IOException, ResourceException {
OpenOption[] options =
new OpenOption[] {StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING};
try (Closer closer = Closer.create()) {
@@ -191,6 +198,11 @@
byte[] bytes = ByteStreams.toByteArray(closer.register(resource.getByteStream()));
ZipUtils.writeToZipStream(out, entryName, bytes, ZipEntry.STORED);
}
+ for (DataEntryResource dataResource : dataResources) {
+ String entryName = dataResource.getName();
+ byte[] bytes = ByteStreams.toByteArray(closer.register(dataResource.getByteStream()));
+ ZipUtils.writeToZipStream(out, entryName, bytes, ZipEntry.STORED);
+ }
}
}
}
diff --git a/src/main/java/com/android/tools/r8/naming/Minifier.java b/src/main/java/com/android/tools/r8/naming/Minifier.java
index 5333125..8f923b0 100644
--- a/src/main/java/com/android/tools/r8/naming/Minifier.java
+++ b/src/main/java/com/android/tools/r8/naming/Minifier.java
@@ -24,6 +24,7 @@
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.Timing;
import com.google.common.collect.ImmutableMap;
+import java.util.HashMap;
import java.util.IdentityHashMap;
import java.util.Map;
import java.util.Set;
@@ -91,6 +92,8 @@
private final AppInfo appInfo;
private final Map<String, String> packageRenaming;
private final Map<DexItem, DexString> renaming = new IdentityHashMap<>();
+ // This set is only used for asserting no duplicated names.
+ private final Map<DexString, DexType> renamedTypesForVerification;
private MinifiedRenaming(
ClassRenaming classRenaming,
@@ -103,6 +106,10 @@
renaming.putAll(methodRenaming.renaming);
renaming.putAll(methodRenaming.callSiteRenaming);
renaming.putAll(fieldRenaming.renaming);
+ renamedTypesForVerification = new HashMap<>();
+ for (Map.Entry<DexType, DexString> entry : classRenaming.classRenaming.entrySet()) {
+ renamedTypesForVerification.put(entry.getValue(), entry.getKey());
+ }
}
@Override
@@ -112,7 +119,18 @@
@Override
public DexString lookupDescriptor(DexType type) {
- return renaming.getOrDefault(type, type.descriptor);
+ DexString dexString = renaming.get(type);
+ if (dexString != null) {
+ return dexString;
+ }
+ assert type.isPrimitiveType()
+ || type.isVoidType()
+ || !renamedTypesForVerification.containsKey(type.descriptor)
+ : "Duplicate minified type '"
+ + type.descriptor
+ + "' already mapped for: "
+ + renamedTypesForVerification.get(type.descriptor);
+ return type.descriptor;
}
@Override
diff --git a/src/main/java/com/android/tools/r8/utils/AndroidApp.java b/src/main/java/com/android/tools/r8/utils/AndroidApp.java
index 28d1628..369367e 100644
--- a/src/main/java/com/android/tools/r8/utils/AndroidApp.java
+++ b/src/main/java/com/android/tools/r8/utils/AndroidApp.java
@@ -13,6 +13,7 @@
import com.android.tools.r8.DataEntryResource;
import com.android.tools.r8.DataResource;
import com.android.tools.r8.DataResourceProvider;
+import com.android.tools.r8.DataResourceProvider.Visitor;
import com.android.tools.r8.DexFilePerClassFileConsumer;
import com.android.tools.r8.DexIndexedConsumer;
import com.android.tools.r8.DirectoryClassFileProvider;
@@ -213,6 +214,29 @@
}
}
+ public List<DataEntryResource> getDataEntryResourcesForTesting() throws ResourceException {
+ List<DataEntryResource> out = new ArrayList<>();
+ for (ProgramResourceProvider programResourceProvider : getProgramResourceProviders()) {
+ DataResourceProvider dataResourceProvider = programResourceProvider.getDataResourceProvider();
+ if (dataResourceProvider != null) {
+ dataResourceProvider.accept(
+ new Visitor() {
+
+ @Override
+ public void visit(DataDirectoryResource directory) {
+ // Ignore.
+ }
+
+ @Override
+ public void visit(DataEntryResource file) {
+ out.add(file);
+ }
+ });
+ }
+ }
+ return out;
+ }
+
/** Get program resource providers. */
public List<ProgramResourceProvider> getProgramResourceProviders() {
return programResourceProviders;
@@ -320,14 +344,12 @@
}
}
- /**
- * Write the dex program resources to @code{archive} and the proguard resource as its sibling.
- */
+ /** Write the dex program resources to @code{archive}. */
public void writeToZip(Path archive, OutputMode outputMode) throws IOException {
try {
if (outputMode == OutputMode.DexIndexed) {
- List<ProgramResource> resources = getDexProgramResourcesForTesting();
- DexIndexedConsumer.ArchiveConsumer.writeResources(archive, resources);
+ DexIndexedConsumer.ArchiveConsumer.writeResources(
+ archive, getDexProgramResourcesForTesting(), getDataEntryResourcesForTesting());
} else if (outputMode == OutputMode.DexFilePerClassFile) {
List<ProgramResource> resources = getDexProgramResourcesForTesting();
DexFilePerClassFileConsumer.ArchiveConsumer.writeResources(
diff --git a/src/main/java/com/android/tools/r8/utils/AndroidAppConsumers.java b/src/main/java/com/android/tools/r8/utils/AndroidAppConsumers.java
index 4371edd..c7078ec 100644
--- a/src/main/java/com/android/tools/r8/utils/AndroidAppConsumers.java
+++ b/src/main/java/com/android/tools/r8/utils/AndroidAppConsumers.java
@@ -6,6 +6,9 @@
import com.android.tools.r8.BaseCompilerCommand;
import com.android.tools.r8.ByteDataView;
import com.android.tools.r8.ClassFileConsumer;
+import com.android.tools.r8.DataDirectoryResource;
+import com.android.tools.r8.DataEntryResource;
+import com.android.tools.r8.DataResourceConsumer;
import com.android.tools.r8.DexFilePerClassFileConsumer;
import com.android.tools.r8.DexIndexedConsumer;
import com.android.tools.r8.DexIndexedConsumer.ForwardingConsumer;
@@ -105,6 +108,29 @@
}
}
+ @Override
+ public DataResourceConsumer getDataResourceConsumer() {
+ assert consumer.getDataResourceConsumer() == null;
+ return new DataResourceConsumer() {
+
+ @Override
+ public void accept(
+ DataDirectoryResource directory, DiagnosticsHandler diagnosticsHandler) {
+ // Ignore.
+ }
+
+ @Override
+ public void accept(DataEntryResource file, DiagnosticsHandler diagnosticsHandler) {
+ builder.addDataResource(file);
+ }
+
+ @Override
+ public void finished(DiagnosticsHandler handler) {
+ // Ignore.
+ }
+ };
+ }
+
synchronized void addDexFile(int fileIndex, byte[] data, Set<String> descriptors) {
files.put(fileIndex, new DescriptorsWithContents(descriptors, data));
}
diff --git a/src/main/java/com/android/tools/r8/utils/StringUtils.java b/src/main/java/com/android/tools/r8/utils/StringUtils.java
index 244639e..c91c288 100644
--- a/src/main/java/com/android/tools/r8/utils/StringUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/StringUtils.java
@@ -141,7 +141,7 @@
return builder.toString();
}
- public static String lines(String... lines) {
+ public static String lines(List<String> lines) {
StringBuilder builder = new StringBuilder();
for (String line : lines) {
builder.append(line).append(LINE_SEPARATOR);
@@ -149,6 +149,10 @@
return builder.toString();
}
+ public static String lines(String... lines) {
+ return lines(Arrays.asList(lines));
+ }
+
public static String withNativeLineSeparator(String s) {
s = s.replace("\r\n", "\n");
if (LINE_SEPARATOR.equals("\r\n")) {
diff --git a/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java b/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java
index 9dfb30d..245a35e 100644
--- a/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/ExternalR8TestBuilder.java
@@ -131,6 +131,11 @@
}
@Override
+ public ExternalR8TestBuilder addDataEntryResources(DataEntryResource... resources) {
+ throw new Unimplemented("No support for adding data entry resources");
+ }
+
+ @Override
public ExternalR8TestBuilder addProgramClasses(Collection<Class<?>> classes) {
// Adding a collection of classes will build a jar of exactly those classes so that no other
// classes are made available via a too broad classpath directory.
diff --git a/src/test/java/com/android/tools/r8/ProguardTestBuilder.java b/src/test/java/com/android/tools/r8/ProguardTestBuilder.java
index 0fd0354..3b4647f 100644
--- a/src/test/java/com/android/tools/r8/ProguardTestBuilder.java
+++ b/src/test/java/com/android/tools/r8/ProguardTestBuilder.java
@@ -181,6 +181,11 @@
}
@Override
+ public ProguardTestBuilder addDataEntryResources(DataEntryResource... resources) {
+ throw new Unimplemented("No support for adding data entry resources");
+ }
+
+ @Override
public ProguardTestBuilder addProgramClasses(Collection<Class<?>> classes) {
// Adding a collection of classes will build a jar of exactly those classes so that no other
// classes are made available via a too broad classpath directory.
diff --git a/src/test/java/com/android/tools/r8/R8TestBuilder.java b/src/test/java/com/android/tools/r8/R8TestBuilder.java
index daf5568..e8555d7 100644
--- a/src/test/java/com/android/tools/r8/R8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/R8TestBuilder.java
@@ -97,6 +97,11 @@
}
@Override
+ public R8TestBuilder addDataEntryResources(DataEntryResource... resources) {
+ return addDataResources(Arrays.asList(resources));
+ }
+
+ @Override
public R8TestBuilder addKeepRuleFiles(List<Path> files) {
builder.addProguardConfigurationFiles(files);
return self();
diff --git a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
index 9d8e03c..a17ab09 100644
--- a/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
@@ -46,6 +46,8 @@
return minification(false);
}
+ public abstract T addDataEntryResources(DataEntryResource... resources);
+
public abstract T addKeepRuleFiles(List<Path> files);
public T addKeepRuleFiles(Path... files) throws IOException {
@@ -62,6 +64,10 @@
return addKeepRules("-keep class ** { *; }");
}
+ public T addKeepAllInterfacesRule() {
+ return addKeepRules("-keep interface ** { *; }");
+ }
+
public T addKeepClassRules(Class<?>... classes) {
for (Class<?> clazz : classes) {
addKeepRules("-keep class " + clazz.getTypeName());
diff --git a/src/test/java/com/android/tools/r8/naming/MinifierTest.java b/src/test/java/com/android/tools/r8/naming/MinifierTest.java
index 473ca53..adf74ce 100644
--- a/src/test/java/com/android/tools/r8/naming/MinifierTest.java
+++ b/src/test/java/com/android/tools/r8/naming/MinifierTest.java
@@ -3,13 +3,16 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.naming;
+import static junit.framework.TestCase.assertTrue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.fail;
import com.android.tools.r8.graph.DexField;
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.naming.Minifier.MinifiedRenaming;
import com.android.tools.r8.utils.ListUtils;
import com.android.tools.r8.utils.Timing;
import java.nio.file.Paths;
@@ -40,6 +43,25 @@
inspection.accept(dexItemFactory, naming);
}
+ @Test
+ public void ensureClassesAddedToRenamingOrNoClashTest() throws Exception {
+ MinifiedRenaming naming =
+ (MinifiedRenaming) runMinifier(ListUtils.map(keepRulesFiles, Paths::get));
+ // Create a type that exists.
+ String existingType = "La/c;";
+ DexType d = dexItemFactory.createType(existingType);
+ try {
+ naming.lookupDescriptor(d);
+ } catch (AssertionError ae) {
+ assertTrue(
+ ae.getMessage()
+ .startsWith(
+ "Duplicate minified type '" + existingType + "' already mapped for: naming001."));
+ return;
+ }
+ fail("Should have thrown an error.");
+ }
+
@Parameters(name = "test: {0} keep: {1}")
public static Collection<Object[]> data() {
List<String> tests = Arrays.asList("naming001");
diff --git a/src/test/java/com/android/tools/r8/shaking/ServiceLoaderTest.java b/src/test/java/com/android/tools/r8/shaking/ServiceLoaderTest.java
new file mode 100644
index 0000000..e005a2c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/shaking/ServiceLoaderTest.java
@@ -0,0 +1,96 @@
+// Copyright (c) 2019, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.shaking;
+
+import com.android.tools.r8.DataEntryResource;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.BooleanUtils;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.google.common.collect.Lists;
+import java.util.List;
+import java.util.ServiceLoader;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class ServiceLoaderTest extends TestBase {
+
+ private final boolean includeWorldGreeter;
+
+ @Parameters(name = "Include WorldGreeter: {0}")
+ public static Boolean[] data() {
+ return BooleanUtils.values();
+ }
+
+ public ServiceLoaderTest(boolean includeWorldGreeter) {
+ this.includeWorldGreeter = includeWorldGreeter;
+ }
+
+ @Test
+ public void test() throws Exception {
+ String expectedOutput = includeWorldGreeter ? "Hello world!" : "Hello";
+
+ List<String> serviceImplementations = Lists.newArrayList(HelloGreeter.class.getTypeName());
+ if (includeWorldGreeter) {
+ serviceImplementations.add(WorldGreeter.class.getTypeName());
+ }
+
+ CodeInspector inspector =
+ testForR8(Backend.DEX)
+ .addInnerClasses(ServiceLoaderTest.class)
+ .addKeepMainRule(TestClass.class)
+ // TODO(b/124181030): Test should work without the following keep-all-rules.
+ .addKeepAllClassesRule()
+ .addKeepAllInterfacesRule()
+ .addDataEntryResources(
+ DataEntryResource.fromBytes(
+ StringUtils.lines(serviceImplementations).getBytes(),
+ "META-INF/services/" + Greeter.class.getTypeName(),
+ Origin.unknown()))
+ .run(TestClass.class)
+ .assertSuccessWithOutput(expectedOutput)
+ .inspector();
+
+ // TODO(b/124181030): Verify that Greeter is merged into HelloGreeter when `includeWorldGreeter`
+ // is false.
+
+ // TODO(b/124181030): Verify that META-INF/services/...WorldGreeter is removed when
+ // `includeWorldGreeter` is false.
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) {
+ for (Greeter greeter : ServiceLoader.load(Greeter.class)) {
+ System.out.print(greeter.greeting());
+ }
+ }
+ }
+
+ public interface Greeter {
+
+ String greeting();
+ }
+
+ public static class HelloGreeter implements Greeter {
+
+ @Override
+ public String greeting() {
+ return "Hello";
+ }
+ }
+
+ public static class WorldGreeter implements Greeter {
+
+ @Override
+ public String greeting() {
+ return " world!";
+ }
+ }
+}
diff --git a/tools/run_on_as_app.py b/tools/run_on_as_app.py
index 53ddbe7..d6dd647 100755
--- a/tools/run_on_as_app.py
+++ b/tools/run_on_as_app.py
@@ -730,10 +730,11 @@
# Make a copy of r8.jar and r8lib.jar such that they stay the same for
# the entire execution of this script.
- if 'r8-nolib' in options.shrinker:
+ all_shrinkers_enabled = (options.shrinker == None)
+ if all_shrinkers_enabled or 'r8-nolib' in options.shrinker:
assert os.path.isfile(utils.R8_JAR), 'Cannot build without r8.jar'
shutil.copyfile(utils.R8_JAR, os.path.join(temp_dir, 'r8.jar'))
- if 'r8' in options.shrinker:
+ if all_shrinkers_enabled or 'r8' in options.shrinker:
assert os.path.isfile(utils.R8LIB_JAR), 'Cannot build without r8lib.jar'
shutil.copyfile(utils.R8LIB_JAR, os.path.join(temp_dir, 'r8lib.jar'))
