Repackage when having -dontobfuscate by moving package to destination
Bug: b/241220445
Change-Id: Ib012e9308e7c9fb8cd1f2b3a8c4b02a4d52c5c28
diff --git a/src/main/java/com/android/tools/r8/repackaging/Repackaging.java b/src/main/java/com/android/tools/r8/repackaging/Repackaging.java
index 8545462..f292409 100644
--- a/src/main/java/com/android/tools/r8/repackaging/Repackaging.java
+++ b/src/main/java/com/android/tools/r8/repackaging/Repackaging.java
@@ -347,7 +347,16 @@
DescriptorUtils.getBinaryNameFromJavaType(proguardConfiguration.getPackagePrefix());
PackageObfuscationMode packageObfuscationMode =
proguardConfiguration.getPackageObfuscationMode();
- if (packageObfuscationMode.isRepackageClasses()) {
+ if (!appView.options().isMinifying()) {
+ // Preserve full package name under destination package when not minifying
+ // (no matter which package obfuscation mode is used).
+ if (newPackageDescriptor.isEmpty()
+ || proguardConfiguration.getKeepPackageNamesPatterns().matches(pkg)
+ || mayHavePinnedPackagePrivateOrProtectedItem(pkg)) {
+ return pkg.getPackageDescriptor();
+ }
+ return newPackageDescriptor + DESCRIPTOR_PACKAGE_SEPARATOR + pkg.getPackageDescriptor();
+ } else if (packageObfuscationMode.isRepackageClasses()) {
return newPackageDescriptor;
} else if (packageObfuscationMode.isMinification()) {
// Always keep top-level classes since their packages can never be minified.
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java
index 7199fcb..9785028 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepClassInfo.java
@@ -61,7 +61,7 @@
public boolean isRepackagingAllowed(
ProgramDefinition definition, GlobalKeepInfoConfiguration configuration) {
return configuration.isRepackagingEnabled()
- && internalIsMinificationAllowed()
+ && internalIsRepackagingAllowed()
&& (definition.getAccessFlags().isPublic()
|| !internalIsAccessModificationRequiredForRepackaging());
}
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
index 9595b06..d9708ce 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
@@ -22,6 +22,7 @@
private final boolean allowAnnotationRemoval;
private final boolean allowMinification;
private final boolean allowOptimization;
+ private final boolean allowRepackaging;
private final boolean allowShrinking;
private final boolean allowSignatureRemoval;
private final boolean checkDiscarded;
@@ -32,6 +33,7 @@
boolean allowAnnotationRemoval,
boolean allowMinification,
boolean allowOptimization,
+ boolean allowRepackaging,
boolean allowShrinking,
boolean allowSignatureRemoval,
boolean checkDiscarded,
@@ -40,6 +42,7 @@
this.allowAnnotationRemoval = allowAnnotationRemoval;
this.allowMinification = allowMinification;
this.allowOptimization = allowOptimization;
+ this.allowRepackaging = allowRepackaging;
this.allowShrinking = allowShrinking;
this.allowSignatureRemoval = allowSignatureRemoval;
this.checkDiscarded = checkDiscarded;
@@ -52,6 +55,7 @@
builder.isAnnotationRemovalAllowed(),
builder.isMinificationAllowed(),
builder.isOptimizationAllowed(),
+ builder.isRepackagingAllowed(),
builder.isShrinkingAllowed(),
builder.isSignatureRemovalAllowed(),
builder.isCheckDiscardedEnabled(),
@@ -168,6 +172,10 @@
public abstract boolean isRepackagingAllowed(
ProgramDefinition definition, GlobalKeepInfoConfiguration configuration);
+ boolean internalIsRepackagingAllowed() {
+ return allowRepackaging;
+ }
+
boolean internalIsAccessModificationRequiredForRepackaging() {
return requireAccessModificationForRepackaging;
}
@@ -234,6 +242,7 @@
&& (allowAnnotationRemoval || !other.internalIsAnnotationRemovalAllowed())
&& (allowMinification || !other.internalIsMinificationAllowed())
&& (allowOptimization || !other.internalIsOptimizationAllowed())
+ && (allowRepackaging || !other.internalIsRepackagingAllowed())
&& (allowShrinking || !other.internalIsShrinkingAllowed())
&& (allowSignatureRemoval || !other.internalIsSignatureRemovalAllowed())
&& (!checkDiscarded || other.internalIsCheckDiscardedEnabled());
@@ -256,6 +265,7 @@
private boolean allowAccessModification;
private boolean allowAnnotationRemoval;
private boolean allowMinification;
+ private boolean allowRepackaging;
private boolean allowOptimization;
private boolean allowShrinking;
private boolean allowSignatureRemoval;
@@ -272,6 +282,7 @@
allowAnnotationRemoval = original.internalIsAnnotationRemovalAllowed();
allowMinification = original.internalIsMinificationAllowed();
allowOptimization = original.internalIsOptimizationAllowed();
+ allowRepackaging = original.internalIsRepackagingAllowed();
allowShrinking = original.internalIsShrinkingAllowed();
allowSignatureRemoval = original.internalIsSignatureRemovalAllowed();
checkDiscarded = original.internalIsCheckDiscardedEnabled();
@@ -284,6 +295,7 @@
disallowAnnotationRemoval();
disallowMinification();
disallowOptimization();
+ disallowRepackaging();
disallowShrinking();
disallowSignatureRemoval();
unsetCheckDiscarded();
@@ -296,6 +308,7 @@
allowAnnotationRemoval();
allowMinification();
allowOptimization();
+ allowRepackaging();
allowShrinking();
allowSignatureRemoval();
unsetCheckDiscarded();
@@ -323,6 +336,7 @@
&& isAnnotationRemovalAllowed() == other.internalIsAnnotationRemovalAllowed()
&& isMinificationAllowed() == other.internalIsMinificationAllowed()
&& isOptimizationAllowed() == other.internalIsOptimizationAllowed()
+ && isRepackagingAllowed() == other.internalIsRepackagingAllowed()
&& isShrinkingAllowed() == other.internalIsShrinkingAllowed()
&& isSignatureRemovalAllowed() == other.internalIsSignatureRemovalAllowed()
&& isCheckDiscardedEnabled() == other.internalIsCheckDiscardedEnabled()
@@ -354,6 +368,10 @@
return allowOptimization;
}
+ public boolean isRepackagingAllowed() {
+ return allowRepackaging;
+ }
+
public boolean isShrinkingAllowed() {
return allowShrinking;
}
@@ -375,6 +393,19 @@
return setAllowMinification(false);
}
+ public B setAllowRepackaging(boolean allowRepackaging) {
+ this.allowRepackaging = allowRepackaging;
+ return self();
+ }
+
+ public B allowRepackaging() {
+ return setAllowRepackaging(true);
+ }
+
+ public B disallowRepackaging() {
+ return setAllowRepackaging(false);
+ }
+
public B setAllowOptimization(boolean allowOptimization) {
this.allowOptimization = allowOptimization;
return self();
@@ -577,6 +608,11 @@
return self();
}
+ public J disallowRepackaging() {
+ builder.disallowRepackaging();
+ return self();
+ }
+
public J disallowOptimization() {
builder.disallowOptimization();
return self();
@@ -608,6 +644,7 @@
applyIf(!builder.isAnnotationRemovalAllowed(), Joiner::disallowAnnotationRemoval);
applyIf(!builder.isMinificationAllowed(), Joiner::disallowMinification);
applyIf(!builder.isOptimizationAllowed(), Joiner::disallowOptimization);
+ applyIf(!builder.isRepackagingAllowed(), Joiner::disallowRepackaging);
applyIf(!builder.isShrinkingAllowed(), Joiner::disallowShrinking);
applyIf(!builder.isSignatureRemovalAllowed(), Joiner::disallowSignatureRemoval);
applyIf(builder.isCheckDiscardedEnabled(), Joiner::setCheckDiscarded);
diff --git a/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java b/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java
index d804fa6..7275e72 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetUtils.java
@@ -1133,7 +1133,8 @@
if (appView.options().isMinificationEnabled() && !modifiers.allowsObfuscation) {
dependentMinimumKeepInfo
.getOrCreateMinimumKeepInfoFor(preconditionEvent, clazz.getReference())
- .disallowMinification();
+ .disallowMinification()
+ .disallowRepackaging();
}
}
@@ -1635,7 +1636,15 @@
if (appView.options().isMinificationEnabled() && !modifiers.allowsObfuscation) {
dependentMinimumKeepInfo
.getOrCreateMinimumKeepInfoFor(preconditionEvent, item.getReference())
- .disallowMinification();
+ .disallowMinification()
+ .disallowRepackaging();
+ context.markAsUsed();
+ }
+
+ if (appView.options().isRepackagingEnabled() && !modifiers.allowsObfuscation) {
+ dependentMinimumKeepInfo
+ .getOrCreateMinimumKeepInfoFor(preconditionEvent, item.getReference())
+ .disallowRepackaging();
context.markAsUsed();
}
@@ -1971,7 +1980,8 @@
void shouldNotBeMinified(ProgramDefinition definition) {
getDependentMinimumKeepInfo()
.getOrCreateUnconditionalMinimumKeepInfoFor(definition.getReference())
- .disallowMinification();
+ .disallowMinification()
+ .disallowRepackaging();
}
public boolean verifyKeptFieldsAreAccessedAndLive(AppView<AppInfoWithLiveness> appView) {
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 f5e86e9..4653b96 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -696,7 +696,10 @@
@Override
public boolean isRepackagingEnabled() {
- return proguardConfiguration.getPackageObfuscationMode().isSome() && isMinifying();
+ return !debug
+ && proguardConfiguration != null
+ && proguardConfiguration.getPackageObfuscationMode().isSome()
+ && (isMinifying() || !isForceProguardCompatibilityEnabled());
}
@Override
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageClassWithKeepPackageNameOnTargetTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageClassWithDontObfuscateKeepPackageNameOnTargetTest.java
similarity index 67%
copy from src/test/java/com/android/tools/r8/repackage/RepackageClassWithKeepPackageNameOnTargetTest.java
copy to src/test/java/com/android/tools/r8/repackage/RepackageClassWithDontObfuscateKeepPackageNameOnTargetTest.java
index ce8088f..6fa0787 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageClassWithKeepPackageNameOnTargetTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageClassWithDontObfuscateKeepPackageNameOnTargetTest.java
@@ -5,9 +5,10 @@
package com.android.tools.r8.repackage;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.CoreMatchers.startsWith;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertEquals;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.TestParameters;
@@ -18,11 +19,11 @@
import org.junit.runners.Parameterized;
@RunWith(Parameterized.class)
-public class RepackageClassWithKeepPackageNameOnTargetTest extends RepackageTestBase {
+public class RepackageClassWithDontObfuscateKeepPackageNameOnTargetTest extends RepackageTestBase {
private static final String DESTINATION_PACKAGE = "other.package";
- public RepackageClassWithKeepPackageNameOnTargetTest(
+ public RepackageClassWithDontObfuscateKeepPackageNameOnTargetTest(
String flattenPackageHierarchyOrRepackageClasses, TestParameters parameters) {
super(flattenPackageHierarchyOrRepackageClasses, parameters);
}
@@ -34,29 +35,25 @@
@Test
public void testR8() throws Exception {
- // TODO(b/241220445): Should be able to relocate when having -dontobfuscate in some way to
- // support mainline.
+ String originalPackage = DescriptorUtils.getPackageNameFromBinaryName(binaryName(Foo.class));
testForR8(parameters.getBackend())
.addInnerClasses(getClass())
.setMinApi(parameters.getApiLevel())
.addKeepMainRule(Main.class)
.enableInliningAnnotations()
.apply(this::configureRepackaging)
+ .addDontObfuscate()
+ .addKeepPackageNamesRule(originalPackage)
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("Foo::foo()")
.inspect(
inspector -> {
ClassSubject clazz = inspector.clazz(Foo.class);
assertThat(clazz, isPresent());
- assertThat(clazz.getFinalName(), startsWith(DESTINATION_PACKAGE));
- String relocatedPackageSuffix =
- DescriptorUtils.getPackageBinaryNameFromJavaType(
- clazz.getFinalName().substring(DESTINATION_PACKAGE.length() + 1));
- String originalPackage =
- DescriptorUtils.getPackageBinaryNameFromJavaType(clazz.getOriginalName());
- // TODO(b/241220445): Have a configuration where the suffix is identicial to the
- // original.
- assertNotEquals(relocatedPackageSuffix, originalPackage);
+ assertThat(clazz.getFinalName(), not(startsWith(DESTINATION_PACKAGE)));
+ String finalPackage =
+ DescriptorUtils.getPackageNameFromBinaryName(clazz.getFinalBinaryName());
+ assertEquals(originalPackage, finalPackage);
});
}
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageClassWithKeepPackageNameOnTargetTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageClassesWithDontObfuscateTest.java
similarity index 81%
rename from src/test/java/com/android/tools/r8/repackage/RepackageClassWithKeepPackageNameOnTargetTest.java
rename to src/test/java/com/android/tools/r8/repackage/RepackageClassesWithDontObfuscateTest.java
index ce8088f..3a36aea 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageClassWithKeepPackageNameOnTargetTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageClassesWithDontObfuscateTest.java
@@ -7,7 +7,7 @@
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
import static org.hamcrest.CoreMatchers.startsWith;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertEquals;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.TestParameters;
@@ -18,11 +18,11 @@
import org.junit.runners.Parameterized;
@RunWith(Parameterized.class)
-public class RepackageClassWithKeepPackageNameOnTargetTest extends RepackageTestBase {
+public class RepackageClassesWithDontObfuscateTest extends RepackageTestBase {
private static final String DESTINATION_PACKAGE = "other.package";
- public RepackageClassWithKeepPackageNameOnTargetTest(
+ public RepackageClassesWithDontObfuscateTest(
String flattenPackageHierarchyOrRepackageClasses, TestParameters parameters) {
super(flattenPackageHierarchyOrRepackageClasses, parameters);
}
@@ -34,29 +34,26 @@
@Test
public void testR8() throws Exception {
- // TODO(b/241220445): Should be able to relocate when having -dontobfuscate in some way to
- // support mainline.
testForR8(parameters.getBackend())
.addInnerClasses(getClass())
.setMinApi(parameters.getApiLevel())
.addKeepMainRule(Main.class)
.enableInliningAnnotations()
.apply(this::configureRepackaging)
+ .addDontObfuscate()
.run(parameters.getRuntime(), Main.class)
.assertSuccessWithOutputLines("Foo::foo()")
.inspect(
inspector -> {
ClassSubject clazz = inspector.clazz(Foo.class);
assertThat(clazz, isPresent());
- assertThat(clazz.getFinalName(), startsWith(DESTINATION_PACKAGE));
+ assertThat(clazz.getFinalName(), startsWith(DESTINATION_PACKAGE + "."));
String relocatedPackageSuffix =
DescriptorUtils.getPackageBinaryNameFromJavaType(
clazz.getFinalName().substring(DESTINATION_PACKAGE.length() + 1));
String originalPackage =
DescriptorUtils.getPackageBinaryNameFromJavaType(clazz.getOriginalName());
- // TODO(b/241220445): Have a configuration where the suffix is identicial to the
- // original.
- assertNotEquals(relocatedPackageSuffix, originalPackage);
+ assertEquals(relocatedPackageSuffix, originalPackage);
});
}
diff --git a/src/test/java/com/android/tools/r8/repackage/RepackageDontObfuscateTest.java b/src/test/java/com/android/tools/r8/repackage/RepackageDontObfuscateTest.java
index 5ece41d..39b3d43 100644
--- a/src/test/java/com/android/tools/r8/repackage/RepackageDontObfuscateTest.java
+++ b/src/test/java/com/android/tools/r8/repackage/RepackageDontObfuscateTest.java
@@ -7,11 +7,14 @@
import static com.android.tools.r8.shaking.ProguardConfigurationParser.FLATTEN_PACKAGE_HIERARCHY;
import static com.android.tools.r8.shaking.ProguardConfigurationParser.REPACKAGE_CLASSES;
import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndNotRenamed;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndRenamed;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assume.assumeTrue;
import com.android.tools.r8.NeverInline;
import com.android.tools.r8.ProguardVersion;
+import com.android.tools.r8.R8TestBuilder;
+import com.android.tools.r8.R8TestCompileResult;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.utils.codeinspector.ClassSubject;
import com.google.common.collect.ImmutableList;
@@ -70,15 +73,23 @@
}
@Test
- public void testR8() throws Exception {
- testForR8(parameters.getBackend())
- .addInnerClasses(getClass())
- .setMinApi(parameters.getApiLevel())
- .addKeepMainRule(Main.class)
- .enableInliningAnnotations()
- .apply(this::configureRepackaging)
- .noMinification()
- .compile()
+ public void testR8Full() throws Exception {
+ setup(testForR8(parameters.getBackend()))
+ .inspect(
+ inspector -> {
+ ClassSubject aClass = inspector.clazz(A.class);
+ assertThat(aClass, isPresentAndRenamed());
+ })
+ .run(
+ parameters.getRuntime(),
+ Main.class,
+ getRepackagePackage() + "." + A.class.getTypeName())
+ .assertSuccessWithOutputLines(EXPECTED);
+ }
+
+ @Test
+ public void testR8Compat() throws Exception {
+ setup(testForR8Compat(parameters.getBackend()))
.inspect(
inspector -> {
ClassSubject aClass = inspector.clazz(A.class);
@@ -88,6 +99,17 @@
.assertSuccessWithOutputLines(EXPECTED);
}
+ private R8TestCompileResult setup(R8TestBuilder<?> r8TestBuilder) throws Exception {
+ return r8TestBuilder
+ .addInnerClasses(getClass())
+ .setMinApi(parameters.getApiLevel())
+ .addKeepMainRule(Main.class)
+ .enableInliningAnnotations()
+ .apply(this::configureRepackaging)
+ .noMinification()
+ .compile();
+ }
+
public static class A {
@NeverInline