Skip to content

scalapb-support-java-generation #5817

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

scalapb-support-java-generation #5817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e8e912
scalapb-support-java-generation
Sep 6, 2025
12b13f9
[autofix.ci] apply automated fixes
autofix-ci[bot] Sep 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBModule.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ trait ScalaPBModule extends ScalaModule {

def scalaPBVersion: T[String]

def scalaPBGenerators: T[Seq[Generator]] = Seq[Generator](ScalaGen)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have some docs here which answers: What is a generator? Which can be choosen from? What is the default?


def scalaPBFlatPackage: T[Boolean] = Task { false }

def scalaPBJavaConversions: T[Boolean] = Task { false }
Expand Down Expand Up @@ -141,7 +143,8 @@ trait ScalaPBModule extends ScalaModule {
scalaPBSources().map(_.path),
scalaPBOptions(),
Task.dest,
scalaPBCompileOptions()
scalaPBCompileOptions(),
scalaPBGenerators()
)
}
}
42 changes: 34 additions & 8 deletions contrib/scalapblib/src/mill/contrib/scalapblib/ScalaPBWorker.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import java.io.File

import mill.api.PathRef
import mill.api.{Discover, ExternalModule}
import upickle.default.ReadWriter

class ScalaPBWorker {

Expand All @@ -15,16 +16,18 @@ 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(" ")}")
Expand Down Expand Up @@ -70,7 +73,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 {
Expand All @@ -86,7 +90,14 @@ 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))
}
}
Expand All @@ -97,10 +108,25 @@ 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
}

Comment on lines +116 to +129
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this could be an enum.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you define Generator in it's own file? It also needs some docs what supportsScalaPBOptions means. I'd prefer to name it supportsScalaPbOptions (small b).

Are there other generators we could support? Can we add an CustomGen to let the user define and use other generators?

object ScalaPBWorkerApi extends ExternalModule {
def scalaPBWorker: Worker[ScalaPBWorker] = Task.Worker { new ScalaPBWorker() }
lazy val millDiscover = Discover[this.type]
Expand Down
87 changes: 60 additions & 27 deletions contrib/scalapblib/test/src/mill/contrib/scalapblib/TutorialTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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", ???)
Expand Down Expand Up @@ -70,19 +72,68 @@ 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)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we need to explicitly define the type of the Seq here, otherwise upickle can't find the implicit reader.
although having a single value in there seems to work fine...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does making Generator an enum fixes the issue?

}

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",
os.rel / "Include.scala",
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") {

Expand All @@ -97,30 +148,12 @@ 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,
Expand Down Expand Up @@ -165,7 +198,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,
Expand Down
Loading