-
-
Notifications
You must be signed in to change notification settings - Fork 413
Closed
Labels
bountyThe fix/close of this issue is rewarded with a bountyThe fix/close of this issue is rewarded with a bounty
Milestone
Description
From the maintainer Li Haoyi: I'm putting a 500USD bounty on this issue, payable by bank transfer on a merged PR implementing this.
The various Jvm.*
methods are a mess. #3770 makes them marginally more messy, but they've been messy for years and could use a cleanup.
The goal of this issue is to consolidate all subprocess and classloader spawning operations in Jvm.scala
into (approximately) the following four signatures
def spawn(mainClass: String,
classPath: Iterable[os.Path],
jvmArgs: Seq[String],
mainArgs: Seq[String],
env: Map[String, String] = null,
cwd: Path = null,
stdin: ProcessInput = Pipe,
stdout: ProcessOutput = Pipe,
stderr: ProcessOutput = os.Inherit,
mergeErrIntoOut: Boolean = false,
propagateEnv: Boolean = true,
useCpPassingJar: Boolean = false): os.SubProcess = {
val commandArgs = jvmCommandArgs(javaExe, mainClass, jvmArgs, classPath, mainArgs, useCpPassingJar)
os.spawn(commandArgs, env, cwd, stdin, stdout, stderr, mergeErrIntoOut, propagateEnv)
}
def call(mainClass: String,
classPath: Iterable[os.Path],
jvmArgs: Seq[String],
mainArgs: Seq[String],
env: Map[String, String] = null,
cwd: Path = null,
stdin: ProcessInput = Pipe,
stdout: ProcessOutput = Pipe,
stderr: ProcessOutput = os.Inherit,
mergeErrIntoOut: Boolean = false,
timeout: Long = -1,
check: Boolean = true,
propagateEnv: Boolean = true,
timeoutGracePeriod: Long = 100,
useCpPassingJar: Boolean = false): os.CommandResult = {
val commandArgs = jvmCommandArgs(javaExe, mainClass, jvmArgs, classPath, mainArgs, useCpPassingJar)
os.call(
commandArgs,
env,
cwd,
stdin,
stdout,
stderr,
mergeErrIntoOut,
timeout,
check,
propagateEnv,
timeoutGracePeriod
)
}
def spawnClassloader(classPath: Iterable[os.Path],
sharedPrefixes: Seq[String],
isolated: Boolean = true): java.net.URLClassLoader = {
mill.api.ClassLoader.create(
classPath.iterator.map(_.toNIO.toUri.toURL).toVector,
if (isolated) null else getClass.getClassLoader,
sharedPrefixes = sharedPrefixes
)()
}
def callClassloader[T](classPath: Iterable[os.Path],
sharedPrefixes: Seq[String],
isolated: Boolean = true)(f: ClassLoader => T): T = {
val oldClassloader = Thread.currentThread().getContextClassLoader
val newClassloader = spawnClassloader(classPath, sharedPrefixes, isolated)
Thread.currentThread().setContextClassLoader(newClassloader)
try {
f(newClassloader)
} finally {
Thread.currentThread().setContextClassLoader(oldClassloader)
newClassloader.close()
}
}
private def jvmCommandArgs(javaExe: String,
mainClass: String,
jvmArgs: Seq[String],
classPath: Agg[os.Path],
mainArgs: Seq[String],
useCpPassingJar: Boolean): Vector[String] = {
val classPath2 =
if (useCpPassingJar && !classPath.iterator.isEmpty) {
val passingJar = os.temp(prefix = "run-", suffix = ".jar", deleteOnExit = false)
createClasspathPassingJar(passingJar, classPath)
Agg(passingJar)
} else classPath
Vector(javaExe) ++
jvmArgs ++
Vector("-cp", classPath2.iterator.mkString(java.io.File.pathSeparator), mainClass) ++
mainArgs
}
Jvm.spawn
and Jvm.call
intentionally follow the signatures of os.spawn
and os.call
. Jvm.spawnClassloader
and Jvm.callClassloader
are in-memory variations of the theme, with no subprocess-related parameters and an API tweaked to work in-memory.
- All existing subprocess/classloader APIs in
Jvm.scala
should be refactored to go through the four APIs above. The other APIs should all be deprecated. - All code outside
Jvm.scala
spawning subprocesses or classloaders should go through those four APIs, - We should ensure all tests pass both with and without change (2) above to ensure that the behavior of the existing methods is maintained (at least as far as they are covered by Mill's existing test suite)
lefou and sake92
Metadata
Metadata
Assignees
Labels
bountyThe fix/close of this issue is rewarded with a bountyThe fix/close of this issue is rewarded with a bounty