Skip to content
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.

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

scalapblib- support specifying individual source files #2126

Merged
merged 4 commits into from
Nov 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions build.sc
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,12 @@ trait MillMimaConfig extends mima.Mima {
"mill.contrib.scoverage.ScoverageReport#workerModule.bspCompileClasspath"
)
),
contrib.scalapblib -> Seq(
// we changed signature of worker API
ProblemFilter.exclude[ReversedMissingMethodProblem](
"mill.contrib.scalapblib.ScalaPBWorkerApi.compileScalaPB"
)
),
// we added a new target and a submodule after 0.10.5
contrib.twirllib -> Seq(
ProblemFilter.exclude[ReversedMissingMethodProblem](
Expand Down
47 changes: 29 additions & 18 deletions contrib/scalapblib/src/ScalaPBWorker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,18 @@ class ScalaPBWorker extends AutoCloseable {

val instance = new ScalaPBWorkerApi {
override def compileScalaPB(
root: File,
roots: Seq[File],
sources: Seq[File],
scalaPBOptions: String,
generatedDirectory: File,
otherArgs: Seq[String]
): Unit = {
val opts = if (scalaPBOptions.isEmpty) "" else scalaPBOptions + ":"
val args = otherArgs ++ Seq(
s"--scala_out=${opts}${generatedDirectory.getCanonicalPath}",
s"--proto_path=${root.getCanonicalPath}"
) ++ sources.map(_.getCanonicalPath)
s"--scala_out=${opts}${generatedDirectory.getCanonicalPath}"
) ++ roots.map(root => s"--proto_path=${root.getCanonicalPath}") ++ sources.map(
_.getCanonicalPath
)
ctx.log.debug(s"ScalaPBC args: ${args.mkString(" ")}")
mainMethod.invoke(null, args.toArray)
}
Expand Down Expand Up @@ -83,21 +84,20 @@ class ScalaPBWorker extends AutoCloseable {
scalaPBCExtraArgs: Seq[String]
)(implicit ctx: mill.api.Ctx): mill.api.Result[PathRef] = {
val compiler = scalaPB(scalaPBClasspath)

def compileScalaPBDir(inputDir: os.Path): Unit = {
// ls throws if the path doesn't exist
if (inputDir.toIO.exists) {
val files = os
.walk(inputDir)
.filter(_.last.matches(".*.proto"))
.map(_.toIO)
.toIndexedSeq
compiler.compileScalaPB(inputDir.toIO, files, scalaPBOptions, dest.toIO, scalaPBCExtraArgs)
}
val sources = scalaPBSources.flatMap {
path =>
val ioFile = path.toIO
// ls throws if the path doesn't exist
if (ioFile.exists() && ioFile.isDirectory)
os
.walk(path)
.filter(_.last.matches(".*.proto"))
.map(_.toIO)
else
Seq(ioFile)
}

scalaPBSources.foreach(compileScalaPBDir)

val roots = scalaPBSources.map(_.toIO).filter(_.isDirectory)
compiler.compileScalaPB(roots, sources, scalaPBOptions, dest.toIO, scalaPBCExtraArgs)
mill.api.Result.Success(PathRef(dest))
}

Expand All @@ -107,12 +107,23 @@ class ScalaPBWorker extends AutoCloseable {
}

trait ScalaPBWorkerApi {

@deprecated("Use other overload instead", "Mill after 0.10.9")
def compileScalaPB(
root: File,
Copy link
Member

Choose a reason for hiding this comment

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

I suggest to keep an overloaded (maybe deprecated) method with the old signature (it can simply forward to the new implementation though), to keep binary compatibility and avoid breaking someones workflow. You never know, who is already using or integrating this module.

source: Seq[File],
scalaPBOptions: String,
generatedDirectory: File,
otherArgs: Seq[String]
): Unit =
compileScalaPB(Seq(root), source, scalaPBOptions, generatedDirectory, otherArgs)

def compileScalaPB(
roots: Seq[File],
source: Seq[File],
scalaPBOptions: String,
generatedDirectory: File,
otherArgs: Seq[String]
): Unit
}

Expand Down
52 changes: 48 additions & 4 deletions contrib/scalapblib/test/src/TutorialTests.scala
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package mill.contrib.scalapblib

import mill.T
import mill.api.PathRef
import mill.api.{PathRef, Result}
import mill.define.Sources
import mill.util.{TestEvaluator, TestUtil}
import utest.framework.TestPath
import utest.{TestSuite, Tests, assert, _}

object TutorialTests extends TestSuite {
val testScalaPbVersion = "0.11.7"

trait TutorialBase extends TestUtil.BaseModule {
override def millSourcePath: os.Path =
Expand All @@ -15,15 +17,15 @@ object TutorialTests extends TestSuite {

trait TutorialModule extends ScalaPBModule {
def scalaVersion = sys.props.getOrElse("TEST_SCALA_2_12_VERSION", ???)
def scalaPBVersion = "0.10.1"
def scalaPBVersion = testScalaPbVersion
def scalaPBFlatPackage = true
def scalaPBIncludePath = Seq(scalaPBUnpackProto())
}

object Tutorial extends TutorialBase {

object core extends TutorialModule {
override def scalaPBVersion = "0.10.1"
override def scalaPBVersion = testScalaPbVersion
}
}

Expand All @@ -43,6 +45,20 @@ object TutorialTests extends TestSuite {
}
}

object TutorialWithSpecificSources extends TutorialBase {
object core extends TutorialModule {
override def scalaPBSources: Sources = T.sources {
millSourcePath / "protobuf" / "tutorial" / "Tutorial.proto"
}

override def scalaPBIncludePath = Seq(
scalaPBUnpackProto(),
PathRef(millSourcePath / "protobuf" / "tutorial")
)

}
}

val resourcePath: os.Path = os.pwd / "contrib" / "scalapblib" / "test" / "protobuf" / "tutorial"

def protobufOutPath(eval: TestEvaluator): os.Path =
Expand Down Expand Up @@ -74,7 +90,7 @@ object TutorialTests extends TestSuite {
val Right((result, evalCount)) = eval.apply(Tutorial.core.scalaPBVersion)

assert(
result == "0.10.1",
result == testScalaPbVersion,
evalCount > 0
)
}
Expand Down Expand Up @@ -104,6 +120,34 @@ object TutorialTests extends TestSuite {
assert(unchangedEvalCount == 0)
}

"calledWithSpecificFile" - workspaceTest(TutorialWithSpecificSources) { eval =>
val Right((result, evalCount)) = eval.apply(TutorialWithSpecificSources.core.compileScalaPB)

val outPath = protobufOutPath(eval)

val outputFiles = os.walk(result.path).filter(os.isFile)

val expectedSourcefiles = Seq[os.RelPath](
os.rel / "AddressBook.scala",
os.rel / "Person.scala",
os.rel / "TutorialProto.scala",
os.rel / "IncludeProto.scala"
).map(outPath / _)

assert(
result.path == eval.outPath / "core" / "compileScalaPB.dest",
outputFiles.nonEmpty,
outputFiles.forall(expectedSourcefiles.contains),
outputFiles.size == 3,
evalCount > 0
)

// don't recompile if nothing changed
val Right((_, unchangedEvalCount)) = eval.apply(Tutorial.core.compileScalaPB)

assert(unchangedEvalCount == 0)
}

// // This throws a NullPointerException in coursier somewhere
// //
// // "triggeredByScalaCompile" - workspaceTest(Tutorial) { eval =>
Expand Down