From 4e8e912ca60bfd379ea5154e82561ecb31eb94c6 Mon Sep 17 00:00:00 2001 From: Armin Motekalem Date: Sat, 6 Sep 2025 09:06:44 -0700 Subject: [PATCH 1/4] scalapb-support-java-generation --- .../contrib/scalapblib/ScalaPBModule.scala | 5 +- .../contrib/scalapblib/ScalaPBWorker.scala | 33 ++++++-- .../contrib/scalapblib/TutorialTests.scala | 84 +++++++++++++------ 3 files changed, 86 insertions(+), 36 deletions(-) diff --git a/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBModule.scala b/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBModule.scala index d7979097e83e..bec7577645c1 100644 --- a/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBModule.scala +++ b/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBModule.scala @@ -22,6 +22,8 @@ trait ScalaPBModule extends ScalaModule { def scalaPBVersion: T[String] + def scalaPBGenerators: T[Seq[Generator]] = Seq[Generator](ScalaGen) + def scalaPBFlatPackage: T[Boolean] = Task { false } def scalaPBJavaConversions: T[Boolean] = Task { false } @@ -141,7 +143,8 @@ trait ScalaPBModule extends ScalaModule { scalaPBSources().map(_.path), scalaPBOptions(), Task.dest, - scalaPBCompileOptions() + scalaPBCompileOptions(), + scalaPBGenerators() ) } } diff --git a/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorker.scala b/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorker.scala index d067f0b35bb9..72e4ab37ef5e 100644 --- a/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorker.scala +++ b/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorker.scala @@ -5,6 +5,7 @@ import java.io.File import mill.api.PathRef import mill.api.{Discover, ExternalModule} +import upickle.default.ReadWriter class ScalaPBWorker { @@ -15,16 +16,17 @@ class ScalaPBWorker { sources: Seq[File], scalaPBOptions: String, generatedDirectory: File, - otherArgs: Seq[String] + otherArgs: Seq[String], + generators: Seq[Generator] ): Unit = { val pbcClasspath = scalaPBClasspath.map(_.path).toVector mill.util.Jvm.withClassLoader(pbcClasspath, null) { cl => val scalaPBCompilerClass = cl.loadClass("scalapb.ScalaPBC") val mainMethod = scalaPBCompilerClass.getMethod("main", classOf[Array[java.lang.String]]) - val opts = if (scalaPBOptions.isEmpty) "" else scalaPBOptions + ":" - val args = otherArgs ++ Seq( - s"--scala_out=${opts}${generatedDirectory.getCanonicalPath}" - ) ++ roots.map(root => s"--proto_path=${root.getCanonicalPath}") ++ sources.map( + val args = otherArgs ++ generators.map { gen => + val opts = if (scalaPBOptions.isEmpty || !gen.supportsScalaPBOptions) "" else scalaPBOptions + ":" + s"${gen.generator}=$opts${generatedDirectory.getCanonicalPath}" + } ++ roots.map(root => s"--proto_path=${root.getCanonicalPath}") ++ sources.map( _.getCanonicalPath ) ctx.log.debug(s"ScalaPBC args: ${args.mkString(" ")}") @@ -70,7 +72,8 @@ class ScalaPBWorker { scalaPBSources: Seq[os.Path], scalaPBOptions: String, dest: os.Path, - scalaPBCExtraArgs: Seq[String] + scalaPBCExtraArgs: Seq[String], + generators: Seq[Generator] )(implicit ctx: mill.api.TaskCtx): mill.api.Result[PathRef] = { val compiler = scalaPB(scalaPBClasspath) val sources = scalaPBSources.flatMap { @@ -86,7 +89,7 @@ class ScalaPBWorker { Seq(ioFile) } val roots = scalaPBSources.map(_.toIO).filter(_.isDirectory) - compiler.compileScalaPB(roots, sources, scalaPBOptions, dest.toIO, scalaPBCExtraArgs) + compiler.compileScalaPB(roots, sources, scalaPBOptions, dest.toIO, scalaPBCExtraArgs, generators) mill.api.Result.Success(PathRef(dest)) } } @@ -97,10 +100,24 @@ trait ScalaPBWorkerApi { source: Seq[File], scalaPBOptions: String, generatedDirectory: File, - otherArgs: Seq[String] + otherArgs: Seq[String], + generators: Seq[Generator] ): Unit } +sealed trait Generator derives ReadWriter { + def generator: String + def supportsScalaPBOptions: Boolean +} +case object ScalaGen extends Generator { + override def generator: String = "--scala_out" + override def supportsScalaPBOptions: Boolean = true +} +case object JavaGen extends Generator { + override def generator: String = "--java_out" + override def supportsScalaPBOptions: Boolean = false // Java options are specified directly in the proto file +} + object ScalaPBWorkerApi extends ExternalModule { def scalaPBWorker: Worker[ScalaPBWorker] = Task.Worker { new ScalaPBWorker() } lazy val millDiscover = Discover[this.type] diff --git a/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala b/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala index 8a32a6df10e9..e9e3b6d26d44 100644 --- a/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala +++ b/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala @@ -11,7 +11,9 @@ import utest.{TestSuite, Tests, assert, *} object TutorialTests extends TestSuite { val testScalaPbVersion = "0.11.7" - trait TutorialBase extends TestRootModule + trait TutorialBase extends TestRootModule { + val core: TutorialModule + } trait TutorialModule extends ScalaPBModule { def scalaVersion = sys.props.getOrElse("TEST_SCALA_2_12_VERSION", ???) @@ -70,12 +72,28 @@ object TutorialTests extends TestSuite { lazy val millDiscover = Discover[this.type] } + object TutorialWithJavaGen extends TutorialBase { + object core extends TutorialModule { + override def scalaPBGenerators = Seq(JavaGen) + } + + lazy val millDiscover = Discover[this.type] + } + + object TutorialWithScalaAndJavaGen extends TutorialBase { + object core extends TutorialModule { + override def scalaPBGenerators = Seq[Generator](ScalaGen, JavaGen) + } + + lazy val millDiscover = Discover[this.type] + } + val resourcePath: os.Path = os.Path(sys.env("MILL_TEST_RESOURCE_DIR")) def protobufOutPath(eval: UnitTester): os.Path = eval.outPath / "core/compileScalaPB.dest/com/example/tutorial" - def compiledSourcefiles: Seq[os.RelPath] = Seq[os.RelPath]( + def compiledScalaSourcefiles: Seq[os.RelPath] = Seq[os.RelPath]( os.rel / "AddressBook.scala", os.rel / "Person.scala", os.rel / "TutorialProto.scala", @@ -83,6 +101,39 @@ object TutorialTests extends TestSuite { os.rel / "IncludeProto.scala" ) + def compiledJavaSourcefiles: Seq[os.RelPath] = Seq[os.RelPath]( + os.rel / "AddressBookProtos.java", + os.rel / "IncludeOuterClass.java" + ) + + // Helper function to test compilation with different generators + def testCompilation( + module: TutorialBase, + expectedFiles: Seq[os.RelPath] + ): Unit = { + UnitTester(module, resourcePath).scoped { eval => + if (!mill.constants.Util.isWindows) { + val Right(result) = eval.apply(module.core.compileScalaPB): @unchecked + + val outPath = protobufOutPath(eval) + val outputFiles = os.walk(result.value.path).filter(os.isFile) + val expectedSourcefiles = expectedFiles.map(outPath / _) + + assert( + result.value.path == eval.outPath / "core/compileScalaPB.dest", + outputFiles.nonEmpty, + outputFiles.forall(expectedSourcefiles.contains), + outputFiles.size == outputFiles.size, + result.evalCount > 0 + ) + + // don't recompile if nothing changed + val Right(result2) = eval.apply(module.core.compileScalaPB): @unchecked + assert(result2.evalCount == 0) + } + } + } + def tests: Tests = Tests { test("scalapbVersion") { @@ -97,30 +148,9 @@ object TutorialTests extends TestSuite { } test("compileScalaPB") { - test("calledDirectly") - UnitTester(Tutorial, resourcePath).scoped { eval => - if (!mill.constants.Util.isWindows) { - val Right(result) = eval.apply(Tutorial.core.compileScalaPB): @unchecked - - val outPath = protobufOutPath(eval) - - val outputFiles = os.walk(result.value.path).filter(os.isFile) - - val expectedSourcefiles = compiledSourcefiles.map(outPath / _) - - assert( - result.value.path == eval.outPath / "core/compileScalaPB.dest", - outputFiles.nonEmpty, - outputFiles.forall(expectedSourcefiles.contains), - outputFiles.size == 5, - result.evalCount > 0 - ) - - // don't recompile if nothing changed - val Right(result2) = eval.apply(Tutorial.core.compileScalaPB): @unchecked - - assert(result2.evalCount == 0) - } - } + test("scalaGen") - testCompilation(Tutorial, compiledScalaSourcefiles) + test("javaGen") - testCompilation(TutorialWithJavaGen, compiledJavaSourcefiles) + test("scalaAndJavaGen") - testCompilation(TutorialWithScalaAndJavaGen, compiledScalaSourcefiles ++ compiledJavaSourcefiles) test("calledWithSpecificFile") - UnitTester( TutorialWithSpecificSources, @@ -165,7 +195,7 @@ object TutorialTests extends TestSuite { // // // val outputFiles = os.walk(outPath).filter(_.isFile) // -// // val expectedSourcefiles = compiledSourcefiles.map(outPath / _) +// // val expectedSourcefiles = compiledScalaSourcefiles.map(outPath / _) // // // assert( // // outputFiles.nonEmpty, From 12b13f9b8ad18760173ae5dc6f444354dfeada69 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sat, 6 Sep 2025 16:21:44 +0000 Subject: [PATCH 2/4] [autofix.ci] apply automated fixes --- .../mill/contrib/scalapblib/ScalaPBWorker.scala | 15 ++++++++++++--- .../mill/contrib/scalapblib/TutorialTests.scala | 9 ++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorker.scala b/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorker.scala index 72e4ab37ef5e..b1bc99a8cbd1 100644 --- a/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorker.scala +++ b/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorker.scala @@ -24,7 +24,8 @@ class ScalaPBWorker { val scalaPBCompilerClass = cl.loadClass("scalapb.ScalaPBC") val mainMethod = scalaPBCompilerClass.getMethod("main", classOf[Array[java.lang.String]]) val args = otherArgs ++ generators.map { gen => - val opts = if (scalaPBOptions.isEmpty || !gen.supportsScalaPBOptions) "" else scalaPBOptions + ":" + val opts = if (scalaPBOptions.isEmpty || !gen.supportsScalaPBOptions) "" + else scalaPBOptions + ":" s"${gen.generator}=$opts${generatedDirectory.getCanonicalPath}" } ++ roots.map(root => s"--proto_path=${root.getCanonicalPath}") ++ sources.map( _.getCanonicalPath @@ -89,7 +90,14 @@ class ScalaPBWorker { Seq(ioFile) } val roots = scalaPBSources.map(_.toIO).filter(_.isDirectory) - compiler.compileScalaPB(roots, sources, scalaPBOptions, dest.toIO, scalaPBCExtraArgs, generators) + compiler.compileScalaPB( + roots, + sources, + scalaPBOptions, + dest.toIO, + scalaPBCExtraArgs, + generators + ) mill.api.Result.Success(PathRef(dest)) } } @@ -115,7 +123,8 @@ case object ScalaGen extends Generator { } case object JavaGen extends Generator { override def generator: String = "--java_out" - override def supportsScalaPBOptions: Boolean = false // Java options are specified directly in the proto file + override def supportsScalaPBOptions: Boolean = + false // Java options are specified directly in the proto file } object ScalaPBWorkerApi extends ExternalModule { diff --git a/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala b/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala index e9e3b6d26d44..30c41fe568da 100644 --- a/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala +++ b/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala @@ -108,8 +108,8 @@ object TutorialTests extends TestSuite { // Helper function to test compilation with different generators def testCompilation( - module: TutorialBase, - expectedFiles: Seq[os.RelPath] + module: TutorialBase, + expectedFiles: Seq[os.RelPath] ): Unit = { UnitTester(module, resourcePath).scoped { eval => if (!mill.constants.Util.isWindows) { @@ -150,7 +150,10 @@ object TutorialTests extends TestSuite { test("compileScalaPB") { test("scalaGen") - testCompilation(Tutorial, compiledScalaSourcefiles) test("javaGen") - testCompilation(TutorialWithJavaGen, compiledJavaSourcefiles) - test("scalaAndJavaGen") - testCompilation(TutorialWithScalaAndJavaGen, compiledScalaSourcefiles ++ compiledJavaSourcefiles) + test("scalaAndJavaGen") - testCompilation( + TutorialWithScalaAndJavaGen, + compiledScalaSourcefiles ++ compiledJavaSourcefiles + ) test("calledWithSpecificFile") - UnitTester( TutorialWithSpecificSources, From 566bcbf5c3193e85bf79e77cb7c398d8e50d478a Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Fri, 17 Oct 2025 21:05:39 +0200 Subject: [PATCH 3/4] Refine alternative Generator support --- .../mill/contrib/scalapblib/Generator.scala | 15 +++++++ .../contrib/scalapblib/ScalaPBModule.scala | 10 ++--- .../contrib/scalapblib/ScalaPBWorker.scala | 42 +++---------------- .../contrib/scalapblib/ScalaPBWorkerApi.scala | 21 ++++++++++ .../contrib/scalapblib/TutorialTests.scala | 4 +- 5 files changed, 48 insertions(+), 44 deletions(-) create mode 100644 contrib/scalapblib/src/mill/contrib/scalapblib/Generator.scala create mode 100644 contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorkerApi.scala diff --git a/contrib/scalapblib/src/mill/contrib/scalapblib/Generator.scala b/contrib/scalapblib/src/mill/contrib/scalapblib/Generator.scala new file mode 100644 index 000000000000..6b19501aa717 --- /dev/null +++ b/contrib/scalapblib/src/mill/contrib/scalapblib/Generator.scala @@ -0,0 +1,15 @@ +package mill.contrib.scalapblib + +import upickle.ReadWriter + +/** + * A ScalaPB generator + * @param generator The CLI option to enable the generator + * @param supportsScalaPbOptions `true` if the [[ScalaPBModule.scalaPBOptions]] should be used to read options. + */ +enum Generator(val generator: String, val supportsScalaPbOptions: Boolean) derives ReadWriter { + case ScalaGen extends Generator("--scala_out", true) + case JavaGen extends Generator("--java_out", false) + case CustomGen(override val generator: String, override val supportsScalaPbOptions: Boolean) + extends Generator(generator, supportsScalaPbOptions) +} diff --git a/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBModule.scala b/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBModule.scala index 17c6d0c228c1..06407bfafdc8 100644 --- a/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBModule.scala +++ b/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBModule.scala @@ -1,9 +1,9 @@ -package mill -package contrib.scalapblib +package mill.contrib.scalapblib import coursier.core.Version -import mill.api.{PathRef} -import mill.scalalib._ +import mill.api.{PathRef, Task} +import mill.scalalib.* +import mill.T import java.util.zip.ZipInputStream import scala.util.Using @@ -22,7 +22,7 @@ trait ScalaPBModule extends ScalaModule { def scalaPBVersion: T[String] - def scalaPBGenerators: T[Seq[Generator]] = Seq[Generator](ScalaGen) + def scalaPBGenerators: T[Seq[Generator]] = Seq(Generator.ScalaGen) def scalaPBFlatPackage: T[Boolean] = Task { false } diff --git a/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorker.scala b/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorker.scala index 8d79738e52a0..f1be74b84b17 100644 --- a/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorker.scala +++ b/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorker.scala @@ -1,11 +1,8 @@ -package mill -package contrib.scalapblib - -import java.io.File +package mill.contrib.scalapblib import mill.api.PathRef -import mill.api.{Discover, ExternalModule} -import upickle.default.ReadWriter + +import java.io.File class ScalaPBWorker { @@ -24,7 +21,7 @@ class ScalaPBWorker { val scalaPBCompilerClass = cl.loadClass("scalapb.ScalaPBC") val mainMethod = scalaPBCompilerClass.getMethod("main", classOf[Array[java.lang.String]]) val args = otherArgs ++ generators.map { gen => - val opts = if (scalaPBOptions.isEmpty || !gen.supportsScalaPBOptions) "" + val opts = if (scalaPBOptions.isEmpty || !gen.supportsScalaPbOptions) "" else scalaPBOptions + ":" s"${gen.generator}=$opts${generatedDirectory.getCanonicalPath}" } ++ roots.map(root => s"--proto_path=${root.getCanonicalPath}") ++ sources.map( @@ -65,6 +62,7 @@ class ScalaPBWorker { * @param scalaPBOptions option string specific for scala generator. (the options in `--scala_out=:output_path`) * @param dest output path * @param scalaPBCExtraArgs extra arguments other than `--scala_out=:output_path`, `--proto_path=source_parent`, `source` + * @oaram generators At least on generators to use. See [[Generator]] for available generators. * * @return execute result with path ref to `dest` */ @@ -101,33 +99,3 @@ class ScalaPBWorker { mill.api.Result.Success(PathRef(dest)) } } - -trait ScalaPBWorkerApi { - def compileScalaPB( - roots: Seq[File], - source: Seq[File], - scalaPBOptions: String, - generatedDirectory: File, - otherArgs: Seq[String], - generators: Seq[Generator] - ): Unit -} - -sealed trait Generator derives ReadWriter { - def generator: String - def supportsScalaPBOptions: Boolean -} -case object ScalaGen extends Generator { - override def generator: String = "--scala_out" - override def supportsScalaPBOptions: Boolean = true -} -case object JavaGen extends Generator { - override def generator: String = "--java_out" - override def supportsScalaPBOptions: Boolean = - false // Java options are specified directly in the proto file -} - -object ScalaPBWorkerApi extends ExternalModule { - def scalaPBWorker: Worker[ScalaPBWorker] = Task.Worker { new ScalaPBWorker() } - lazy val millDiscover = Discover[this.type] -} diff --git a/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorkerApi.scala b/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorkerApi.scala new file mode 100644 index 000000000000..3d70799d3434 --- /dev/null +++ b/contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorkerApi.scala @@ -0,0 +1,21 @@ +package mill.contrib.scalapblib + +import mill.api.{Discover, ExternalModule, Task} + +import java.io.File + +trait ScalaPBWorkerApi { + def compileScalaPB( + roots: Seq[File], + source: Seq[File], + scalaPBOptions: String, + generatedDirectory: File, + otherArgs: Seq[String], + generators: Seq[Generator] + ): Unit +} + +object ScalaPBWorkerApi extends ExternalModule { + def scalaPBWorker: Task.Worker[ScalaPBWorker] = Task.Worker { new ScalaPBWorker() } + protected lazy val millDiscover = Discover[this.type] +} diff --git a/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala b/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala index 30c41fe568da..0cb322b84e75 100644 --- a/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala +++ b/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala @@ -74,7 +74,7 @@ object TutorialTests extends TestSuite { object TutorialWithJavaGen extends TutorialBase { object core extends TutorialModule { - override def scalaPBGenerators = Seq(JavaGen) + override def scalaPBGenerators = Seq(Generator.JavaGen) } lazy val millDiscover = Discover[this.type] @@ -82,7 +82,7 @@ object TutorialTests extends TestSuite { object TutorialWithScalaAndJavaGen extends TutorialBase { object core extends TutorialModule { - override def scalaPBGenerators = Seq[Generator](ScalaGen, JavaGen) + override def scalaPBGenerators = Seq(Generator.ScalaGen, Generator.JavaGen) } lazy val millDiscover = Discover[this.type] From ce5f6f7048c34e4b5c6276fd49f771411f8f1cc0 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Fri, 17 Oct 2025 21:30:09 +0200 Subject: [PATCH 4/4] . --- .../test/src/mill/contrib/scalapblib/TutorialTests.scala | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala b/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala index 0cb322b84e75..534be19a58a8 100644 --- a/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala +++ b/contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala @@ -112,7 +112,8 @@ object TutorialTests extends TestSuite { expectedFiles: Seq[os.RelPath] ): Unit = { UnitTester(module, resourcePath).scoped { eval => - if (!mill.constants.Util.isWindows) { + if (Util.isWindows) "Skipped test on Windows" + else { val Right(result) = eval.apply(module.core.compileScalaPB): @unchecked val outPath = protobufOutPath(eval) @@ -159,7 +160,8 @@ object TutorialTests extends TestSuite { TutorialWithSpecificSources, resourcePath ).scoped { eval => - if (!Util.isWindows) { + if (Util.isWindows) "Skipped test on Windows" + else { val Right(result) = eval.apply(TutorialWithSpecificSources.core.compileScalaPB): @unchecked