Enable new repackaging by default
Bug: 165783399
Change-Id: Id9aae345dc0ef3ab31607585d33a606f684864cd
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 9bee3f3..8cd402f 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -807,11 +807,9 @@
appView.setGraphLens(memberRebindingLens);
// Perform repackaging.
- if (options.isRepackagingEnabled() && options.testing.enableExperimentalRepackaging) {
+ if (options.isRepackagingEnabled()) {
DirectMappedDexApplication.Builder appBuilder =
appView.appInfo().app().asDirect().builder();
- // TODO(b/165783399): We need to deal with non-rebound member references in the writer,
- // possibly by adding a member rebinding lens on top of the repackaging lens.
RepackagingLens lens =
new Repackaging(appView.withLiveness()).run(appBuilder, executorService, timing);
if (lens != null) {
diff --git a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
index 40713b8..e7ca5b5 100644
--- a/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
+++ b/src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
@@ -21,7 +21,6 @@
import com.android.tools.r8.shaking.ProguardPackageNameList;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.InternalOptions;
-import com.android.tools.r8.utils.InternalOptions.PackageObfuscationMode;
import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.Timing;
import com.google.common.collect.ImmutableMap;
@@ -40,7 +39,6 @@
private final ClassNamingStrategy classNamingStrategy;
private final PackageNamingStrategy packageNamingStrategy;
private final Iterable<? extends DexClass> classes;
- private final PackageObfuscationMode packageObfuscationMode;
private final boolean isAccessModificationAllowed;
private final Set<String> noObfuscationPrefixes = Sets.newHashSet();
private final Set<String> usedPackagePrefixes = Sets.newHashSet();
@@ -63,27 +61,16 @@
this.packageNamingStrategy = packageNamingStrategy;
this.classes = classes;
InternalOptions options = appView.options();
- this.packageObfuscationMode =
- options.testing.enableExperimentalRepackaging
- ? PackageObfuscationMode.NONE
- : options.getProguardConfiguration().getPackageObfuscationMode();
this.isAccessModificationAllowed =
options.getProguardConfiguration().isAccessModificationAllowed();
this.keepInnerClassStructure = options.keepInnerClassStructure();
// Initialize top-level naming state.
- if (options.testing.enableExperimentalRepackaging) {
- topLevelState = new Namespace("");
- String newPackageDescriptor =
- StringUtils.replaceAll(options.getProguardConfiguration().getPackagePrefix(), ".", "/");
- if (!newPackageDescriptor.isEmpty()) {
- registerPackagePrefixesAsUsed(newPackageDescriptor, false);
- }
- } else {
- topLevelState =
- new Namespace(
- getPackageBinaryNameFromJavaType(
- options.getProguardConfiguration().getPackagePrefix()));
+ topLevelState = new Namespace("");
+ String newPackageDescriptor =
+ StringUtils.replaceAll(options.getProguardConfiguration().getPackagePrefix(), ".", "/");
+ if (!newPackageDescriptor.isEmpty()) {
+ registerPackagePrefixesAsUsed(newPackageDescriptor, false);
}
states.put("", topLevelState);
@@ -284,25 +271,7 @@
if (noObfuscationPrefixes.contains(packageName) || keepPackageNames.matches(type)) {
return states.computeIfAbsent(packageName, Namespace::new);
}
- Namespace state = topLevelState;
- switch (packageObfuscationMode) {
- case NONE:
- // For general obfuscation, rename all the involved package prefixes.
- state = getStateForPackagePrefix(packageName);
- break;
- case REPACKAGE:
- // For repackaging, all classes are repackaged to a single package.
- state = topLevelState;
- break;
- case FLATTEN:
- // For flattening, all packages are repackaged to a single package.
- state = states.computeIfAbsent(packageName, k -> {
- String renamedPackagePrefix = topLevelState.nextPackagePrefix();
- return new Namespace(renamedPackagePrefix);
- });
- break;
- }
- return state;
+ return getStateForPackagePrefix(packageName);
}
private Namespace getStateForPackagePrefix(String prefix) {
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 091325a..a429fb1 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -1253,7 +1253,6 @@
public boolean alwaysUsePessimisticRegisterAllocation = false;
public boolean enableCheckCastAndInstanceOfRemoval = true;
public boolean enableDeadSwitchCaseElimination = true;
- public boolean enableExperimentalRepackaging = false;
public boolean enableInvokeSuperToInvokeVirtualRewriting = true;
public boolean enableSwitchToIfRewriting = true;
public boolean enableEnumUnboxingDebugLogs = false;
diff --git a/src/test/examples/naming101/a/b/c.java b/src/test/examples/naming101/a/b/c.java
index b399bda..04dc965 100644
--- a/src/test/examples/naming101/a/b/c.java
+++ b/src/test/examples/naming101/a/b/c.java
@@ -4,5 +4,5 @@
package naming101.a.b;
public class c {
- static boolean flag = true;
+ public static boolean flag = true;
}
diff --git a/src/test/java/com/android/tools/r8/naming/b72391662/B72391662.java b/src/test/java/com/android/tools/r8/naming/b72391662/B72391662.java
index a29c664..ddf20ef 100644
--- a/src/test/java/com/android/tools/r8/naming/b72391662/B72391662.java
+++ b/src/test/java/com/android/tools/r8/naming/b72391662/B72391662.java
@@ -140,7 +140,7 @@
assertThat(testClass, isPresent());
// Test the totally unused method.
- MethodSubject staticMethod = testClass.method("void", "unused", ImmutableList.of());
+ MethodSubject staticMethod = testClass.uniqueMethodWithName("unused");
assertThat(staticMethod, isPresent());
assertEquals(minify, staticMethod.isRenamed());
if (shrinker.isR8()) {
@@ -150,7 +150,7 @@
}
// Test an indirectly referred method.
- staticMethod = testClass.method("java.lang.String", "staticMethod", ImmutableList.of());
+ staticMethod = testClass.uniqueMethodWithName("staticMethod");
assertThat(staticMethod, isPresent());
assertEquals(minify, staticMethod.isRenamed());
boolean publicizeCondition = shrinker.isR8() ? allowAccessModification
@@ -191,7 +191,7 @@
assertThat(testClass, isPresent());
// Test the totally unused method.
- MethodSubject staticMethod = testClass.method("void", "unused", ImmutableList.of());
+ MethodSubject staticMethod = testClass.uniqueMethodWithName("unused");
assertThat(staticMethod, isPresent());
assertEquals(minify, staticMethod.isRenamed());
if (shrinker.isR8()) {
@@ -201,7 +201,7 @@
}
// Test an indirectly referred method.
- staticMethod = testClass.method("java.lang.String", "staticMethod", ImmutableList.of());
+ staticMethod = testClass.uniqueMethodWithName("staticMethod");
assertThat(staticMethod, isPresent());
assertEquals(minify, staticMethod.isRenamed());
boolean publicizeCondition = shrinker.isR8() ? allowAccessModification
@@ -254,11 +254,11 @@
assertThat(testClass, isPresent());
// Test the totally unused method.
- MethodSubject staticMethod = testClass.method("void", "unused", ImmutableList.of());
+ MethodSubject staticMethod = testClass.uniqueMethodWithName("unused");
assertThat(staticMethod, not(isPresent()));
// Test an indirectly referred method.
- staticMethod = testClass.method("java.lang.String", "staticMethod", ImmutableList.of());
+ staticMethod = testClass.uniqueMethodWithName("staticMethod");
assertThat(staticMethod, isPresent());
assertEquals(minify, staticMethod.isRenamed());
boolean publicizeCondition = shrinker.isR8() ? allowAccessModification
diff --git a/src/test/java/com/android/tools/r8/proguard/configuration/RepackagingCompatibilityTest.java b/src/test/java/com/android/tools/r8/proguard/configuration/RepackagingCompatibilityTest.java
index 8d62a04..b078b2f 100644
--- a/src/test/java/com/android/tools/r8/proguard/configuration/RepackagingCompatibilityTest.java
+++ b/src/test/java/com/android/tools/r8/proguard/configuration/RepackagingCompatibilityTest.java
@@ -62,7 +62,7 @@
@Test
public void testProguard() throws Exception {
- runTest(testForProguard(), "Proguard");
+ runTest(testForProguard().addKeepRules("-dontwarn " + getClass().getTypeName()), "Proguard");
}
private void runTest(TestShrinkerBuilder<?, ?, ?, ?, ?> builder, String shrinker)
@@ -125,11 +125,11 @@
throw new Unreachable();
}
}
-}
-class RepackagingCompatabilityTestClass {
+ public static class RepackagingCompatabilityTestClass {
- public static void main(String[] args) {
- System.out.println("Hello world!");
+ public static void main(String[] args) {
+ System.out.println("Hello world!");
+ }
}
}
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageTest.java
index b9d3c40..2bfe216 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageTest.java
@@ -66,28 +66,25 @@
private final boolean allowAccessModification;
- @Parameters(name = "{3}, allow access modification: {0}, experimental: {1}, kind: {2}")
+ @Parameters(name = "{3}, allow access modification: {0}, kind: {1}")
public static List<Object[]> data() {
return buildParameters(
BooleanUtils.values(),
- BooleanUtils.values(),
ImmutableList.of(FLATTEN_PACKAGE_HIERARCHY, REPACKAGE_CLASSES),
getTestParameters().withAllRuntimesAndApiLevels().build());
}
public RepackageTest(
boolean allowAccessModification,
- boolean enableExperimentalRepackaging,
String flattenPackageHierarchyOrRepackageClasses,
TestParameters parameters) {
- super(enableExperimentalRepackaging, flattenPackageHierarchyOrRepackageClasses, parameters);
+ super(flattenPackageHierarchyOrRepackageClasses, parameters);
this.allowAccessModification = allowAccessModification;
}
@Test
public void testJvm() throws Exception {
assumeFalse(allowAccessModification);
- assumeFalse(isExperimentalRepackaging());
assumeTrue(isFlattenPackageHierarchy());
assumeTrue(parameters.isCfRuntime());
testForJvm()
@@ -134,11 +131,7 @@
* eligible for repackaging (or it needs to stay in its original package).
*/
private void forEachClass(BiConsumer<Class<?>, Boolean> consumer) {
- // TODO(b/165783399): This should be renamed to markAlwaysEligible() and always pass `true` to
- // the consumer, since these classes should be repackaged independent of
- // -allowaccessmodification.
- Consumer<Class<?>> markShouldAlwaysBeEligible =
- clazz -> consumer.accept(clazz, allowAccessModification || isExperimentalRepackaging());
+ Consumer<Class<?>> markShouldAlwaysBeEligible = clazz -> consumer.accept(clazz, true);
Consumer<Class<?>> markEligibleWithAllowAccessModification =
clazz -> consumer.accept(clazz, allowAccessModification);
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageTestBase.java b/src/test/java/com/android/tools/r8/repackage/RepackageTestBase.java
index 3e2f695..9a1ad72 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageTestBase.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageTestBase.java
@@ -6,7 +6,6 @@
import static com.android.tools.r8.shaking.ProguardConfigurationParser.FLATTEN_PACKAGE_HIERARCHY;
import static com.android.tools.r8.shaking.ProguardConfigurationParser.REPACKAGE_CLASSES;
-import static org.junit.Assert.assertFalse;
import com.android.tools.r8.R8TestBuilder;
import com.android.tools.r8.TestBase;
@@ -24,7 +23,6 @@
public abstract class RepackageTestBase extends TestBase {
- private final boolean enableExperimentalRepackaging;
private final String flattenPackageHierarchyOrRepackageClasses;
protected final TestParameters parameters;
@@ -36,15 +34,8 @@
}
public RepackageTestBase(
- String flattenPackageHierarchyOrRepackageClasses, TestParameters parameters) {
- this(true, flattenPackageHierarchyOrRepackageClasses, parameters);
- }
-
- public RepackageTestBase(
- boolean enableExperimentalRepackaging,
String flattenPackageHierarchyOrRepackageClasses,
TestParameters parameters) {
- this.enableExperimentalRepackaging = enableExperimentalRepackaging;
this.flattenPackageHierarchyOrRepackageClasses = flattenPackageHierarchyOrRepackageClasses;
this.parameters = parameters;
}
@@ -140,18 +131,8 @@
}
protected void configureRepackaging(R8TestBuilder<?> testBuilder) {
- testBuilder
- .addKeepRules(
- "-" + flattenPackageHierarchyOrRepackageClasses + " \"" + getRepackagePackage() + "\"")
- .addOptionsModification(
- options -> {
- assertFalse(options.testing.enableExperimentalRepackaging);
- options.testing.enableExperimentalRepackaging = enableExperimentalRepackaging;
- });
- }
-
- protected boolean isExperimentalRepackaging() {
- return enableExperimentalRepackaging;
+ testBuilder.addKeepRules(
+ "-" + flattenPackageHierarchyOrRepackageClasses + " \"" + getRepackagePackage() + "\"");
}
protected boolean isFlattenPackageHierarchy() {
