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