Automated rollback of commit 17f6894.

*** Reason for rollback *** Breaking of tap tests *** Original change description *** Add context mapping to desugar output. This allow subsequent sharding tools to group synthetics with their contexts. PiperOrigin-RevId: 644089925 Change-Id: Ie8fb4d831e08506f2ee3c96a29d9748b059bc654
bazelbuild · Jun 17, 2024 · 8d62528 · 8d62528
1 parent c3b5d86
commit 8d62528
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 94 deletions.
diff --git a/src/test/java/com/google/devtools/build/android/r8/desugar/DesugarBasicTest.java b/src/test/java/com/google/devtools/build/android/r8/desugar/DesugarBasicTest.java
@@ -15,7 +15,6 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.devtools.build.android.r8.R8Utils.INTERFACE_COMPANION_SUFFIX;
-import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.objectweb.asm.Opcodes.V1_7;
 
 import com.google.common.collect.ImmutableList;
@@ -135,15 +134,6 @@ public void checkAfterDesugarClasspath() throws Exception {
 
   @Test
   public void checkMetaDataAfterDoubleDesugaring() throws Exception {
-    String contextMap = extractContextMaps(doubleDesugaredWithDependencyMetadata);
-    assertThat(contextMap)
-        .isEqualTo(
-            String.join(
-                "",
-                "com/google/devtools/build/android/r8/desugar/basic/I$-CC;com/google/devtools/build/android/r8/desugar/basic/I\n",
-                "com/google/devtools/build/android/r8/desugar/basic/J$-CC;com/google/devtools/build/android/r8/desugar/basic/J\n",
-                "com/google/devtools/build/android/r8/desugar/basic/K$-CC;com/google/devtools/build/android/r8/desugar/basic/K\n",
-                "com/google/devtools/build/android/r8/desugar/basic/TestClass$$ExternalSyntheticLambda0;com/google/devtools/build/android/r8/desugar/basic/TestClass\n"));
     DesugarDepsInfo info = extractDesugarDeps(doubleDesugaredWithDependencyMetadata);
     assertThat(info.getInterfaceWithCompanionCount()).isEqualTo(0);
     assertThat(info.getAssumePresentCount()).isEqualTo(0);
@@ -165,14 +155,6 @@ private static DesugarDepsInfo extractDesugarDeps(Path jar) throws Exception {
     }
   }
 
-  private static String extractContextMaps(Path jar) throws Exception {
-    try (ZipFile zip = new ZipFile(jar.toFile())) {
-      ZipEntry contextMapEntry = zip.getEntry(Desugar.CONTEXT_MAP_FILENAME);
-      assertThat(contextMapEntry).isNotNull();
-      return new String(zip.getInputStream(contextMapEntry).readAllBytes(), UTF_8);
-    }
-  }
-
   private static DesugarDeps.Type classToType(Class<?> clazz) {
     return DesugarDeps.Type.newBuilder()
         .setBinaryName(DescriptorUtils.classToBinaryName(clazz))

diff --git a/src/tools/android/java/com/google/devtools/build/android/r8/CompatDexBuilder.java b/src/tools/android/java/com/google/devtools/build/android/r8/CompatDexBuilder.java
@@ -71,8 +71,6 @@
  */
 public class CompatDexBuilder {
 
-  public static final String CONTEXT_MAP_FILENAME = "META-INF/synthetic-contexts.map";
-
   private static final long ONE_MEG = 1024 * 1024;
 
   private static class ContextConsumer implements SyntheticInfoConsumer {
@@ -288,14 +286,12 @@ private void dexEntries(
         final Enumeration<? extends ZipEntry> entries = zipFile.entries();
         while (entries.hasMoreElements()) {
           ZipEntry entry = entries.nextElement();
-          if (entry.getName().endsWith(".class")) {
-            toDex.add(entry);
-          } else if (!entry.getName().equals(CONTEXT_MAP_FILENAME)) {
-            // The context mapping is rebuilt from the compiler inputs so any previous map
-            // should be removed.
+          if (!entry.getName().endsWith(".class")) {
             try (InputStream stream = zipFile.getInputStream(entry)) {
               ZipUtils.addEntry(entry.getName(), stream, out);
             }
+          } else {
+            toDex.add(entry);
           }
         }
 
@@ -327,7 +323,10 @@ private void dexEntries(
         String contextMapping = contextMappingBuilder.toString();
         if (!contextMapping.isEmpty()) {
           ZipUtils.addEntry(
-              CONTEXT_MAP_FILENAME, contextMapping.getBytes(UTF_8), ZipEntry.STORED, out);
+              "META-INF/synthetic-contexts.map",
+              contextMapping.getBytes(UTF_8),
+              ZipEntry.STORED,
+              out);
         }
       }
     } finally {

diff --git a/src/tools/android/java/com/google/devtools/build/android/r8/Desugar.java b/src/tools/android/java/com/google/devtools/build/android/r8/Desugar.java
@@ -57,8 +57,6 @@
 public class Desugar {
 
   public static final String DESUGAR_DEPS_FILENAME = "META-INF/desugar_deps";
-  public static final String CONTEXT_MAP_FILENAME = "META-INF/synthetic-contexts.map";
-
   // We shard the compilation if we have more than this number of entries to avoid timing out.
   private static final int NUMBER_OF_ENTRIES_PER_SHARD = 100000;
   private static final Logger logger = Logger.getLogger(Desugar.class.getName());
@@ -513,7 +511,6 @@ private void desugar(
               .addProgramResourceProvider(programProvider)
               .setIntermediate(true)
               .setMinApiLevel(options.minSdkVersion)
-              .setSyntheticInfoConsumer(consumer.getContextConsumer())
               .setProgramConsumer(consumer);
       bootclasspathProviders.forEach(builder::addLibraryResourceProvider);
       if (desugaredLibConfig != null) {

diff --git a/src/tools/android/java/com/google/devtools/build/android/r8/desugar/OutputConsumer.java b/src/tools/android/java/com/google/devtools/build/android/r8/desugar/OutputConsumer.java
@@ -16,7 +16,6 @@
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.devtools.build.android.r8.desugar.OutputConsumer.Flags.EXCLUDE_PATH_ENTRIES;
 import static com.google.devtools.build.android.r8.desugar.OutputConsumer.Flags.INCLUDE_PATH_ENTRIES;
-import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.objectweb.asm.Opcodes.ACC_BRIDGE;
 import static org.objectweb.asm.Opcodes.ACC_INTERFACE;
 import static org.objectweb.asm.Opcodes.INVOKESTATIC;
@@ -25,11 +24,8 @@
 import com.android.tools.r8.ByteDataView;
 import com.android.tools.r8.ClassFileConsumer;
 import com.android.tools.r8.DiagnosticsHandler;
-import com.android.tools.r8.SyntheticInfoConsumer;
-import com.android.tools.r8.SyntheticInfoConsumerData;
 import com.android.tools.r8.origin.Origin;
 import com.android.tools.r8.origin.PathOrigin;
-import com.android.tools.r8.references.ClassReference;
 import com.android.tools.r8.utils.ExceptionDiagnostic;
 import com.google.common.io.ByteStreams;
 import com.google.devtools.build.android.r8.DependencyCollector;
@@ -42,12 +38,9 @@
 import java.io.InputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.ArrayList;
-import java.util.Comparator;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Map;
 import java.util.NavigableSet;
 import java.util.Set;
@@ -86,31 +79,6 @@ public void finished(DiagnosticsHandler handler) {
     if (!finished()) {
       return;
     }
-
-    byte[] contextMappingContent;
-    Map<ClassReference, Set<ClassReference>> contextToSyntheticsMap =
-        getContextConsumer().contextToSyntheticsMap;
-    if (contextToSyntheticsMap.isEmpty()) {
-      contextMappingContent = null;
-    } else {
-      StringBuilder contextMappingBuilder = new StringBuilder();
-      List<ClassReference> contexts = new ArrayList<>(contextToSyntheticsMap.keySet());
-      contexts.sort(Comparator.comparing(ClassReference::getDescriptor));
-      contexts.forEach(
-          context -> {
-            List<ClassReference> synthetics = new ArrayList<>(contextToSyntheticsMap.get(context));
-            synthetics.sort(Comparator.comparing(ClassReference::getDescriptor));
-            synthetics.forEach(
-                synthetic ->
-                    contextMappingBuilder
-                        .append(synthetic.getBinaryName())
-                        .append(';')
-                        .append(context.getBinaryName())
-                        .append('\n'));
-          });
-      contextMappingContent = contextMappingBuilder.toString().getBytes(UTF_8);
-    }
-
     FilteringDependencyCollector dependencyCollector =
         new FilteringDependencyCollector(this.dependencyCollector);
     initializeInputDependencies(handler, dependencyCollector);
@@ -121,11 +89,6 @@ public void finished(DiagnosticsHandler handler) {
         ZipUtils.addEntry(classFile.fileName, classFile.data, ZipEntry.STORED, out);
         new DesugaredClassFileDependencyCollector(classFile, dependencyCollector).run();
       }
-      // Add context-mapping if required.
-      if (contextMappingContent != null) {
-        ZipUtils.addEntry(
-            Desugar.CONTEXT_MAP_FILENAME, contextMappingContent, ZipEntry.STORED, out);
-      }
       // Add dependency metadata if required.
       byte[] desugarDeps = dependencyCollector.toByteArray();
       if (desugarDeps != null) {
@@ -135,13 +98,9 @@ public void finished(DiagnosticsHandler handler) {
           input,
           out,
           entryName ->
-              (entryName.equals("module-info.class")
-                  // The context mapping is rebuilt from the compiler inputs so any previous map
-                  // should be removed.
-                  || entryName.equals(Desugar.CONTEXT_MAP_FILENAME)
-                  || entryName.startsWith("META-INF/versions/")
+              ("module-info.class".equals(entryName) || entryName.startsWith("META-INF/versions/"))
                   || ArchiveProgramResourceProvider.includeClassFileEntries(entryName)
-                  || (entryName.endsWith("/") && flags == EXCLUDE_PATH_ENTRIES)),
+                  || (entryName.endsWith("/") && flags == EXCLUDE_PATH_ENTRIES),
           name -> {
             final String metainfServicesPrefix = "META-INF/services/";
             if (name.startsWith(metainfServicesPrefix)) {
@@ -183,29 +142,11 @@ public int compareTo(ClassFileData other) {
     }
   }
 
-  private static class ContextConsumer implements SyntheticInfoConsumer {
-
-    final Map<ClassReference, Set<ClassReference>> contextToSyntheticsMap = new HashMap<>();
-
-    @Override
-    public synchronized void acceptSyntheticInfo(SyntheticInfoConsumerData data) {
-      contextToSyntheticsMap
-          .computeIfAbsent(data.getSynthesizingContextClass(), k -> new HashSet<>())
-          .add(data.getSyntheticClass());
-    }
-
-    @Override
-    public void finished() {
-      // Do nothing.
-    }
-  }
-
   private final Path archive;
   private final Origin origin;
   private final DependencyCollector dependencyCollector;
   private final Path input;
   private final Flags flags;
-  private final ContextConsumer contextConsumer = new ContextConsumer();
 
   private final NavigableSet<ClassFileData> classFiles = new TreeSet<>();
   private boolean finish = true;
@@ -223,10 +164,6 @@ public OutputConsumer(
     this.flags = flags;
   }
 
-  public ContextConsumer getContextConsumer() {
-    return contextConsumer;
-  }
-
   @Override
   public void accept(ByteDataView data, String descriptor, DiagnosticsHandler handler) {
     classFiles.add(