Skip to content

Commit

Permalink
Consolidate jvm subprocess and inprocess functions (com-lihaoyi#4456)
Browse files Browse the repository at this point in the history
Changes:
- added functions based on com-lihaoyi#3772 description
- the process related ones now return os-lib process result or
subprocess handle
- `callClassLoader` uses a loan pattern function and closes the
classloader after usage
- added `mill.util.ProcessUtil.toResult(processResult)` to handle
process result in same way
https://github.com/com-lihaoyi/mill/blob/main/main/util/src/mill/util/Jvm.scala#L352-L357

Some minor questions/suggestions:
1. current `spawnClassloader` is more flexible, you can pass your own
classloader rather that use `getClass.getClassLoader`, this is ok I
guess?
1. suggestion to name `spawnClassLoader` as `createClassLoader`
1. suggestion to name `callClassloader` as `withClassloader` (common
loan pattern name)
1. `getMainMethod` is not public so `runLocal` can't be removed.. so
expose `getMainMethod` or leave `runLocal` as is?
1. Some calls are inside build.mill files.
I have to wait for new version of Mill with these changes included, and
then update them?
1. Not sure if `closeClassLoaderWhenDone` is needed, I don't understand
why it is left unclosed in few places.

Closes com-lihaoyi#3772

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
  • Loading branch information
sake92 and autofix-ci[bot] committed Feb 7, 2025
1 parent 0ee85fa commit 383becf
Show file tree
Hide file tree
Showing 31 changed files with 620 additions and 253 deletions.
27 changes: 12 additions & 15 deletions bsp/worker/src/mill/bsp/worker/MillScalaBuildServer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -119,21 +119,18 @@ private trait MillScalaBuildServer extends ScalaBuildServer { this: MillBuildSer
) {
case (ev, state, id, m: TestModule, Some((classpath, testFramework, testClasspath))) =>
val (frameworkName, classFingerprint): (String, Agg[(Class[_], Fingerprint)]) =
Jvm.inprocess(
classpath.map(_.path),
classLoaderOverrideSbtTesting = true,
isolated = true,
closeContextClassLoaderWhenDone = false,
cl => {
val framework = Framework.framework(testFramework)(cl)
val discoveredTests = TestRunnerUtils.discoverTests(
cl,
framework,
Agg.from(testClasspath.map(_.path))
)
(framework.name(), discoveredTests)
}
)(new mill.api.Ctx.Home { def home = os.home })
Jvm.withClassLoader(
classPath = classpath.map(_.path).toVector,
sharedPrefixes = Seq("sbt.testing.")
) { classLoader =>
val framework = Framework.framework(testFramework)(classLoader)
val discoveredTests = TestRunnerUtils.discoverTests(
classLoader,
framework,
Agg.from(testClasspath.map(_.path))
)
(framework.name(), discoveredTests)
}
val classes = Seq.from(classFingerprint.map(classF => classF._1.getName.stripSuffix("$")))
new ScalaTestClassesItem(id, classes.asJava).tap { it =>
it.setFramework(frameworkName)
Expand Down
21 changes: 13 additions & 8 deletions contrib/jmh/src/mill/contrib/jmh/JmhModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,17 @@ trait JmhModule extends JavaModule {
def runJmh(args: String*) =
Task.Command {
val (_, resources) = generateBenchmarkSources()
Jvm.runSubprocess(
"org.openjdk.jmh.Main",
Jvm.callProcess(
mainClass = "org.openjdk.jmh.Main",
classPath = (runClasspath() ++ generatorDeps()).map(_.path) ++
Seq(compileGeneratedSources().path, resources),
mainArgs = args,
workingDir = Task.ctx().dest,
javaHome = zincWorker().javaHome().map(_.path)
cwd = Task.ctx().dest,
javaHome = zincWorker().javaHome().map(_.path),
stdin = os.Inherit,
stdout = os.Inherit
)
()
}

def listJmhBenchmarks(args: String*) = runJmh(("-l" +: args): _*)
Expand Down Expand Up @@ -82,17 +85,19 @@ trait JmhModule extends JavaModule {
os.remove.all(resourcesDir)
os.makeDir.all(resourcesDir)

Jvm.runSubprocess(
"org.openjdk.jmh.generators.bytecode.JmhBytecodeGenerator",
(runClasspath() ++ generatorDeps()).map(_.path),
Jvm.callProcess(
mainClass = "org.openjdk.jmh.generators.bytecode.JmhBytecodeGenerator",
classPath = (runClasspath() ++ generatorDeps()).map(_.path),
mainArgs = Seq(
compile().classes.path.toString,
sourcesDir.toString,
resourcesDir.toString,
"default"
),
javaHome = zincWorker().javaHome().map(_.path),
jvmArgs = forkedArgs
jvmArgs = forkedArgs,
stdin = os.Inherit,
stdout = os.Inherit
)

(sourcesDir, resourcesDir)
Expand Down
8 changes: 5 additions & 3 deletions contrib/proguard/src/mill/contrib/proguard/Proguard.scala
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,13 @@ trait Proguard extends ScalaModule {
// val result = os.proc(cmd).call(stdout = Task.dest / "stdout.txt", stderr = Task.dest / "stderr.txt")
// Task.log.debug(s"result: ${result}")

Jvm.runSubprocess(
Jvm.callProcess(
mainClass = "proguard.ProGuard",
classPath = proguardClasspath().map(_.path),
classPath = proguardClasspath().map(_.path).toVector,
mainArgs = args,
workingDir = Task.dest
cwd = Task.dest,
stdin = os.Inherit,
stdout = os.Inherit
)

// the call above already throws an exception on a non-zero exit code,
Expand Down
8 changes: 5 additions & 3 deletions kotlinlib/src/mill/kotlinlib/KotlinModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,13 @@ trait KotlinModule extends JavaModule { outer =>

Task.log.info("dokka options: " + options)

Jvm.runSubprocess(
Jvm.callProcess(
mainClass = "",
classPath = Agg.empty,
classPath = Seq.empty,
jvmArgs = Seq("-jar", dokkaCliClasspath().head.path.toString()),
mainArgs = options
mainArgs = options,
stdin = os.Inherit,
stdout = os.Inherit
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,14 +243,18 @@ trait AndroidAppKotlinModule extends AndroidAppModule with KotlinModule { outer
* @return
*/
def generatePreviews: T[Agg[PathRef]] = Task {
val previewGenOut = mill.util.Jvm.callSubprocess(
val previewGenOut = mill.util.Jvm.callProcess(
mainClass = "com.android.tools.render.compose.MainKt",
classPath = composePreviewRenderer().map(_.path) ++ layoutLibRenderer().map(_.path),
classPath =
composePreviewRenderer().map(_.path).toVector ++ layoutLibRenderer().map(_.path).toVector,
jvmArgs = Seq(
"-Dlayoutlib.thread.profile.timeoutms=10000",
"-Djava.security.manager=allow"
),
mainArgs = Seq(composePreviewArgs().path.toString())
mainArgs = Seq(composePreviewArgs().path.toString()),
cwd = Task.dest,
stdin = os.Inherit,
stdout = os.Inherit
).out.lines()

Task.log.info(previewGenOut.mkString("\n"))
Expand Down
9 changes: 5 additions & 4 deletions kotlinlib/src/mill/kotlinlib/detekt/DetektModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,13 @@ trait DetektModule extends KotlinModule {
Task.log.info("running detekt ...")
Task.log.debug(s"with $args")

Jvm.callSubprocess(
Jvm.callProcess(
mainClass = "io.gitlab.arturbosch.detekt.cli.Main",
classPath = detektClasspath().map(_.path),
classPath = detektClasspath().map(_.path).toVector,
mainArgs = args,
workingDir = millSourcePath, // allow passing relative paths for sources like src/a/b
streamOut = true,
cwd = millSourcePath, // allow passing relative paths for sources like src/a/b
stdin = os.Inherit,
stdout = os.Inherit,
check = false
).exitCode
}
Expand Down
28 changes: 18 additions & 10 deletions kotlinlib/src/mill/kotlinlib/js/KotlinJsModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,18 @@ trait KotlinJsModule extends KotlinModule { outer =>
case Some(RunTarget.Node) =>
val binaryPath = (binaryDir / s"$artifactId.${moduleKind.extension}")
.toIO.getAbsolutePath
Jvm.runSubprocessWithResult(
commandArgs = Seq(
"node"
) ++ args.value ++ Seq(binaryPath),
envArgs = envArgs,
workingDir = workingDir
val processResult = os.call(
cmd = Seq("node") ++ args.value ++ Seq(binaryPath),
env = envArgs,
cwd = workingDir,
stdin = os.Inherit,
stdout = os.Inherit,
check = false
)
if (processResult.exitCode == 0) Result.Success(processResult.exitCode)
else Result.Failure(
"Interactive Subprocess Failed (exit code " + processResult.exitCode + ")",
Some(processResult.exitCode)
)
case Some(x) =>
Result.Failure(s"Run target $x is not supported")
Expand Down Expand Up @@ -474,10 +480,12 @@ trait KotlinJsModule extends KotlinModule { outer =>
// TODO may be optimized if there is a single folder for all modules
// but may be problematic if modules use different NPM packages versions
private def nodeModulesDir = Task(persistent = true) {
Jvm.runSubprocess(
commandArgs = Seq("npm", "install", "mocha@10.2.0", "source-map-support@0.5.21"),
envArgs = Task.env,
workingDir = Task.dest
os.call(
cmd = Seq("npm", "install", "mocha@10.2.0", "source-map-support@0.5.21"),
env = Task.env,
cwd = Task.dest,
stdin = os.Inherit,
stdout = os.Inherit
)
PathRef(Task.dest)
}
Expand Down
8 changes: 5 additions & 3 deletions kotlinlib/src/mill/kotlinlib/kover/KoverModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -212,12 +212,14 @@ object Kover extends ExternalModule with KoverReportBaseModule {
s"${reportPath.toString()}.xml"
} else reportPath.toString()
args ++= Seq(s"--${reportType.toString.toLowerCase(Locale.US)}", output)
Jvm.runSubprocess(
Jvm.callProcess(
mainClass = "kotlinx.kover.cli.MainKt",
classPath = classpath,
classPath = classpath.toVector,
jvmArgs = Seq.empty[String],
mainArgs = args.result(),
workingDir = workingDir
cwd = workingDir,
stdin = os.Inherit,
stdout = os.Inherit
)
PathRef(os.Path(output))
}
Expand Down
9 changes: 5 additions & 4 deletions kotlinlib/src/mill/kotlinlib/ktfmt/KtfmtModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,13 @@ object KtfmtModule extends ExternalModule with KtfmtBaseModule with TaskModule {
if (!format) args += "--set-exit-if-changed"
args ++= sources.iterator.map(_.path.toString())

val exitCode = Jvm.callSubprocess(
val exitCode = Jvm.callProcess(
mainClass = "com.facebook.ktfmt.cli.Main",
classPath = classPath.map(_.path),
classPath = classPath.map(_.path).toVector,
mainArgs = args.result(),
workingDir = millSourcePath, // allow passing relative paths for sources like src/a/b
streamOut = true,
cwd = millSourcePath, // allow passing relative paths for sources like src/a/b
stdin = os.Inherit,
stdout = os.Inherit,
check = false
).exitCode

Expand Down
9 changes: 5 additions & 4 deletions kotlinlib/src/mill/kotlinlib/ktlint/KtlintModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,13 @@ object KtlintModule extends ExternalModule with KtlintModule with TaskModule {
.filter(f => os.exists(f) && (f.ext == "kt" || f.ext == "kts"))
.map(_.toString())

val exitCode = Jvm.callSubprocess(
val exitCode = Jvm.callProcess(
mainClass = "com.pinterest.ktlint.Main",
classPath = classPath.map(_.path),
classPath = classPath.map(_.path).toVector,
mainArgs = args.result(),
workingDir = millSourcePath,
streamOut = true,
cwd = millSourcePath,
stdin = os.Inherit,
stdout = os.Inherit,
check = false
).exitCode

Expand Down
1 change: 1 addition & 0 deletions main/api/src/mill/api/ClassLoader.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ object ClassLoader {
}
def java9OrAbove: Boolean = !System.getProperty("java.specification.version").startsWith("1.")

@deprecated("Use callClassLoader", "Mill 0.12.7")
def create(
urls: Seq[URL],
parent: java.lang.ClassLoader,
Expand Down
12 changes: 7 additions & 5 deletions main/init/src/mill/init/BuildGenModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,24 @@ trait BuildGenModule extends TaskModule {

val mainClass = buildGenMainClass()
val classPath = buildGenClasspath().map(_.path)
val exit = Jvm.callSubprocess(
val exitCode = Jvm.callProcess(
mainClass = mainClass,
classPath = classPath,
classPath = classPath.toVector,
mainArgs = args,
workingDir = root
cwd = root,
stdin = os.Inherit,
stdout = os.Inherit
).exitCode

if (exit == 0) {
if (exitCode == 0) {
val files = BuildGenUtil.buildFiles(root).map(PathRef(_)).toSeq
val config = buildGenScalafmtConfig()
Task.log.info("formatting Mill build files")
ScalafmtWorkerModule.worker().reformat(files, config)

Task.log.info("init completed, run \"mill resolve _\" to list available tasks")
} else {
throw BuildGenException(s"$mainClass exit($exit)")
throw BuildGenException(s"$mainClass exit($exitCode)")
}
}
}
Expand Down
10 changes: 6 additions & 4 deletions main/src/mill/main/VisualizeModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,12 @@ trait VisualizeModule extends mill.define.TaskModule {

g = g.graphAttr().`with`(Rank.dir(RankDir.LEFT_TO_RIGHT))

mill.util.Jvm.runSubprocess(
"mill.main.graphviz.GraphvizTools",
classpath().map(_.path),
mainArgs = Seq(s"${os.temp(g.toString)};$dest;txt,dot,json,png,svg")
mill.util.Jvm.callProcess(
mainClass = "mill.main.graphviz.GraphvizTools",
classPath = classpath().map(_.path).toVector,
mainArgs = Seq(s"${os.temp(g.toString)};$dest;txt,dot,json,png,svg"),
stdin = os.Inherit,
stdout = os.Inherit
)

os.list(dest).sorted.map(PathRef(_))
Expand Down
Loading

0 comments on commit 383becf

Please sign in to comment.