Lookup in library classes before program classes
For the internal tests where the build setup is broken (overlap
between library classes and program classes) the script for running
these tests will now first sanatize the library classes by removing
all classes from it which is also in the program classes (the program
classes are untouched).
Bug: 120884788
Change-Id: I02f1c746cde9203e9edcd689caea1757db2bc405
diff --git a/src/main/java/com/android/tools/r8/graph/DirectMappedDexApplication.java b/src/main/java/com/android/tools/r8/graph/DirectMappedDexApplication.java
index fbfb968..a5d9143 100644
--- a/src/main/java/com/android/tools/r8/graph/DirectMappedDexApplication.java
+++ b/src/main/java/com/android/tools/r8/graph/DirectMappedDexApplication.java
@@ -9,7 +9,6 @@
import com.android.tools.r8.DataResourceProvider;
import com.android.tools.r8.graph.LazyLoadedDexApplication.AllClasses;
import com.android.tools.r8.naming.ClassNameMapper;
-import com.android.tools.r8.utils.ProgramClassCollection;
import com.android.tools.r8.utils.Timing;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
@@ -131,9 +130,9 @@
super(application);
// As a side-effect, this will force-load all classes.
AllClasses allClasses = application.loadAllClasses();
- assert application.programClasses().equals(allClasses.getProgramClasses());
libraryClasses = allClasses.getLibraryClasses();
classpathClasses = allClasses.getClasspathClasses();
+ replaceProgramClasses(allClasses.getProgramClasses());
}
private Builder(DirectMappedDexApplication application) {
@@ -150,15 +149,10 @@
@Override
public DexApplication build() {
// Rebuild the map. This will fail if keys are not unique.
- // TODO(zerny): It seems weird that we have conflict resolution here.
- ImmutableList<DexProgramClass> newProgramClasses =
- ImmutableList.copyOf(
- ProgramClassCollection.create(
- programClasses, ProgramClassCollection::resolveClassConflictImpl)
- .getAllClasses());
// TODO(zerny): Consider not rebuilding the map if no program classes are added.
- Map<DexType, DexClass> allClasses = new IdentityHashMap<>(
- newProgramClasses.size() + classpathClasses.size() + libraryClasses.size());
+ Map<DexType, DexClass> allClasses =
+ new IdentityHashMap<>(
+ programClasses.size() + classpathClasses.size() + libraryClasses.size());
// Note: writing classes in reverse priority order, so a duplicate will be correctly ordered.
// There should never be duplicates and that is asserted in the addAll subroutine.
addAll(allClasses, libraryClasses);
@@ -167,7 +161,7 @@
return new DirectMappedDexApplication(
proguardMap,
allClasses,
- newProgramClasses,
+ ImmutableList.copyOf(programClasses),
classpathClasses,
libraryClasses,
ImmutableList.copyOf(dataResourceProviders),
diff --git a/src/main/java/com/android/tools/r8/graph/LazyLoadedDexApplication.java b/src/main/java/com/android/tools/r8/graph/LazyLoadedDexApplication.java
index a58b9ac..b037db7 100644
--- a/src/main/java/com/android/tools/r8/graph/LazyLoadedDexApplication.java
+++ b/src/main/java/com/android/tools/r8/graph/LazyLoadedDexApplication.java
@@ -60,13 +60,16 @@
@Override
public DexClass definitionFor(DexType type) {
assert type.isClassType() : "Cannot lookup definition for type: " + type;
- DexClass clazz = programClasses.get(type);
+ DexClass clazz = null;
+ if (libraryClasses != null) {
+ clazz = libraryClasses.get(type);
+ }
+ if (clazz == null) {
+ clazz = programClasses.get(type);
+ }
if (clazz == null && classpathClasses != null) {
clazz = classpathClasses.get(type);
}
- if (clazz == null && libraryClasses != null) {
- clazz = libraryClasses.get(type);
- }
return clazz;
}
@@ -89,23 +92,9 @@
LibraryClassCollection libraryClassesLoader,
ClasspathClassCollection classpathClassesLoader,
ProgramClassCollection programClassesLoader) {
- // Collect loaded classes in the precedence order program classes, class path classes and
- // library classes.
- // TODO(b/120884788): Change library priority.
- assert programClassesLoader != null;
- // Program classes are supposed to be loaded, but force-loading them is no-op.
- programClassesLoader.forceLoad(type -> true);
- Map<DexType, DexProgramClass> allProgramClasses = programClassesLoader.getAllClassesInMap();
- int expectedMaxSize = allProgramClasses.size();
- programClasses = ImmutableList.copyOf(allProgramClasses.values());
+ int expectedMaxSize = 0;
- Map<DexType, DexClasspathClass> allClasspathClasses = null;
- if (classpathClassesLoader != null) {
- classpathClassesLoader.forceLoad(type -> true);
- allClasspathClasses = classpathClassesLoader.getAllClassesInMap();
- expectedMaxSize += allClasspathClasses.size();
- }
-
+ // Force-load library classes.
Map<DexType, DexLibraryClass> allLibraryClasses = null;
if (libraryClassesLoader != null) {
libraryClassesLoader.forceLoad(type -> true);
@@ -113,9 +102,51 @@
expectedMaxSize += allLibraryClasses.size();
}
+ // Program classes should be fully loaded.
+ assert programClassesLoader != null;
+ assert programClassesLoader.isFullyLoaded();
+ programClassesLoader.forceLoad(type -> true);
+ Map<DexType, DexProgramClass> allProgramClasses = programClassesLoader.getAllClassesInMap();
+ expectedMaxSize += allProgramClasses.size();
+
+ // Force-load classpath classes.
+ Map<DexType, DexClasspathClass> allClasspathClasses = null;
+ if (classpathClassesLoader != null) {
+ classpathClassesLoader.forceLoad(type -> true);
+ allClasspathClasses = classpathClassesLoader.getAllClassesInMap();
+ expectedMaxSize += allClasspathClasses.size();
+ }
+
+
+ // Collect loaded classes in the precedence order library classes, program classes and
+ // class path classes.
// Note: using hash map for building as the immutable builder does not support contains.
Map<DexType, DexClass> prioritizedClasses = new IdentityHashMap<>(expectedMaxSize);
- prioritizedClasses.putAll(allProgramClasses);
+ if (allLibraryClasses != null) {
+ ImmutableList.Builder<DexLibraryClass> builder = ImmutableList.builder();
+ allLibraryClasses.forEach(
+ (type, clazz) -> {
+ if (!prioritizedClasses.containsKey(type)) {
+ prioritizedClasses.put(type, clazz);
+ builder.add(clazz);
+ }
+ });
+ libraryClasses = builder.build();
+ } else {
+ libraryClasses = ImmutableList.of();
+ }
+
+ {
+ ImmutableList.Builder<DexProgramClass> builder = ImmutableList.builder();
+ allProgramClasses.forEach(
+ (type, clazz) -> {
+ if (!prioritizedClasses.containsKey(type)) {
+ prioritizedClasses.put(type, clazz);
+ builder.add(clazz);
+ }
+ });
+ programClasses = builder.build();
+ }
if (allClasspathClasses != null) {
ImmutableList.Builder<DexClasspathClass> builder = ImmutableList.builder();
@@ -131,20 +162,10 @@
classpathClasses = ImmutableList.of();
}
- if (allLibraryClasses != null) {
- ImmutableList.Builder<DexLibraryClass> builder = ImmutableList.builder();
- allLibraryClasses.forEach(
- (type, clazz) -> {
- if (!prioritizedClasses.containsKey(type)) {
- prioritizedClasses.put(type, clazz);
- builder.add(clazz);
- }
- });
- libraryClasses = builder.build();
- } else {
- libraryClasses = ImmutableList.of();
- }
allClasses = Collections.unmodifiableMap(prioritizedClasses);
+
+ assert prioritizedClasses.size()
+ == libraryClasses.size() + classpathClasses.size() + programClasses.size();
}
public Map<DexType, DexClass> getAllClasses() {
diff --git a/src/main/java/com/android/tools/r8/utils/ClassMap.java b/src/main/java/com/android/tools/r8/utils/ClassMap.java
index d8366df..416b389 100644
--- a/src/main/java/com/android/tools/r8/utils/ClassMap.java
+++ b/src/main/java/com/android/tools/r8/utils/ClassMap.java
@@ -163,10 +163,10 @@
ClassProvider<T> classProvider;
// Cache value of class provider, as it might change concurrently.
- classProvider = this.classProvider.get();
- if (classProvider == null) {
+ if (isFullyLoaded()) {
return;
}
+ classProvider = this.classProvider.get();
// Collects the types which might be represented in fully loaded class map.
knownClasses = Sets.newIdentityHashSet();
@@ -224,6 +224,10 @@
}
}
+ public boolean isFullyLoaded() {
+ return this.classProvider.get() == null;
+ }
+
// Supplier implementing a thread-safe loader for a class loaded from a
// class provider. Helps avoid synchronizing on the whole class map
// when loading a class.
diff --git a/src/test/java/com/android/tools/r8/JunitAvailabilityInHostArtTest.java b/src/test/java/com/android/tools/r8/JunitAvailabilityInHostArtTest.java
new file mode 100644
index 0000000..9e82042
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/JunitAvailabilityInHostArtTest.java
@@ -0,0 +1,71 @@
+// 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;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.ToolHelper.DexVm;
+import com.android.tools.r8.utils.StringUtils;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class JunitAvailabilityInHostArtTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().withAllApiLevels().build();
+ }
+
+ public JunitAvailabilityInHostArtTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ // It of the Art host VMs only 5.1.1 and 6.0.1 have junit.framework.Assert present at runtime.
+ private void checkResult(TestRunResult<?> result) {
+ if (parameters.getRuntime().isDex()
+ && ((parameters.getRuntime().asDex().getVm() == DexVm.ART_6_0_1_HOST)
+ || (parameters.getRuntime().asDex().getVm() == DexVm.ART_5_1_1_HOST))) {
+ result.assertSuccessWithOutput(StringUtils.lines("class junit.framework.Assert"));
+ } else {
+ result.assertFailureWithErrorThatMatches(containsString("ClassNotFoundException"));
+ }
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addInnerClasses(JunitAvailabilityInHostArtTest.class)
+ .addKeepMainRule(TestClass.class)
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .run(parameters.getRuntime(), TestClass.class)
+ .apply(this::checkResult);
+ }
+
+ @Test
+ public void testD8() throws Exception {
+ assumeTrue("Only run D8 for Dex backend", parameters.getBackend() == Backend.DEX);
+ testForD8()
+ .addInnerClasses(JunitAvailabilityInHostArtTest.class)
+ .setMinApi(parameters.getRuntime())
+ .compile()
+ .run(parameters.getRuntime(), TestClass.class)
+ .apply(this::checkResult);
+ }
+
+ static class TestClass {
+
+ public static void main(String[] args) throws Exception {
+ System.out.println(Class.forName("junit.framework.Assert"));
+ }
+ }
+
+ static class A {}
+}
diff --git a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
index afba9d9..67a4573 100644
--- a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
@@ -764,6 +764,7 @@
.match(TestCondition
.runtimes(DexVm.Version.V4_0_4, DexVm.Version.V4_4_4, DexVm.Version.V5_1_1,
DexVm.Version.V6_0_1));
+ // TODO(herhut): Change to V8_0_0 once we have a new art VM.
private static final TestCondition beforeAndroidO =
TestCondition.match(TestCondition.runtimesUpTo(DexVm.Version.V7_0_0));
// TODO(herhut): Change to V8_0_0 once we have a new art VM.
@@ -912,6 +913,26 @@
CompilerUnderTest.D8_AFTER_R8CF),
TestCondition.runtimes(DexVm.Version.V4_0_4, DexVm.Version.V4_4_4))))
.put("979-const-method-handle", beforeAndroidP)
+ // Missing class junit.framework.Assert (see JunitAvailabilityInHostArtTest).
+ .put(
+ "021-string2",
+ TestCondition.or(
+ TestCondition.match(
+ TestCondition.compilers(CompilerUnderTest.D8_AFTER_R8CF),
+ TestCondition.runtimesFrom(DexVm.Version.V7_0_0)),
+ TestCondition.match(
+ TestCondition.compilers(CompilerUnderTest.D8_AFTER_R8CF),
+ TestCondition.runtimes(DexVm.Version.V4_0_4, DexVm.Version.V4_4_4))))
+ // Missing class junit.framework.Assert (see JunitAvailabilityInHostArtTest).
+ .put(
+ "082-inline-execute",
+ TestCondition.or(
+ TestCondition.match(
+ TestCondition.compilers(CompilerUnderTest.D8_AFTER_R8CF),
+ TestCondition.runtimesFrom(DexVm.Version.V7_0_0)),
+ TestCondition.match(
+ TestCondition.compilers(CompilerUnderTest.D8_AFTER_R8CF),
+ TestCondition.runtimes(DexVm.Version.V4_0_4, DexVm.Version.V4_4_4))))
.build();
// Tests where code generation fails.
@@ -933,9 +954,6 @@
.put("974-verify-interface-super", TestCondition.match(TestCondition.R8DEX_COMPILER))
// R8 generates too large code in Goto.bigGoto(). b/74327727
.put("003-omnibus-opcodes", TestCondition.match(TestCondition.D8_AFTER_R8CF_COMPILER))
- // Contains a subset of JUnit which collides with library definitions of JUnit.
- .put("021-string2", TestCondition.match(TestCondition.D8_AFTER_R8CF_COMPILER))
- .put("082-inline-execute", TestCondition.match(TestCondition.D8_AFTER_R8CF_COMPILER))
.build();
// Tests that are invalid dex files and on which R8/D8 fails and that is OK.
diff --git a/src/test/java/com/android/tools/r8/classlookup/LibraryClassExtendsProgramClassTest.java b/src/test/java/com/android/tools/r8/classlookup/LibraryClassExtendsProgramClassTest.java
index 0b5a60b..1dd5afd 100644
--- a/src/test/java/com/android/tools/r8/classlookup/LibraryClassExtendsProgramClassTest.java
+++ b/src/test/java/com/android/tools/r8/classlookup/LibraryClassExtendsProgramClassTest.java
@@ -4,19 +4,32 @@
package com.android.tools.r8.classlookup;
-import static org.hamcrest.CoreMatchers.containsString;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assume.assumeTrue;
-import com.android.tools.r8.CompilationFailedException;
-import com.android.tools.r8.R8TestCompileResult;
import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.jasmin.JasminBuilder;
-import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
import java.util.List;
import org.junit.BeforeClass;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+// This test used to test R8 errors/warnings when library class extends program class. Before
+// the change fixing b/120884788, that could easily happen as lookup would lookup in program
+// classes before library classes, and the Android library included parts of JUnit, which could
+// also easily end up as program classes when JUnit was used by a program.
+//
+// Now that library classes are looked up before program classes these JUnit classes will be
+// found in the library and the ones in program will be ignored and not end up in the output.
+//
+// For a D8 compilation any class passed as input will end up in the output.
+@RunWith(Parameterized.class)
public class LibraryClassExtendsProgramClassTest extends TestBase {
private static List<byte[]> junitClasses;
@@ -28,51 +41,65 @@
junitClasses = builder.buildClasses();
}
- @Test
- public void testFullModeError() {
- try {
- testForR8(Backend.DEX)
- .setMinApi(AndroidApiLevel.O)
- .addProgramClassFileData(junitClasses)
- .addKeepAllClassesRule()
- .compile();
- fail("Succeeded in full mode");
- } catch (Throwable t) {
- assertTrue(t instanceof CompilationFailedException);
+ private final TestParameters parameters;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimes().withAllApiLevels().build();
+ }
+
+ public LibraryClassExtendsProgramClassTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ private void checkClassesInResult(CodeInspector inspector) {
+ if (parameters.getBackend() == Backend.DEX) {
+ noClassesInResult(inspector);
+ } else {
+ testCaseClassInResult(inspector);
}
}
- @Test
- public void testCompatibilityModeWarning() throws Exception {
- R8TestCompileResult result = testForR8Compat(Backend.DEX)
- .setMinApi(AndroidApiLevel.O)
- .addProgramClassFileData(junitClasses)
- .addKeepAllClassesRule()
- .compile()
- .assertOnlyWarnings();
+ private void noClassesInResult(CodeInspector inspector) {
+ assertEquals(0, inspector.allClasses().size());
+ }
- String[] libraryClassesExtendingTestCase = new String[]{
- "android.test.InstrumentationTestCase",
- "android.test.AndroidTestCase",
- "android.test.suitebuilder.TestSuiteBuilder$FailedToCreateTests"
- };
-
- for (String name : libraryClassesExtendingTestCase) {
- result
- .assertWarningMessageThatMatches(
- containsString(
- "Library class " + name + " extends program class junit.framework.TestCase"));
- }
+ private void testCaseClassInResult(CodeInspector inspector) {
+ assertEquals(1, inspector.allClasses().size());
+ assertThat(inspector.clazz("junit.framework.TestCase"), isPresent());
}
@Test
- public void testWithDontWarn() throws Exception {
- testForR8(Backend.DEX)
- .setMinApi(AndroidApiLevel.O)
+ public void testFullMode() throws Exception {
+ testForR8(parameters.getBackend())
+ .setMinApi(parameters.getApiLevel())
.addProgramClassFileData(junitClasses)
.addKeepAllClassesRule()
- .addKeepRules("-dontwarn android.test.**")
.compile()
+ .inspect(this::checkClassesInResult)
.assertNoMessages();
}
+
+ @Test
+ public void testCompatibilityMode() throws Exception {
+ testForR8Compat(parameters.getBackend())
+ .setMinApi(parameters.getApiLevel())
+ .addProgramClassFileData(junitClasses)
+ .addKeepAllClassesRule()
+ .compile()
+ .inspect(this::checkClassesInResult)
+ .assertNoMessages();
+ }
+
+ @Test
+ public void testD8() throws Exception {
+ assumeTrue("Only run D8 for Dex backend", parameters.getBackend() == Backend.DEX);
+ testForD8()
+ .setMinApi(parameters.getApiLevel())
+ .addProgramClassFileData(junitClasses)
+ .compile()
+ .inspect(this::testCaseClassInResult)
+ .assertNoMessages();
+ }
+
}
diff --git a/src/test/java/com/android/tools/r8/internal/CompilationTestBase.java b/src/test/java/com/android/tools/r8/internal/CompilationTestBase.java
index 9f25230..46583c9 100644
--- a/src/test/java/com/android/tools/r8/internal/CompilationTestBase.java
+++ b/src/test/java/com/android/tools/r8/internal/CompilationTestBase.java
@@ -19,6 +19,7 @@
import com.android.tools.r8.R8RunArtTestsTest.CompilerUnderTest;
import com.android.tools.r8.ResourceException;
import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.naming.MemberNaming.FieldSignature;
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.utils.AndroidApiLevel;
@@ -35,6 +36,7 @@
import com.android.tools.r8.utils.codeinspector.FoundClassSubject;
import com.android.tools.r8.utils.codeinspector.FoundFieldSubject;
import com.android.tools.r8.utils.codeinspector.FoundMethodSubject;
+import com.google.common.collect.ImmutableList;
import com.google.common.io.ByteStreams;
import com.google.common.io.Closer;
import java.io.File;
@@ -137,6 +139,18 @@
R8Command.Builder builder = R8Command.builder(reporter);
builder.addProgramFiles(ListUtils.map(inputs, Paths::get));
if (pgConfs != null) {
+ // Sanitize libraries for apps relying on the Proguard behaviour of lookup in program
+ // classes before library classes. See tools/sanitize_libraries.py for more information.
+ Path sanitizedLibrary = temp.getRoot().toPath().resolve("sanitized_lib.jar");
+ Path sanitizedPgConf = temp.getRoot().toPath().resolve("sanitized.config");
+ List<String> command = new ImmutableList.Builder<String>()
+ .add("tools/sanitize_libraries.py")
+ .add(sanitizedLibrary.toString())
+ .add(sanitizedPgConf.toString())
+ .addAll(pgConfs)
+ .build();
+ ProcessResult result = ToolHelper.runProcess(new ProcessBuilder(command));
+ assert result.exitCode == 0;
builder.addProguardConfigurationFiles(
pgConfs.stream().map(Paths::get).collect(Collectors.toList()));
} else {
diff --git a/src/test/java/com/android/tools/r8/internal/R8GMSCoreV10TreeShakeJarVerificationTest.java b/src/test/java/com/android/tools/r8/internal/R8GMSCoreV10TreeShakeJarVerificationTest.java
index 62c6bc8..4374d0e 100644
--- a/src/test/java/com/android/tools/r8/internal/R8GMSCoreV10TreeShakeJarVerificationTest.java
+++ b/src/test/java/com/android/tools/r8/internal/R8GMSCoreV10TreeShakeJarVerificationTest.java
@@ -7,9 +7,7 @@
import com.android.tools.r8.CompilationMode;
import com.android.tools.r8.utils.AndroidApp;
-import org.junit.Rule;
import org.junit.Test;
-import org.junit.rules.ExpectedException;
public class R8GMSCoreV10TreeShakeJarVerificationTest
extends R8GMSCoreTreeShakeJarVerificationTest {
diff --git a/src/test/java/com/android/tools/r8/maindexlist/b72312389/B72312389.java b/src/test/java/com/android/tools/r8/maindexlist/b72312389/B72312389.java
index c1741c9..056eaad 100644
--- a/src/test/java/com/android/tools/r8/maindexlist/b72312389/B72312389.java
+++ b/src/test/java/com/android/tools/r8/maindexlist/b72312389/B72312389.java
@@ -4,12 +4,11 @@
package com.android.tools.r8.maindexlist.b72312389;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
import com.android.tools.r8.BaseCommand;
import com.android.tools.r8.CompatProguardCommandBuilder;
-import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.DexIndexedConsumer;
import com.android.tools.r8.Diagnostic;
import com.android.tools.r8.DiagnosticsHandler;
@@ -58,7 +57,7 @@
.addMainDexRules(keepInstrumentationTestCaseRules, Origin.unknown())
.build();
List<String> mainDexList = GenerateMainDexList.run(command);
- assertTrue(mainDexList.contains("junit/framework/TestCase.class"));
+ assertFalse(mainDexList.contains("junit/framework/TestCase.class"));
diagnostics.assertEmpty();
}
@@ -86,9 +85,9 @@
.build();
CodeInspector inspector = new CodeInspector(ToolHelper.runR8(command));
assertTrue(inspector.clazz("instrumentationtest.InstrumentationTest").isPresent());
- assertTrue(mainDexList.content.contains("junit/framework/TestCase.class"));
- // TODO(72794301): Two copies of this message is a bit over the top.
- assertEquals(2,
+ assertFalse(mainDexList.content.contains("junit/framework/TestCase.class"));
+ assertEquals(
+ 0,
diagnostics.countLibraryClassExtensdProgramClassWarnings(
"android.test.InstrumentationTestCase", "junit.framework.TestCase"));
}
@@ -102,12 +101,8 @@
.addMainDexRules(keepInstrumentationTestCaseRules, Origin.unknown())
.setProgramConsumer(DexIndexedConsumer.emptyConsumer())
.build();
- try {
- R8.run(command);
- fail();
- } catch (CompilationFailedException e) {
- // Expected, as library class extending program class is an error for R8.
- }
+ R8.run(command);
+ diagnostics.assertEmpty();
}
private static class CollectingDiagnosticHandler implements DiagnosticsHandler {
diff --git a/tools/run_on_app.py b/tools/run_on_app.py
index a480214..b8bd0ed 100755
--- a/tools/run_on_app.py
+++ b/tools/run_on_app.py
@@ -15,6 +15,7 @@
import gmscore_data
import golem
import nest_data
+from sanitize_libraries import SanitizeLibraries
import toolhelper
import utils
import youtube_data
@@ -301,9 +302,14 @@
if options.compiler == 'r8':
if 'pgconf' in values and not options.k:
- for pgconf in values['pgconf']:
- args.extend(['--pg-conf', pgconf])
- app_provided_pg_conf = True
+ sanitized_lib_path = os.path.join(
+ os.path.abspath(outdir), 'sanitized_lib.jar')
+ sanitized_pgconf_path = os.path.join(
+ os.path.abspath(outdir), 'sanitized.config')
+ SanitizeLibraries(
+ sanitized_lib_path, sanitized_pgconf_path, values['pgconf'])
+ args.extend(['--pg-conf', sanitized_pgconf_path])
+ app_provided_pg_conf = True
if options.k:
args.extend(['--pg-conf', options.k])
if 'maindexrules' in values:
diff --git a/tools/sanitize_libraries.py b/tools/sanitize_libraries.py
new file mode 100755
index 0000000..e67a06e
--- /dev/null
+++ b/tools/sanitize_libraries.py
@@ -0,0 +1,76 @@
+#!/usr/bin/env python
+# 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.
+from __future__ import print_function
+import os
+import sys
+import zipfile
+
+# Proguard lookup program classes before library classes. In R8 this is
+# not the behaviour (it used to be) R8 will check library classes before
+# program classes. Some apps have duplicate classes in the library and program.
+# To make these apps work with R8 simulate program classes before library
+# classes by creating a new library jar which have all the provided library
+# classes which are not also in program classes.
+def SanitizeLibraries(sanitized_lib_path, sanitized_pgconf_path, pgconfs):
+
+ injars = []
+ libraryjars = []
+
+ with open(sanitized_pgconf_path, 'w') as sanitized_pgconf:
+ for pgconf in pgconfs:
+ pgconf_dirname = os.path.abspath(os.path.dirname(pgconf))
+ first_library_jar = True
+ with open(pgconf) as pgconf_file:
+ for line in pgconf_file:
+ trimmed = line.strip()
+ if trimmed.startswith('-injars'):
+ # Collect -injars and leave them in the configuration.
+ injar = os.path.join(
+ pgconf_dirname, trimmed[len('-injars'):].strip())
+ injars.append(injar)
+ sanitized_pgconf.write('-injars {}\n'.format(injar))
+ elif trimmed.startswith('-libraryjars'):
+ # Collect -libraryjars and replace them with the sanitized library.
+ libraryjar = os.path.join(
+ pgconf_dirname, trimmed[len('-libraryjars'):].strip())
+ libraryjars.append(libraryjar)
+ if first_library_jar:
+ sanitized_pgconf.write(
+ '-libraryjars {}\n'.format(sanitized_lib_path))
+ first_library_jar = False
+ sanitized_pgconf.write('# {}'.format(line))
+ else:
+ sanitized_pgconf.write(line)
+
+ program_entries = set()
+ library_entries = set()
+
+ for injar in injars:
+ with zipfile.ZipFile(injar, 'r') as injar_zf:
+ for zipinfo in injar_zf.infolist():
+ program_entries.add(zipinfo.filename)
+
+ with zipfile.ZipFile(sanitized_lib_path, 'w') as output_zf:
+ for libraryjar in libraryjars:
+ with zipfile.ZipFile(libraryjar, 'r') as input_zf:
+ for zipinfo in input_zf.infolist():
+ if (not zipinfo.filename in program_entries
+ and not zipinfo.filename in library_entries):
+ library_entries.add(zipinfo.filename)
+ output_zf.writestr(zipinfo, input_zf.read(zipinfo))
+
+ return sanitized_pgconf_path
+
+def main(argv):
+ if (len(argv) < 3):
+ print("Wrong number of arguments!")
+ print("Usage: sanitize_libraries.py " +
+ "<sanitized_lib> <sanitized_pgconf> (<existing_pgconf)+")
+ return 1
+ else:
+ SanitizeLibraries(argv[0], argv[1], argv[2:])
+
+if __name__ == '__main__':
+ sys.exit(main(sys.argv[1:]))
diff --git a/tools/utils.py b/tools/utils.py
index 4e658cc..9bd7711 100644
--- a/tools/utils.py
+++ b/tools/utils.py
@@ -337,16 +337,18 @@
return exit_code == 0
class TempDir(object):
- def __init__(self, prefix=''):
+ def __init__(self, prefix='', delete=True):
self._temp_dir = None
self._prefix = prefix
+ self._delete = delete
def __enter__(self):
self._temp_dir = tempfile.mkdtemp(self._prefix)
return self._temp_dir
def __exit__(self, *_):
- shutil.rmtree(self._temp_dir, ignore_errors=True)
+ if self._delete:
+ shutil.rmtree(self._temp_dir, ignore_errors=True)
class ChangedWorkingDirectory(object):
def __init__(self, working_directory, quiet=False):
