From e98c4da0a74c211eea84ce01508dc86e22668284 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Wed, 5 Jul 2023 17:09:52 +0200 Subject: [PATCH 1/9] Analyze assembly error - Reproduction of #528 and #2650 Reprodcution of issues * https://github.com/com-lihaoyi/mill/issues/528 * https://github.com/com-lihaoyi/mill/issues/2650 --- integration/feature/assembly/repo/build.sc | 21 ++++++++++ .../feature/assembly/repo/foo/src/Main.scala | 17 ++++++++ .../assembly/test/src/AssemblyTests.scala | 41 +++++++++++++++++++ 3 files changed, 79 insertions(+) create mode 100644 integration/feature/assembly/repo/build.sc create mode 100644 integration/feature/assembly/repo/foo/src/Main.scala create mode 100644 integration/feature/assembly/test/src/AssemblyTests.scala diff --git a/integration/feature/assembly/repo/build.sc b/integration/feature/assembly/repo/build.sc new file mode 100644 index 00000000000..038e9d63410 --- /dev/null +++ b/integration/feature/assembly/repo/build.sc @@ -0,0 +1,21 @@ +import mill._, scalalib._ + +trait Setup extends ScalaModule { + def scalaVersion = "2.13.11" + def ivyDeps = Agg( + ivy"com.lihaoyi::scalatags:0.8.2", + ivy"com.lihaoyi::mainargs:0.4.0", + ivy"org.apache.avro:avro:1.11.1", + ivy"dev.zio::zio:2.0.15", + ivy"org.typelevel::cats-core:2.9.0", + ivy"org.apache.spark::spark-core:3.4.0", + ivy"dev.zio::zio-metrics-connectors:2.0.8", + ivy"dev.zio::zio-http:3.0.0-RC2" + ) +} + +object foo extends Setup { + override def prependShellScript: T[String] = "" +} + +object bar extends Setup diff --git a/integration/feature/assembly/repo/foo/src/Main.scala b/integration/feature/assembly/repo/foo/src/Main.scala new file mode 100644 index 00000000000..5ba36647ba4 --- /dev/null +++ b/integration/feature/assembly/repo/foo/src/Main.scala @@ -0,0 +1,17 @@ +package ultra + +import scalatags.Text.all._ +import mainargs.{main, ParserForMethods} + +object Main { + def generateHtml(text: String) = { + h1(text).toString + } + + @main + def main(text: String) = { + println(generateHtml(text)) + } + + def main(args: Array[String]): Unit = ParserForMethods(this).runOrExit(args) +} diff --git a/integration/feature/assembly/test/src/AssemblyTests.scala b/integration/feature/assembly/test/src/AssemblyTests.scala new file mode 100644 index 00000000000..b0fb3b753d3 --- /dev/null +++ b/integration/feature/assembly/test/src/AssemblyTests.scala @@ -0,0 +1,41 @@ +package mill.integration.local + +import mill.integration.IntegrationTestSuite +import mill.util.Jvm +import utest._ + +// Ensure the assembly is runnable, even if we have assembled lots of dependencies into it +// Reproduction of issues: +// - https://github.com/com-lihaoyi/mill/issues/528 +// - https://github.com/com-lihaoyi/mill/issues/2650 + +object AssemblyTests extends IntegrationTestSuite { + def tests: Tests = Tests { + test("Assembly") { + test("without-prependShellScript") { + val workspacePath = initWorkspace() + assert(eval("foo.assembly")) + val assemblyFile = workspacePath / "out" / "foo" / "assembly.dest" / "out.jar" + assert(os.exists(assemblyFile)) + println(s"File size: ${os.stat(assemblyFile).size}") + Jvm.runSubprocess( + commandArgs = Seq(Jvm.javaExe, "-jar", assemblyFile.toString(), "--text", "tutu"), + envArgs = Map.empty[String, String], + workingDir = workspacePath + ) + } + test("with-prependShellScript") { + val workspacePath = initWorkspace() + assert(eval("bar.assembly")) + val assemblyFile = workspacePath / "out" / "bar" / "assembly.dest" / "out.jar" + assert(os.exists(assemblyFile)) + println(s"File size: ${os.stat(assemblyFile).size}") + Jvm.runSubprocess( + commandArgs = Seq(Jvm.javaExe, "-jar", assemblyFile.toString(), "--text", "tutu"), + envArgs = Map.empty[String, String], + workingDir = workspacePath + ) + } + } + } +} From ef73889406d1005ab77c50818dc0ffd813959e00 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Mon, 10 Jul 2023 11:48:16 +0200 Subject: [PATCH 2/9] Improved test --- integration/feature/assembly/repo/build.sc | 23 +++- .../assembly/repo/{foo => }/src/Main.scala | 0 .../assembly/test/src/AssemblyTests.scala | 107 ++++++++++++++---- scalalib/src/mill/scalalib/Assembly.scala | 43 +++++-- 4 files changed, 137 insertions(+), 36 deletions(-) rename integration/feature/assembly/repo/{foo => }/src/Main.scala (100%) diff --git a/integration/feature/assembly/repo/build.sc b/integration/feature/assembly/repo/build.sc index 038e9d63410..43d09cd4150 100644 --- a/integration/feature/assembly/repo/build.sc +++ b/integration/feature/assembly/repo/build.sc @@ -2,10 +2,15 @@ import mill._, scalalib._ trait Setup extends ScalaModule { def scalaVersion = "2.13.11" - def ivyDeps = Agg( + def sources = T.sources(T.workspace / "src") + def ivyDeps = super.ivyDeps() ++ Agg( ivy"com.lihaoyi::scalatags:0.8.2", ivy"com.lihaoyi::mainargs:0.4.0", - ivy"org.apache.avro:avro:1.11.1", + ivy"org.apache.avro:avro:1.11.1" + ) +} +trait ExtraDeps extends ScalaModule { + def ivyDeps = super.ivyDeps() ++ Agg( ivy"dev.zio::zio:2.0.15", ivy"org.typelevel::cats-core:2.9.0", ivy"org.apache.spark::spark-core:3.4.0", @@ -14,8 +19,16 @@ trait Setup extends ScalaModule { ) } -object foo extends Setup { - override def prependShellScript: T[String] = "" +object noExe extends Module { + object small extends Setup { + override def prependShellScript: T[String] = "" + } + object large extends Setup with ExtraDeps { + override def prependShellScript: T[String] = "" + } } -object bar extends Setup +object exe extends Module { + object small extends Setup + object large extends Setup with ExtraDeps +} diff --git a/integration/feature/assembly/repo/foo/src/Main.scala b/integration/feature/assembly/repo/src/Main.scala similarity index 100% rename from integration/feature/assembly/repo/foo/src/Main.scala rename to integration/feature/assembly/repo/src/Main.scala diff --git a/integration/feature/assembly/test/src/AssemblyTests.scala b/integration/feature/assembly/test/src/AssemblyTests.scala index b0fb3b753d3..435636e83f6 100644 --- a/integration/feature/assembly/test/src/AssemblyTests.scala +++ b/integration/feature/assembly/test/src/AssemblyTests.scala @@ -1,7 +1,8 @@ package mill.integration.local +import mill.api.IO import mill.integration.IntegrationTestSuite -import mill.util.Jvm +import mill.util.{Jvm, Util} import utest._ // Ensure the assembly is runnable, even if we have assembled lots of dependencies into it @@ -10,32 +11,100 @@ import utest._ // - https://github.com/com-lihaoyi/mill/issues/2650 object AssemblyTests extends IntegrationTestSuite { + + /** + * Unpacks the given `src` path into the context specific destination directory. + * + * @param src The ZIP file + * @param dest The relative output folder under the context specifix destination directory. + * @param ctx The target context + * @return The [[PathRef]] to the unpacked folder. + */ + def unpackZip(src: os.Path, dest: os.Path): os.Path = { + + val byteStream = os.read.inputStream(src) + val zipStream = new java.util.zip.ZipInputStream(byteStream) + while ({ + zipStream.getNextEntry match { + case null => false + case entry => + if (!entry.isDirectory) { + val entryDest = dest / os.RelPath(entry.getName) + os.makeDir.all(entryDest / os.up) + val fileOut = new java.io.FileOutputStream(entryDest.toString) + IO.stream(zipStream, fileOut) + fileOut.close() + } + zipStream.closeEntry() + true + } + }) () + dest + } + def tests: Tests = Tests { - test("Assembly") { - test("without-prependShellScript") { - val workspacePath = initWorkspace() - assert(eval("foo.assembly")) - val assemblyFile = workspacePath / "out" / "foo" / "assembly.dest" / "out.jar" - assert(os.exists(assemblyFile)) - println(s"File size: ${os.stat(assemblyFile).size}") - Jvm.runSubprocess( - commandArgs = Seq(Jvm.javaExe, "-jar", assemblyFile.toString(), "--text", "tutu"), - envArgs = Map.empty[String, String], - workingDir = workspacePath + + def checkAssembly( + targetSegments: Seq[String], + checkExe: Boolean = false, + checkTmp: Boolean = false + ): Unit = { + val workspacePath = initWorkspace() + val targetName = "assembly" + assert(eval(targetSegments.mkString(".") + "." + targetName)) + + if (checkTmp) { + val tmpFile = workspacePath / "out" / targetSegments / targetName / "out-tmp.jar" + val unpackDir = os.temp.dir( + dir = workspacePath / "out" / targetSegments / s"${targetName}.dest", + prefix = "out-tmp.jar", + deleteOnExit = false ) + assert(os.exists(tmpFile)) + println(s"Tmp File size: ${os.stat(tmpFile).size}") + val unpacked = unpackZip(tmpFile, unpackDir) + assert(os.exists(unpacked / "ultra" / "Main.class")) } - test("with-prependShellScript") { - val workspacePath = initWorkspace() - assert(eval("bar.assembly")) - val assemblyFile = workspacePath / "out" / "bar" / "assembly.dest" / "out.jar" - assert(os.exists(assemblyFile)) - println(s"File size: ${os.stat(assemblyFile).size}") + + val assemblyFile = workspacePath / "out" / targetSegments / s"${targetName}.dest" / "out.jar" + assert(os.exists(assemblyFile)) + println(s"File size: ${os.stat(assemblyFile).size}") + +// val unpackDir = os.temp.dir(dir = workspacePath / "out" / targetSegments.dropRight(1) / targetSegments.takeRight(1).map(_ + ".dest"), prefix = "out.jar",deleteOnExit = false) +// val unpacked = unpackZip(assemblyFile, unpackDir) +// assert(os.exists(unpacked / "ultra" / "Main.class")) + Jvm.runSubprocess( + commandArgs = Seq(Jvm.javaExe, "-jar", assemblyFile.toString(), "--text", "tutu"), + envArgs = Map.empty[String, String], + workingDir = workspacePath + ) + + if (checkExe) { Jvm.runSubprocess( - commandArgs = Seq(Jvm.javaExe, "-jar", assemblyFile.toString(), "--text", "tutu"), + commandArgs = Seq(assemblyFile.toString(), "--text", "tutu"), envArgs = Map.empty[String, String], workingDir = workspacePath ) } } + + test("Assembly") { + test("noExe") { + test("small") { + checkAssembly(Seq("noExe", "small")) + } + test("large") { + checkAssembly(Seq("noExe", "large")) + } + } + test("exe") { + test("small") { + checkAssembly(Seq("noExe", "small"), checkExe = true) + } + test("large") { + checkAssembly(Seq("noExe", "large"), checkExe = true) + } + } + } } } diff --git a/scalalib/src/mill/scalalib/Assembly.scala b/scalalib/src/mill/scalalib/Assembly.scala index 19f5d8a0d77..9293c37edd5 100644 --- a/scalalib/src/mill/scalalib/Assembly.scala +++ b/scalalib/src/mill/scalalib/Assembly.scala @@ -5,7 +5,15 @@ import mill.Agg import mill.api.{Ctx, IO, PathRef} import os.Generator -import java.io.{ByteArrayInputStream, InputStream, SequenceInputStream} +import java.io.{ + BufferedInputStream, + BufferedOutputStream, + ByteArrayInputStream, + FileInputStream, + FileOutputStream, + InputStream, + SequenceInputStream +} import java.net.URI import java.nio.file.attribute.PosixFilePermission import java.nio.file.{FileSystems, Files, StandardOpenOption} @@ -195,13 +203,16 @@ object Assembly { base: Option[os.Path] = None, assemblyRules: Seq[Assembly.Rule] = Assembly.defaultRules )(implicit ctx: Ctx.Dest with Ctx.Log): PathRef = { - val tmp = ctx.dest / "out-tmp.jar" + val rawJar = ctx.dest / "out-tmp.jar" - val baseUri = "jar:" + tmp.toIO.getCanonicalFile.toURI.toASCIIString + // use the `base` (the upsteam assembly) as a start + val baseUri = "jar:" + rawJar.toIO.getCanonicalFile.toURI.toASCIIString val hm = base.fold(Map("create" -> "true")) { b => - os.copy(b, tmp) + os.copy(b, rawJar) Map.empty } + + // Add more files by copying files to a JAR file system Using.resource(FileSystems.newFileSystem(URI.create(baseUri), hm.asJava)) { zipFs => val manifestPath = zipFs.getPath(JarFile.MANIFEST_NAME) Files.createDirectories(manifestPath.getParent) @@ -236,18 +247,26 @@ object Assembly { } } - val output = ctx.dest / "out.jar" + + val finalJar = ctx.dest / "out.jar" // Prepend shell script and make it executable - if (prependShellScript.isEmpty) os.move(tmp, output) - else { + if (prependShellScript.isEmpty) { + os.move(rawJar, finalJar) + } else { val lineSep = if (!prependShellScript.endsWith("\n")) "\n\r\n" else "" - os.write(output, prependShellScript + lineSep) - os.write.append(output, os.read.inputStream(tmp)) + val prepend = prependShellScript + lineSep + // Write the prepend-part into the final jar file + // https://en.wikipedia.org/wiki/Zip_(file_format)#Combination_with_other_file_formats + os.write(finalJar, prepend) + // Append the actual JAR content + Using.resource(os.read.inputStream(rawJar)) { is => + os.write.append(finalJar, is) + } if (!scala.util.Properties.isWin) { os.perms.set( - output, - os.perms(output) + finalJar, + os.perms(finalJar) + PosixFilePermission.GROUP_EXECUTE + PosixFilePermission.OWNER_EXECUTE + PosixFilePermission.OTHERS_EXECUTE @@ -255,7 +274,7 @@ object Assembly { } } - PathRef(output) + PathRef(finalJar) } private def writeEntry(p: java.nio.file.Path, inputStream: InputStream, append: Boolean): Unit = { From 6693a8a813e31b2ec1e8f6f0accaa47f1e11c8b2 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Mon, 10 Jul 2023 13:01:51 +0200 Subject: [PATCH 3/9] Fixed typo --- integration/feature/assembly/test/src/AssemblyTests.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration/feature/assembly/test/src/AssemblyTests.scala b/integration/feature/assembly/test/src/AssemblyTests.scala index 435636e83f6..d5cac5016ea 100644 --- a/integration/feature/assembly/test/src/AssemblyTests.scala +++ b/integration/feature/assembly/test/src/AssemblyTests.scala @@ -99,10 +99,10 @@ object AssemblyTests extends IntegrationTestSuite { } test("exe") { test("small") { - checkAssembly(Seq("noExe", "small"), checkExe = true) + checkAssembly(Seq("exe", "small"), checkExe = true) } test("large") { - checkAssembly(Seq("noExe", "large"), checkExe = true) + checkAssembly(Seq("exe", "large"), checkExe = true) } } } From 3f8209ad6dc365aec18d07a6ca7dbe8a76ba8f35 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Fri, 3 May 2024 09:07:59 +0200 Subject: [PATCH 4/9] Detect assemblies with too many entries to fail shell script prepending This PR is based on https://github.com/com-lihaoyi/mill/pull/2655 --- scalalib/src/mill/scalalib/Assembly.scala | 87 ++++++++++++--------- scalalib/src/mill/scalalib/JavaModule.scala | 50 ++++++++++-- 2 files changed, 96 insertions(+), 41 deletions(-) diff --git a/scalalib/src/mill/scalalib/Assembly.scala b/scalalib/src/mill/scalalib/Assembly.scala index 9293c37edd5..4771720e42d 100644 --- a/scalalib/src/mill/scalalib/Assembly.scala +++ b/scalalib/src/mill/scalalib/Assembly.scala @@ -5,15 +5,7 @@ import mill.Agg import mill.api.{Ctx, IO, PathRef} import os.Generator -import java.io.{ - BufferedInputStream, - BufferedOutputStream, - ByteArrayInputStream, - FileInputStream, - FileOutputStream, - InputStream, - SequenceInputStream -} +import java.io.{ByteArrayInputStream, InputStream, SequenceInputStream} import java.net.URI import java.nio.file.attribute.PosixFilePermission import java.nio.file.{FileSystems, Files, StandardOpenOption} @@ -24,8 +16,12 @@ import scala.jdk.CollectionConverters._ import scala.tools.nsc.io.Streamable import scala.util.Using +case class Assembly(pathRef: PathRef, addedEntries: Int) + object Assembly { + implicit val assemblyJsonRW: upickle.default.ReadWriter[Assembly] = upickle.default.macroRW + val defaultRules: Seq[Rule] = Seq( Rule.Append("reference.conf", separator = "\n"), Rule.Exclude(JarFile.MANIFEST_NAME), @@ -203,15 +199,35 @@ object Assembly { base: Option[os.Path] = None, assemblyRules: Seq[Assembly.Rule] = Assembly.defaultRules )(implicit ctx: Ctx.Dest with Ctx.Log): PathRef = { - val rawJar = ctx.dest / "out-tmp.jar" + create( + destJar = ctx.dest / "out.jar", + inputPaths = inputPaths, + manifest = manifest, + prependShellScript = Option(prependShellScript).filter(_ != ""), + base = base, + assemblyRules = assemblyRules + ).pathRef + } + + def create( + destJar: os.Path, + inputPaths: Agg[os.Path], + manifest: mill.api.JarManifest = mill.api.JarManifest.MillDefault, + prependShellScript: Option[String] = None, + base: Option[os.Path] = None, + assemblyRules: Seq[Assembly.Rule] = Assembly.defaultRules + ): Assembly = { + val rawJar = os.temp("out-tmp") - // use the `base` (the upsteam assembly) as a start + // use the `base` (the upstream assembly) as a start val baseUri = "jar:" + rawJar.toIO.getCanonicalFile.toURI.toASCIIString val hm = base.fold(Map("create" -> "true")) { b => os.copy(b, rawJar) Map.empty } + var addedEntryCount = 0 + // Add more files by copying files to a JAR file system Using.resource(FileSystems.newFileSystem(URI.create(baseUri), hm.asJava)) { zipFs => val manifestPath = zipFs.getPath(JarFile.MANIFEST_NAME) @@ -236,10 +252,12 @@ object Assembly { Seq(new ByteArrayInputStream(entry.separator.getBytes), inputStream()) ) val cleaned = if (Files.exists(path)) separated else separated.drop(1) - val concatenated = - new SequenceInputStream(Collections.enumeration(cleaned.asJava)) + val concatenated = new SequenceInputStream(Collections.enumeration(cleaned.asJava)) + addedEntryCount += 1 writeEntry(path, concatenated, append = true) - case entry: WriteOnceEntry => writeEntry(path, entry.inputStream(), append = false) + case entry: WriteOnceEntry => + addedEntryCount += 1 + writeEntry(path, entry.inputStream(), append = false) } } } finally { @@ -247,34 +265,33 @@ object Assembly { } } - - val finalJar = ctx.dest / "out.jar" // Prepend shell script and make it executable - if (prependShellScript.isEmpty) { - os.move(rawJar, finalJar) - } else { - val lineSep = if (!prependShellScript.endsWith("\n")) "\n\r\n" else "" - val prepend = prependShellScript + lineSep - // Write the prepend-part into the final jar file - // https://en.wikipedia.org/wiki/Zip_(file_format)#Combination_with_other_file_formats - os.write(finalJar, prepend) + prependShellScript match { + case None => + os.move(rawJar, destJar) + case Some(prependShellScript) => + val lineSep = if (!prependShellScript.endsWith("\n")) "\n\r\n" else "" + val prepend = prependShellScript + lineSep + // Write the prepend-part into the final jar file + // https://en.wikipedia.org/wiki/Zip_(file_format)#Combination_with_other_file_formats + os.write(destJar, prepend) // Append the actual JAR content Using.resource(os.read.inputStream(rawJar)) { is => - os.write.append(finalJar, is) + os.write.append(destJar, is) } - if (!scala.util.Properties.isWin) { - os.perms.set( - finalJar, - os.perms(finalJar) - + PosixFilePermission.GROUP_EXECUTE - + PosixFilePermission.OWNER_EXECUTE - + PosixFilePermission.OTHERS_EXECUTE - ) - } + if (!scala.util.Properties.isWin) { + os.perms.set( + destJar, + os.perms(destJar) + + PosixFilePermission.GROUP_EXECUTE + + PosixFilePermission.OWNER_EXECUTE + + PosixFilePermission.OTHERS_EXECUTE + ) + } } - PathRef(finalJar) + Assembly(PathRef(destJar), addedEntryCount) } private def writeEntry(p: java.nio.file.Path, inputStream: InputStream, append: Boolean): Unit = { diff --git a/scalalib/src/mill/scalalib/JavaModule.scala b/scalalib/src/mill/scalalib/JavaModule.scala index 3a7bad49b5c..e7b8a999c27 100644 --- a/scalalib/src/mill/scalalib/JavaModule.scala +++ b/scalalib/src/mill/scalalib/JavaModule.scala @@ -569,11 +569,27 @@ trait JavaModule * * This should allow much faster assembly creation in the common case where * upstream dependencies do not change + * + * This implementation is deprecated because of it's return value. + * Please use [[upstreamAssembly2]] instead. */ + @deprecated("Use upstreamAssembly2 instead, which has a richer return value", "Mill 0.11.8") def upstreamAssembly: T[PathRef] = T { - Assembly.createAssembly( - upstreamAssemblyClasspath().map(_.path), - manifest(), + upstreamAssembly2().pathRef + } + + /** + * Build the assembly for upstream dependencies separate from the current + * classpath + * + * This should allow much faster assembly creation in the common case where + * upstream dependencies do not change + */ + def upstreamAssembly2: T[Assembly] = T { + Assembly.create( + destJar = T.dest / "out.jar", + inputPaths = upstreamAssemblyClasspath().map(_.path), + manifest = manifest(), assemblyRules = assemblyRules ) } @@ -583,13 +599,35 @@ trait JavaModule * classfiles from this module and all it's upstream modules and dependencies */ def assembly: T[PathRef] = T { - Assembly.createAssembly( + val prependScript = Option(prependShellScript()).filter(_ != "") + val upstream = upstreamAssembly2() + + val created = Assembly.create( + destJar = T.dest / "out.jar", Agg.from(localClasspath().map(_.path)), manifest(), - prependShellScript(), - Some(upstreamAssembly().path), + prependScript, + Some(upstream.pathRef.path), assemblyRules ) + // See https://github.com/com-lihaoyi/mill/pull/2655#issuecomment-1672468284 + val problematicEntryCount = 65535 + if ( + prependScript.isDefined && + (upstream.addedEntries + created.addedEntries) > problematicEntryCount + ) { + Result.Failure( + s"""The created assembly would contain more than $problematicEntryCount ZIP entries. + |JARs of that size are known to not work correctly with a prepended shell script. + |Either reduce the entries count of the assembly or disable the prepended shell script with: + | + | def prependShellScript = "" + |""".stripMargin, + Some(created.pathRef) + ) + } else { + Result.Success(created.pathRef) + } } /** From 725b3b2d2e33948f7995c28715e03e88a0331946 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Fri, 3 May 2024 10:57:53 +0200 Subject: [PATCH 5/9] Refined test --- .../assembly/test/src/AssemblyTests.scala | 109 +++++++----------- scalalib/src/mill/scalalib/Assembly.scala | 5 +- 2 files changed, 44 insertions(+), 70 deletions(-) diff --git a/integration/feature/assembly/test/src/AssemblyTests.scala b/integration/feature/assembly/test/src/AssemblyTests.scala index d5cac5016ea..47d01f03009 100644 --- a/integration/feature/assembly/test/src/AssemblyTests.scala +++ b/integration/feature/assembly/test/src/AssemblyTests.scala @@ -2,7 +2,7 @@ package mill.integration.local import mill.api.IO import mill.integration.IntegrationTestSuite -import mill.util.{Jvm, Util} +import mill.util.Jvm import utest._ // Ensure the assembly is runnable, even if we have assembled lots of dependencies into it @@ -12,82 +12,43 @@ import utest._ object AssemblyTests extends IntegrationTestSuite { - /** - * Unpacks the given `src` path into the context specific destination directory. - * - * @param src The ZIP file - * @param dest The relative output folder under the context specifix destination directory. - * @param ctx The target context - * @return The [[PathRef]] to the unpacked folder. - */ - def unpackZip(src: os.Path, dest: os.Path): os.Path = { + def checkAssembly( + targetSegments: Seq[String], + checkExe: Boolean = false, + failMsg: Option[String] = None + ): Unit = { + val workspacePath = initWorkspace() + val targetName = "assembly" + val res = evalStdout(targetSegments.mkString(".") + "." + targetName) - val byteStream = os.read.inputStream(src) - val zipStream = new java.util.zip.ZipInputStream(byteStream) - while ({ - zipStream.getNextEntry match { - case null => false - case entry => - if (!entry.isDirectory) { - val entryDest = dest / os.RelPath(entry.getName) - os.makeDir.all(entryDest / os.up) - val fileOut = new java.io.FileOutputStream(entryDest.toString) - IO.stream(zipStream, fileOut) - fileOut.close() - } - zipStream.closeEntry() - true - } - }) () - dest - } - - def tests: Tests = Tests { - - def checkAssembly( - targetSegments: Seq[String], - checkExe: Boolean = false, - checkTmp: Boolean = false - ): Unit = { - val workspacePath = initWorkspace() - val targetName = "assembly" - assert(eval(targetSegments.mkString(".") + "." + targetName)) - - if (checkTmp) { - val tmpFile = workspacePath / "out" / targetSegments / targetName / "out-tmp.jar" - val unpackDir = os.temp.dir( - dir = workspacePath / "out" / targetSegments / s"${targetName}.dest", - prefix = "out-tmp.jar", - deleteOnExit = false - ) - assert(os.exists(tmpFile)) - println(s"Tmp File size: ${os.stat(tmpFile).size}") - val unpacked = unpackZip(tmpFile, unpackDir) - assert(os.exists(unpacked / "ultra" / "Main.class")) - } - - val assemblyFile = workspacePath / "out" / targetSegments / s"${targetName}.dest" / "out.jar" - assert(os.exists(assemblyFile)) - println(s"File size: ${os.stat(assemblyFile).size}") + assert(res.isSuccess == failMsg.isEmpty) -// val unpackDir = os.temp.dir(dir = workspacePath / "out" / targetSegments.dropRight(1) / targetSegments.takeRight(1).map(_ + ".dest"), prefix = "out.jar",deleteOnExit = false) -// val unpacked = unpackZip(assemblyFile, unpackDir) -// assert(os.exists(unpacked / "ultra" / "Main.class")) - Jvm.runSubprocess( - commandArgs = Seq(Jvm.javaExe, "-jar", assemblyFile.toString(), "--text", "tutu"), - envArgs = Map.empty[String, String], - workingDir = workspacePath - ) + failMsg match { + case None => + val assemblyFile = + workspacePath / "out" / targetSegments / s"${targetName}.dest" / "out.jar" + assert(os.exists(assemblyFile)) + println(s"File size: ${os.stat(assemblyFile).size}") - if (checkExe) { Jvm.runSubprocess( - commandArgs = Seq(assemblyFile.toString(), "--text", "tutu"), + commandArgs = Seq(Jvm.javaExe, "-jar", assemblyFile.toString(), "--text", "tutu"), envArgs = Map.empty[String, String], workingDir = workspacePath ) - } + + if (checkExe) { + Jvm.runSubprocess( + commandArgs = Seq(assemblyFile.toString(), "--text", "tutu"), + envArgs = Map.empty[String, String], + workingDir = workspacePath + ) + } + case Some(msg) => + assert((res.out ++ res.err).contains(msg)) } + } + def tests: Tests = Tests { test("Assembly") { test("noExe") { test("small") { @@ -102,7 +63,17 @@ object AssemblyTests extends IntegrationTestSuite { checkAssembly(Seq("exe", "small"), checkExe = true) } test("large") { - checkAssembly(Seq("exe", "large"), checkExe = true) + checkAssembly( + Seq("exe", "large"), + failMsg = Some( + """exe.large.assembly The created assembly would contain more than 65535 ZIP entries. + |JARs of that size are known to not work correctly with a prepended shell script. + |Either reduce the entries count of the assembly or disable the prepended shell script with: + | + | def prependShellScript = "" + |""".stripMargin + ) + ) } } } diff --git a/scalalib/src/mill/scalalib/Assembly.scala b/scalalib/src/mill/scalalib/Assembly.scala index 4771720e42d..8b389863b2d 100644 --- a/scalalib/src/mill/scalalib/Assembly.scala +++ b/scalalib/src/mill/scalalib/Assembly.scala @@ -217,7 +217,9 @@ object Assembly { base: Option[os.Path] = None, assemblyRules: Seq[Assembly.Rule] = Assembly.defaultRules ): Assembly = { - val rawJar = os.temp("out-tmp") + val rawJar = os.temp("out-tmp", deleteOnExit = false) + // we create the file later + os.remove(rawJar) // use the `base` (the upstream assembly) as a start val baseUri = "jar:" + rawJar.toIO.getCanonicalFile.toURI.toASCIIString @@ -279,6 +281,7 @@ object Assembly { Using.resource(os.read.inputStream(rawJar)) { is => os.write.append(destJar, is) } + os.remove(rawJar) if (!scala.util.Properties.isWin) { os.perms.set( From a5c74558f9f7e2b685f2bee06e36b6c388507353 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Sat, 4 May 2024 14:53:15 +0200 Subject: [PATCH 6/9] Converted assembly test into unit test --- integration/feature/assembly/repo/build.sc | 34 ---- .../feature/assembly/repo/src/Main.scala | 17 -- .../assembly/test/src/AssemblyTests.scala | 81 ---------- .../src/mill/scalalib/AssemblyTests.scala | 147 ++++++++++++++++++ 4 files changed, 147 insertions(+), 132 deletions(-) delete mode 100644 integration/feature/assembly/repo/build.sc delete mode 100644 integration/feature/assembly/repo/src/Main.scala delete mode 100644 integration/feature/assembly/test/src/AssemblyTests.scala create mode 100644 scalalib/test/src/mill/scalalib/AssemblyTests.scala diff --git a/integration/feature/assembly/repo/build.sc b/integration/feature/assembly/repo/build.sc deleted file mode 100644 index 43d09cd4150..00000000000 --- a/integration/feature/assembly/repo/build.sc +++ /dev/null @@ -1,34 +0,0 @@ -import mill._, scalalib._ - -trait Setup extends ScalaModule { - def scalaVersion = "2.13.11" - def sources = T.sources(T.workspace / "src") - def ivyDeps = super.ivyDeps() ++ Agg( - ivy"com.lihaoyi::scalatags:0.8.2", - ivy"com.lihaoyi::mainargs:0.4.0", - ivy"org.apache.avro:avro:1.11.1" - ) -} -trait ExtraDeps extends ScalaModule { - def ivyDeps = super.ivyDeps() ++ Agg( - ivy"dev.zio::zio:2.0.15", - ivy"org.typelevel::cats-core:2.9.0", - ivy"org.apache.spark::spark-core:3.4.0", - ivy"dev.zio::zio-metrics-connectors:2.0.8", - ivy"dev.zio::zio-http:3.0.0-RC2" - ) -} - -object noExe extends Module { - object small extends Setup { - override def prependShellScript: T[String] = "" - } - object large extends Setup with ExtraDeps { - override def prependShellScript: T[String] = "" - } -} - -object exe extends Module { - object small extends Setup - object large extends Setup with ExtraDeps -} diff --git a/integration/feature/assembly/repo/src/Main.scala b/integration/feature/assembly/repo/src/Main.scala deleted file mode 100644 index 5ba36647ba4..00000000000 --- a/integration/feature/assembly/repo/src/Main.scala +++ /dev/null @@ -1,17 +0,0 @@ -package ultra - -import scalatags.Text.all._ -import mainargs.{main, ParserForMethods} - -object Main { - def generateHtml(text: String) = { - h1(text).toString - } - - @main - def main(text: String) = { - println(generateHtml(text)) - } - - def main(args: Array[String]): Unit = ParserForMethods(this).runOrExit(args) -} diff --git a/integration/feature/assembly/test/src/AssemblyTests.scala b/integration/feature/assembly/test/src/AssemblyTests.scala deleted file mode 100644 index 47d01f03009..00000000000 --- a/integration/feature/assembly/test/src/AssemblyTests.scala +++ /dev/null @@ -1,81 +0,0 @@ -package mill.integration.local - -import mill.api.IO -import mill.integration.IntegrationTestSuite -import mill.util.Jvm -import utest._ - -// Ensure the assembly is runnable, even if we have assembled lots of dependencies into it -// Reproduction of issues: -// - https://github.com/com-lihaoyi/mill/issues/528 -// - https://github.com/com-lihaoyi/mill/issues/2650 - -object AssemblyTests extends IntegrationTestSuite { - - def checkAssembly( - targetSegments: Seq[String], - checkExe: Boolean = false, - failMsg: Option[String] = None - ): Unit = { - val workspacePath = initWorkspace() - val targetName = "assembly" - val res = evalStdout(targetSegments.mkString(".") + "." + targetName) - - assert(res.isSuccess == failMsg.isEmpty) - - failMsg match { - case None => - val assemblyFile = - workspacePath / "out" / targetSegments / s"${targetName}.dest" / "out.jar" - assert(os.exists(assemblyFile)) - println(s"File size: ${os.stat(assemblyFile).size}") - - Jvm.runSubprocess( - commandArgs = Seq(Jvm.javaExe, "-jar", assemblyFile.toString(), "--text", "tutu"), - envArgs = Map.empty[String, String], - workingDir = workspacePath - ) - - if (checkExe) { - Jvm.runSubprocess( - commandArgs = Seq(assemblyFile.toString(), "--text", "tutu"), - envArgs = Map.empty[String, String], - workingDir = workspacePath - ) - } - case Some(msg) => - assert((res.out ++ res.err).contains(msg)) - } - } - - def tests: Tests = Tests { - test("Assembly") { - test("noExe") { - test("small") { - checkAssembly(Seq("noExe", "small")) - } - test("large") { - checkAssembly(Seq("noExe", "large")) - } - } - test("exe") { - test("small") { - checkAssembly(Seq("exe", "small"), checkExe = true) - } - test("large") { - checkAssembly( - Seq("exe", "large"), - failMsg = Some( - """exe.large.assembly The created assembly would contain more than 65535 ZIP entries. - |JARs of that size are known to not work correctly with a prepended shell script. - |Either reduce the entries count of the assembly or disable the prepended shell script with: - | - | def prependShellScript = "" - |""".stripMargin - ) - ) - } - } - } - } -} diff --git a/scalalib/test/src/mill/scalalib/AssemblyTests.scala b/scalalib/test/src/mill/scalalib/AssemblyTests.scala new file mode 100644 index 00000000000..4026e3b728f --- /dev/null +++ b/scalalib/test/src/mill/scalalib/AssemblyTests.scala @@ -0,0 +1,147 @@ +package mill.scalalib + +import mill._ +import mill.api.Result +import mill.eval.Evaluator +import mill.util.{Jvm, TestEvaluator, TestUtil} +import utest._ +import utest.framework.TestPath + +import java.io.PrintStream + +// Ensure the assembly is runnable, even if we have assembled lots of dependencies into it +// Reproduction of issues: +// - https://github.com/com-lihaoyi/mill/issues/528 +// - https://github.com/com-lihaoyi/mill/issues/2650 + +object AssemblyTests extends TestSuite { + + object TestCase extends TestUtil.BaseModule { + trait Setup extends ScalaModule { + def scalaVersion = "2.13.11" + def sources = T.sources(T.workspace / "src") + def ivyDeps = super.ivyDeps() ++ Agg( + ivy"com.lihaoyi::scalatags:0.8.2", + ivy"com.lihaoyi::mainargs:0.4.0", + ivy"org.apache.avro:avro:1.11.1" + ) + } + trait ExtraDeps extends ScalaModule { + def ivyDeps = super.ivyDeps() ++ Agg( + ivy"dev.zio::zio:2.0.15", + ivy"org.typelevel::cats-core:2.9.0", + ivy"org.apache.spark::spark-core:3.4.0", + ivy"dev.zio::zio-metrics-connectors:2.0.8", + ivy"dev.zio::zio-http:3.0.0-RC2" + ) + } + + object noExe extends Module { + object small extends Setup { + override def prependShellScript: T[String] = "" + } + object large extends Setup with ExtraDeps { + override def prependShellScript: T[String] = "" + } + } + + object exe extends Module { + object small extends Setup + object large extends Setup with ExtraDeps + } + + } + + val sources = Map( + os.rel / "src" / "Main.scala" -> + """package ultra + | + |import scalatags.Text.all._ + |import mainargs.{main, ParserForMethods} + | + |object Main { + | def generateHtml(text: String) = { + | h1(text).toString + | } + | + | @main + | def main(text: String) = { + | println(generateHtml(text)) + | } + | + | def main(args: Array[String]): Unit = ParserForMethods(this).runOrExit(args) + |}""".stripMargin + ) + + def workspaceTest[T]( + m: TestUtil.BaseModule, + env: Map[String, String] = Evaluator.defaultEnv, + debug: Boolean = false, + errStream: PrintStream = System.err + )(t: TestEvaluator => T)(implicit tp: TestPath): T = { + val eval = new TestEvaluator(m, env = env, debugEnabled = debug, errStream = errStream) + os.remove.all(m.millSourcePath) + sources.foreach { case (file, content) => + os.write(m.millSourcePath / file, content, createFolders = true) + } + os.remove.all(eval.outPath) + os.makeDir.all(m.millSourcePath / os.up) + t(eval) + } + + def runAssembly(file: os.Path, wd: os.Path, checkExe: Boolean = false): Unit = { + println(s"File size: ${os.stat(file).size}") + Jvm.runSubprocess( + commandArgs = Seq(Jvm.javaExe, "-jar", file.toString(), "--text", "tutu"), + envArgs = Map.empty[String, String], + workingDir = wd + ) + if(checkExe) { + Jvm.runSubprocess( + commandArgs = Seq(file.toString(), "--text", "tutu"), + envArgs = Map.empty[String, String], + workingDir = wd + ) + } + } + + def tests: Tests = Tests { + test("Assembly") { + test("noExe") { + test("small") { + workspaceTest(TestCase) { eval => + val Right((res, _)) = eval(TestCase.noExe.small.assembly) + runAssembly(res.path, TestCase.millSourcePath) + } + } + test("large") { + workspaceTest(TestCase) { eval => + val Right((res, _)) = eval(TestCase.noExe.large.assembly) + runAssembly(res.path, TestCase.millSourcePath) + } + } + } + test("exe") { + test("small") { + workspaceTest(TestCase) { eval => + val Right((res, _)) = eval(TestCase.exe.small.assembly) + runAssembly(res.path, TestCase.millSourcePath, checkExe = true) + } + } + test("large-should-fail") { + workspaceTest(TestCase) { eval => + val Left(Result.Failure(msg, Some(res))) = eval(TestCase.exe.large.assembly) + val expectedMsg = + """The created assembly would contain more than 65535 ZIP entries. + |JARs of that size are known to not work correctly with a prepended shell script. + |Either reduce the entries count of the assembly or disable the prepended shell script with: + | + | def prependShellScript = "" + |""".stripMargin + assert(msg == expectedMsg) + } + } + } + } + } +} From 2b78fab9b435b6ddecbd1a12d1e15a4d8bd591b7 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Sat, 4 May 2024 16:40:32 +0200 Subject: [PATCH 7/9] Refined error message --- scalalib/src/mill/scalalib/JavaModule.scala | 2 +- scalalib/test/src/mill/scalalib/AssemblyTests.scala | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scalalib/src/mill/scalalib/JavaModule.scala b/scalalib/src/mill/scalalib/JavaModule.scala index e7b8a999c27..546f3655370 100644 --- a/scalalib/src/mill/scalalib/JavaModule.scala +++ b/scalalib/src/mill/scalalib/JavaModule.scala @@ -617,7 +617,7 @@ trait JavaModule (upstream.addedEntries + created.addedEntries) > problematicEntryCount ) { Result.Failure( - s"""The created assembly would contain more than $problematicEntryCount ZIP entries. + s"""The created assembly jar contains more than ${problematicEntryCount} ZIP entries. |JARs of that size are known to not work correctly with a prepended shell script. |Either reduce the entries count of the assembly or disable the prepended shell script with: | diff --git a/scalalib/test/src/mill/scalalib/AssemblyTests.scala b/scalalib/test/src/mill/scalalib/AssemblyTests.scala index 4026e3b728f..975d916b607 100644 --- a/scalalib/test/src/mill/scalalib/AssemblyTests.scala +++ b/scalalib/test/src/mill/scalalib/AssemblyTests.scala @@ -96,7 +96,7 @@ object AssemblyTests extends TestSuite { envArgs = Map.empty[String, String], workingDir = wd ) - if(checkExe) { + if (checkExe) { Jvm.runSubprocess( commandArgs = Seq(file.toString(), "--text", "tutu"), envArgs = Map.empty[String, String], @@ -132,7 +132,7 @@ object AssemblyTests extends TestSuite { workspaceTest(TestCase) { eval => val Left(Result.Failure(msg, Some(res))) = eval(TestCase.exe.large.assembly) val expectedMsg = - """The created assembly would contain more than 65535 ZIP entries. + """The created assembly jar contains more than 65535 ZIP entries. |JARs of that size are known to not work correctly with a prepended shell script. |Either reduce the entries count of the assembly or disable the prepended shell script with: | From 90563d7f736cb928a41b776f0055e5e952fc7314 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Mon, 6 May 2024 08:32:25 +0200 Subject: [PATCH 8/9] Fix changed dependency path in example test --- example/basic/4-builtin-commands/build.sc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/example/basic/4-builtin-commands/build.sc b/example/basic/4-builtin-commands/build.sc index 5d9b005830b..1413089adcb 100644 --- a/example/basic/4-builtin-commands/build.sc +++ b/example/basic/4-builtin-commands/build.sc @@ -202,7 +202,8 @@ foo.sources foo.allSources foo.allSourceFiles foo.compile -foo.localClasspath +foo.finalMainClassOpt +foo.prependShellScript foo.assembly */ From 75bff80feddba645dbbb571311cf91506c7866c5 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Mon, 6 May 2024 09:38:43 +0200 Subject: [PATCH 9/9] Added some log output when upstreamAssembly is used --- scalalib/src/mill/scalalib/JavaModule.scala | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/scalalib/src/mill/scalalib/JavaModule.scala b/scalalib/src/mill/scalalib/JavaModule.scala index 546f3655370..2bf618462d9 100644 --- a/scalalib/src/mill/scalalib/JavaModule.scala +++ b/scalalib/src/mill/scalalib/JavaModule.scala @@ -18,6 +18,8 @@ import mill.scalalib.publish.Artifact import mill.util.Jvm import os.{Path, ProcessOutput} +import scala.annotation.nowarn + /** * Core configuration required to compile a single Java compilation target */ @@ -575,6 +577,10 @@ trait JavaModule */ @deprecated("Use upstreamAssembly2 instead, which has a richer return value", "Mill 0.11.8") def upstreamAssembly: T[PathRef] = T { + T.log.error( + s"upstreamAssembly target is deprecated and should no longer used." + + s" Please make sure to use upstreamAssembly2 instead." + ) upstreamAssembly2().pathRef } @@ -599,6 +605,16 @@ trait JavaModule * classfiles from this module and all it's upstream modules and dependencies */ def assembly: T[PathRef] = T { + // detect potential inconsistencies due to `upstreamAssembly` deprecation after 0.11.7 + if ( + (upstreamAssembly.ctx.enclosing: @nowarn) != s"${classOf[JavaModule].getName}#upstreamAssembly" + ) { + T.log.error( + s"${upstreamAssembly.ctx.enclosing: @nowarn} is overriding a deprecated target which is no longer used." + + s" Please make sure to override upstreamAssembly2 instead." + ) + } + val prependScript = Option(prependShellScript()).filter(_ != "") val upstream = upstreamAssembly2()