Refactored JAR creation code and create dir entries by default (#2138)

Move JAR creation code into separate `JarOps` object to create a
`os-lib` like user experience.

Added support for creating directory entries, to fix #2136

Also prepared to use fixed timestamps to assist creating reproducible
jars.

Added use of buffered stream when writing the file.

Pull request: #2138

main/api/src/mill/api/JarOps.scala

@@ -0,0 +1,85 @@
+package mill.api
+
+import mill.api.Loose.Agg
+
+import java.io.{BufferedOutputStream, FileOutputStream}
+import java.util.jar.{JarEntry, JarOutputStream, Manifest}
+import scala.collection.mutable
+
+@experimental
+trait JarOps {
+
+  /**
+   * Create a JAR file with default inflation level.
+   * d
+   * @param jar The final JAR file
+   * @param inputPaths The input paths resembling the content of the JAR file.
+   *     Files will be directly included in the root of the archive,
+   *     whereas for directories their content is added to the root of the archive.
+   * @param manifest The JAR Manifest
+   * @param fileFilter A filter to support exclusions of selected files
+   * @param includeDirs If `true` the JAR archive will contain directory entries.
+   *                    According to the ZIP specification, directory entries are not required.
+   *                    In the Java ecosystem, most JARs have directory entries, so including them may reduce compatibility issues.
+   *                    Directory entry names will result with a trailing `/`.
+   * @param timestamp If specified, this timestamp is used as modification timestamp (mtime) for all entries in the JAR file.
+   *                  Having a stable timestamp may result in reproducible files, if all other content, including the JAR Manifest, keep stable.
+   */
+  def jar(
+      jar: os.Path,
+      inputPaths: Agg[os.Path],
+      manifest: Manifest,
+      fileFilter: (os.Path, os.RelPath) => Boolean = (_, _) => true,
+      includeDirs: Boolean = false,
+      timestamp: Option[Long] = None
+  ): Unit = {
+
+    val curTime = timestamp.getOrElse(System.currentTimeMillis())
+    def mTime(file: os.Path) = timestamp.getOrElse(os.mtime(file))
+
+    os.makeDir.all(jar / os.up)
+    os.remove.all(jar)
+
+    val seen = mutable.Set.empty[os.RelPath]
+    seen.add(os.sub / "META-INF" / "MANIFEST.MF")
+
+    val jarStream = new JarOutputStream(
+      new BufferedOutputStream(new FileOutputStream(jar.toIO)),
+      manifest
+    )
+
+    try {
+      assert(inputPaths.iterator.forall(os.exists(_)))
+
+      if (includeDirs) {
+        val entry = new JarEntry("META-INF/")
+        entry.setTime(curTime)
+        jarStream.putNextEntry(entry)
+        jarStream.closeEntry()
+      }
+
+      // Note: we only sort each input path, but not the whole archive
+      for {
+        p <- inputPaths
+        (file, mapping) <-
+          if (os.isFile(p)) Seq((p, os.sub / p.last))
+          else os.walk(p).map(sub => (sub, sub.subRelativeTo(p))).sorted
+        if (includeDirs || os.isFile(file)) && !seen(mapping) && fileFilter(p, mapping)
+      } {
+        seen.add(mapping)
+        val name = mapping.toString() + (if (os.isDir(file)) "/" else "")
+        val entry = new JarEntry(name)
+        entry.setTime(mTime(file))
+        jarStream.putNextEntry(entry)
+        if (os.isFile(file)) jarStream.write(os.read.bytes(file))
+        jarStream.closeEntry()
+      }
+    } finally {
+      jarStream.close()
+    }
+  }
+
+}
+
+@experimental
+object JarOps extends JarOps

main/src/mill/modules/Jvm.scala

@@ -1,38 +1,28 @@
 package mill.modules
 
 import coursier.cache.ArtifactError
+import coursier.util.{Gather, Task}
+import coursier.{Dependency, Repository, Resolution}
+import mill.BuildInfo
+import mill.api.Loose.Agg
+import mill.api._
+import mill.main.client.InputPumper
+import mill.modules.Assembly.{AppendEntry, WriteOnceEntry}
+import os.SubProcess
+import upickle.default.{ReadWriter => RW}
 
-import java.io.{
-  ByteArrayInputStream,
-  File,
-  FileOutputStream,
-  InputStream,
-  PipedInputStream,
-  SequenceInputStream
-}
+import java.io._
 import java.lang.reflect.Modifier
 import java.net.URI
-import java.nio.file.{FileSystems, Files, NoSuchFileException, StandardOpenOption}
 import java.nio.file.attribute.PosixFilePermission
-import java.util.jar.{Attributes, JarEntry, JarFile, JarOutputStream, Manifest}
-import coursier.{Dependency, Repository, Resolution}
-import coursier.util.{Gather, Task}
-
+import java.nio.file.{FileSystems, Files, NoSuchFileException, StandardOpenOption}
 import java.util.Collections
-import mill.main.client.InputPumper
-import mill.api.{Ctx, IO, PathRef, Result}
-import mill.api.Loose.Agg
-import mill.modules.Assembly.{AppendEntry, WriteOnceEntry}
-
+import java.util.jar.{Attributes, JarFile, Manifest}
+import scala.annotation.tailrec
 import scala.collection.mutable
-import scala.util.Properties.isWin
 import scala.jdk.CollectionConverters._
+import scala.util.Properties.isWin
 import scala.util.{Failure, Success, Try, Using}
-import mill.BuildInfo
-import os.SubProcess
-import upickle.default.{ReadWriter => RW}
-
-import scala.annotation.tailrec
 
 object Jvm {
 
@@ -339,38 +329,8 @@ object Jvm {
       inputPaths: Agg[os.Path],
       manifest: JarManifest,
       fileFilter: (os.Path, os.RelPath) => Boolean
-  ): Unit = {
-    os.makeDir.all(jar / os.up)
-    os.remove.all(jar)
-
-    val seen = mutable.Set.empty[os.RelPath]
-    seen.add(os.rel / "META-INF" / "MANIFEST.MF")
-
-    val jarStream = new JarOutputStream(
-      new FileOutputStream(jar.toIO),
-      manifest.build
-    )
-
-    try {
-      assert(inputPaths.iterator.forall(os.exists(_)))
-      for {
-        p <- inputPaths
-        (file, mapping) <-
-          if (os.isFile(p)) Iterator(p -> os.rel / p.last)
-          else os.walk(p).filter(os.isFile).map(sub => sub -> sub.relativeTo(p)).sorted
-        if !seen(mapping) && fileFilter(p, mapping)
-      } {
-        seen.add(mapping)
-        val entry = new JarEntry(mapping.toString)
-        entry.setTime(os.mtime(file))
-        jarStream.putNextEntry(entry)
-        jarStream.write(os.read.bytes(file))
-        jarStream.closeEntry()
-      }
-    } finally {
-      jarStream.close()
-    }
-  }
+  ): Unit =
+    JarOps.jar(jar, inputPaths, manifest.build, fileFilter, includeDirs = true, timestamp = None)
 
   def createClasspathPassingJar(jar: os.Path, classpath: Agg[os.Path]): Unit = {
     createJar(
@@ -553,8 +513,8 @@ object Jvm {
     tried match {
       case Failure(e: NoSuchFileException)
           if retryCount > 0 && e.getMessage.contains("__sha1.computed") =>
-          // this one is not detected by coursier itself, so we try-catch handle it
-          // I assume, this happens when another coursier thread already moved or rename dthe temporary file
+        // this one is not detected by coursier itself, so we try-catch handle it
+        // I assume, this happens when another coursier thread already moved or rename dthe temporary file
         ctx.foreach(_.log.debug(
           s"Detected a concurrent download issue in coursier. Attempting a retry (${retryCount} left)"
         ))

main/test/src/eval/JavaCompileJarTests.scala

@@ -127,6 +127,8 @@ object JavaCompileJarTests extends TestSuite {
       ).out.text()
       val expectedJarContents =
         """META-INF/MANIFEST.MF
+          |META-INF/
+          |test/
           |test/Bar.class
           |test/BarThree.class
           |test/BarTwo.class

scalalib/test/src/HelloWorldTests.scala

@@ -670,17 +670,18 @@ object HelloWorldTests extends TestSuite {
         )
 
         Using.resource(new JarFile(result.path.toIO)) { jarFile =>
-          val entries = jarFile.entries().asScala.map(_.getName).toSet
+          val entries = jarFile.entries().asScala.map(_.getName).toSeq.sorted
 
-          val otherFiles = Seq[os.RelPath](
-            os.rel / "META-INF" / "MANIFEST.MF",
-            os.rel / "reference.conf"
+          val otherFiles = Seq(
+            "META-INF/",
+            "META-INF/MANIFEST.MF",
+            "reference.conf"
           )
-          val expectedFiles = compileClassfiles ++ otherFiles
+          val expectedFiles = (compileClassfiles.map(_.toString()) ++ otherFiles).sorted
 
           assert(
             entries.nonEmpty,
-            entries == expectedFiles.map(_.toString()).toSet
+            entries == expectedFiles
           )
 
           val mainClass = jarMainClass(jarFile)