Write META-INF/MANIFEST.MF entries at the beginning of jars

This is consistent with assumptions made by jar implementations, for example [`JarInputStream#getManifest`](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/jar/JarInputStream.html#getManifest()) only checks the first two entries. PiperOrigin-RevId: 604133807
google · Feb 4, 2024 · 3a9d792 · 3a9d792
1 parent bc7e3ca
commit 3a9d792
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 17 deletions.
diff --git a/java/com/google/turbine/main/Main.java b/java/com/google/turbine/main/Main.java
@@ -369,10 +369,10 @@ private static void writeSources(
     try (OutputStream os = Files.newOutputStream(path);
         BufferedOutputStream bos = new BufferedOutputStream(os, BUFFER_SIZE);
         JarOutputStream jos = new JarOutputStream(bos)) {
+      writeManifest(jos, manifest());
       for (SourceFile source : generatedSources.values()) {
         addEntry(jos, source.path(), source.source().getBytes(UTF_8));
       }
-      writeManifest(jos, manifest());
     }
   }
 
@@ -412,18 +412,18 @@ private static void writeOutput(
     try (OutputStream os = Files.newOutputStream(path);
         BufferedOutputStream bos = new BufferedOutputStream(os, BUFFER_SIZE);
         JarOutputStream jos = new JarOutputStream(bos)) {
-      for (Map.Entry<String, byte[]> entry : lowered.entrySet()) {
-        addEntry(jos, entry.getKey() + ".class", entry.getValue());
-      }
-      for (Map.Entry<String, byte[]> entry : generated.entrySet()) {
-        addEntry(jos, entry.getKey(), entry.getValue());
+      if (options.targetLabel().isPresent()) {
+        writeManifest(jos, manifest(options));
       }
       for (Map.Entry<String, byte[]> entry : transitive.entrySet()) {
         addEntry(
             jos, ClassPathBinder.TRANSITIVE_PREFIX + entry.getKey() + ".class", entry.getValue());
       }
-      if (options.targetLabel().isPresent()) {
-        writeManifest(jos, manifest(options));
+      for (Map.Entry<String, byte[]> entry : lowered.entrySet()) {
+        addEntry(jos, entry.getKey() + ".class", entry.getValue());
+      }
+      for (Map.Entry<String, byte[]> entry : generated.entrySet()) {
+        addEntry(jos, entry.getKey(), entry.getValue());
       }
     }
   }

diff --git a/javatests/com/google/turbine/deps/TransitiveTest.java b/javatests/com/google/turbine/deps/TransitiveTest.java
@@ -125,10 +125,13 @@ public void transitive() throws Exception {
     // libb repackages A, and any member types
     assertThat(readJar(libb).keySet())
         .containsExactly(
-            "b/B.class",
+            "META-INF/",
+            "META-INF/MANIFEST.MF",
             "META-INF/TRANSITIVE/a/A.class",
             "META-INF/TRANSITIVE/a/A$Anno.class",
-            "META-INF/TRANSITIVE/a/A$Inner.class");
+            "META-INF/TRANSITIVE/a/A$Inner.class",
+            "b/B.class")
+        .inOrder();
 
     ClassFile a = ClassReader.read(null, readJar(libb).get("META-INF/TRANSITIVE/a/A.class"));
     // methods and non-constant fields are removed
@@ -176,15 +179,19 @@ public void transitive() throws Exception {
                 ImmutableList.of(libb).stream().map(Path::toString).collect(toImmutableList()))
             .setOutput(libc.toString())
             .setOutputDeps(libcDeps.toString())
+            .setTargetLabel("//foo:foo")
             .build());
 
     assertThat(readJar(libc).keySet())
         .containsExactly(
-            "c/C.class",
+            "META-INF/",
+            "META-INF/MANIFEST.MF",
             "META-INF/TRANSITIVE/b/B.class",
             "META-INF/TRANSITIVE/a/A.class",
             "META-INF/TRANSITIVE/a/A$Anno.class",
-            "META-INF/TRANSITIVE/a/A$Inner.class");
+            "META-INF/TRANSITIVE/a/A$Inner.class",
+            "c/C.class")
+        .inOrder();
 
     // liba is recorded as an explicit dep, even thought it's only present as a transitive class
     // repackaged in lib
@@ -247,7 +254,12 @@ public void anonymous() throws Exception {
     // libb repackages A and any named member types
     assertThat(readJar(libb).keySet())
         .containsExactly(
-            "b/B.class", "META-INF/TRANSITIVE/a/A.class", "META-INF/TRANSITIVE/a/A$I.class");
+            "META-INF/",
+            "META-INF/MANIFEST.MF",
+            "META-INF/TRANSITIVE/a/A.class",
+            "META-INF/TRANSITIVE/a/A$I.class",
+            "b/B.class")
+        .inOrder();
   }
 
   @Test
@@ -283,11 +295,14 @@ public void childClass() throws Exception {
 
     assertThat(readJar(libb).keySet())
         .containsExactly(
-            "b/B.class",
-            "b/B$I.class",
-            "META-INF/TRANSITIVE/a/A.class",
+            "META-INF/",
+            "META-INF/MANIFEST.MF",
             "META-INF/TRANSITIVE/a/A$I.class",
-            "META-INF/TRANSITIVE/a/S.class");
+            "META-INF/TRANSITIVE/a/S.class",
+            "META-INF/TRANSITIVE/a/A.class",
+            "b/B$I.class",
+            "b/B.class")
+        .inOrder();
   }
 
   private Path runTurbine(ImmutableList<Path> sources, ImmutableList<Path> classpath)
@@ -298,6 +313,7 @@ private Path runTurbine(ImmutableList<Path> sources, ImmutableList<Path> classpa
             .setSources(sources.stream().map(Path::toString).collect(toImmutableList()))
             .setClassPath(classpath.stream().map(Path::toString).collect(toImmutableList()))
             .setOutput(out.toString())
+            .setTargetLabel("//foo:foo")
             .build());
     return out;
   }

diff --git a/javatests/com/google/turbine/main/MainTest.java b/javatests/com/google/turbine/main/MainTest.java
@@ -225,6 +225,11 @@ public void testManifest() throws IOException {
                   .toInstant())
           .isEqualTo(
               LocalDateTime.of(2010, 1, 1, 0, 0, 0).atZone(ZoneId.systemDefault()).toInstant());
+      // JarInputStream#getManifest only checks the first two entries for the manifest, so ensure
+      // that turbine writes jars with the manifest at the beginning
+      assertThat(jarFile.stream().limit(2).map(JarEntry::getName))
+          .containsExactly("META-INF/", "META-INF/MANIFEST.MF")
+          .inOrder();
     }
     try (JarFile jarFile = new JarFile(gensrcOutput.toFile())) {
       Manifest manifest = requireNonNull(jarFile.getManifest());
@@ -236,6 +241,9 @@ public void testManifest() throws IOException {
           .containsExactly(
               "Created-By", "bazel",
               "Manifest-Version", "1.0");
+      assertThat(jarFile.stream().limit(2).map(JarEntry::getName))
+          .containsExactly("META-INF/", "META-INF/MANIFEST.MF")
+          .inOrder();
     }
   }