Skip to content

Isolation via null parent ClassLoader #1707

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

Draft
kubukoz wants to merge 6 commits into
base: series/0.18
Choose a base branch
from
+249 −203
Draft

Isolation via null parent ClassLoader #1707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6ca4370
What if... there was no isolation?
kubukoz Apr 29, 2025
08362f1
bootstrapped!
kubukoz May 6, 2025
6e6f0b0
maybe make the plugin work sometimes
kubukoz May 7, 2025
b41ecf3
fix flag
kubukoz May 7, 2025
e2d2686
fix mill
kubukoz May 7, 2025
e918cbd
add todos
kubukoz May 7, 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
25 changes: 20 additions & 5 deletions build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ lazy val codegen = projectMatrix
.taskValue,
},
(Compile / compile) := (Compile / compile)
.dependsOn((protocol.jvm(autoScalaLibrary = false) / publishLocal))
.dependsOn((protocolJvm / publishLocal))
.value
)

Expand All @@ -479,7 +479,7 @@ lazy val `codegen-cli` = projectMatrix
.in(file("modules/codegen-cli"))
.enablePlugins(BuildInfoPlugin)
.dependsOn(codegen)
.jvmPlatform(List(Scala213), jvmDimSettings)
.jvmPlatform(List(Scala212, Scala213), jvmDimSettings)
.settings(
buildInfoPackage := "smithy4s.codegen.cli",
libraryDependencies ++= Seq(
Expand All @@ -493,7 +493,7 @@ lazy val `codegen-cli` = projectMatrix
*/
lazy val codegenPlugin = (projectMatrix in file("modules/codegen-plugin"))
.enablePlugins(SbtPlugin)
.dependsOn(codegen)
.dependsOn(`codegen-cli`)
.jvmPlatform(
scalaVersions = List(Scala212),
jvmDimSettings
Expand Down Expand Up @@ -524,6 +524,7 @@ lazy val codegenPlugin = (projectMatrix in file("modules/codegen-plugin"))

// for sbt
(codegen.jvm(Scala212) / publishLocal).value,
(`codegen-cli`.jvm(Scala212) / publishLocal).value,
(protocolJvm / publishLocal).value
)
publishLocal.value
Expand Down Expand Up @@ -563,6 +564,7 @@ lazy val millCodegenPlugin = projectMatrix
(core.jvm(Scala3) / publishLocal).value,
(dynamic.jvm(Scala213) / publishLocal).value,
(codegen.jvm(Scala213) / publishLocal).value,
(`codegen-cli`.jvm(Scala213) / publishLocal).value,

// for mill
(protocolJvm / publishLocal).value
Expand All @@ -572,7 +574,7 @@ lazy val millCodegenPlugin = projectMatrix
Test / test := (Test / test).dependsOn(publishLocal).value,
libraryDependencies ++= munitDeps.value
)
.dependsOn(codegen)
.dependsOn(`codegen-cli`)

lazy val decline = (projectMatrix in file("modules/decline"))
.settings(
Expand Down Expand Up @@ -1257,6 +1259,18 @@ def genSmithyImpl(config: Configuration) = Def.task {
m.root
}

(protocolJvm / publishLocal).value

(codegen.jvm(
Smithy4sBuildPlugin.Scala213
) / publishLocal).value

// todo: silence these publishes a bit!

(`codegen-cli`.jvm(
Smithy4sBuildPlugin.Scala213
) / publishLocal).value

val codegenCp =
(`codegen-cli`.jvm(
Smithy4sBuildPlugin.Scala213
Expand Down Expand Up @@ -1342,7 +1356,8 @@ def genSmithyImpl(config: Configuration) = Def.task {
inputs ++
skipOpt ++
dependenciesOpt ++
reposOpt
reposOpt ++
List("--fork")

val cp = codegenCp
.map(_.getAbsolutePath())
Expand Down
29 changes: 14 additions & 15 deletions modules/codegen-cli/src/smithy4s/codegen/cli/CodegenCommand.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,6 @@ object CodegenCommand {
.orNone
.map(_.getOrElse(Set.empty))

val discoverModelsOpt =
Opts
.flag(
long = "discover-models",
help =
"Indicates whether the model assembler should try to discover models in the classpath"
)
.orFalse

val allowedNSOpt: Opts[Option[Set[String]]] =
Opts
.option[List[String]](
Expand All @@ -101,24 +92,32 @@ object CodegenCommand {
)
.orNone

val forkOpt: Opts[Boolean] =
Opts
.flag(
"fork",
help = "Fork the codegen process to avoid classpath issues"
)
.orFalse

val options =
(
outputOpt,
resourceOutputOpt,
skipOpts,
discoverModelsOpt,
allowedNSOpt,
excludedNSOpt,
repositoriesOpt,
dependenciesOpt,
transformersOpt,
localJarsOpt,
specsArgs,
smithyBuildOpt
smithyBuildOpt,
forkOpt
)
.mapN {
// format: off
case (output, resourseOutput, skip, discoverModels, allowedNS, excludedNS, repositories, dependencies, transformers, localJars, specsArgs, smithyBuild) =>
case (output, resourceOutput, skip, allowedNS, excludedNS, repositories, dependencies, transformers, localJars, specsArgs, smithyBuild, fork) =>
// format: on
val dependenciesWithDefaults = {
import Defaults._
Expand All @@ -127,16 +126,16 @@ object CodegenCommand {
CodegenArgs(
specsArgs,
output.getOrElse(os.pwd),
resourseOutput.getOrElse(os.pwd),
resourceOutput.getOrElse(os.pwd),
skip,
discoverModels,
allowedNS,
excludedNS,
repositories.getOrElse(List.empty),
dependenciesWithDefaults,
transformers.getOrElse(List.empty),
localJars.getOrElse(List.empty),
smithyBuild
smithyBuild,
fork
)
}

Expand Down
99 changes: 85 additions & 14 deletions modules/codegen-cli/src/smithy4s/codegen/cli/Main.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@
*/

package smithy4s.codegen.cli

import coursier._
import coursier.cache.FileCache
import coursier.parse._
import cats.data.NonEmptyList
import cats.syntax.all._
import com.monovore.decline.Command
import com.monovore.decline.Opts
import smithy4s.codegen.Codegen
import java.net.URLClassLoader

object Main {

Expand All @@ -36,28 +39,58 @@ object Main {
)

def main(args: Array[String]): Unit = {
val argsArray = args
val out = System.out
try {
System.setOut(System.err)
commands
.parse(args.toList)
.map {
case Smithy4sCommand.Generate(args) =>
val res = Codegen.generateToDisk(args)
if (res.isEmpty) {
// Printing to stderr because we print generated files path to stdout
Console.err.println(
List(
"Nothing was generated. Make sure your targetting Smithy files or folders",
"that include Smithy definitions. Otherwise, you can also use",
"--dependencies to pull external JARs or use --local-jars to use",
"JARs located on your file system."
).mkString(" ")
Console.err.println("fork value: " + args.fork)
if (args.fork) {
// todo: less hardcoding
val cp = resolveDependencies(
s"com.disneystreaming.smithy4s:smithy4s-codegen-cli_2.13:${BuildInfo.version}" ::
args.dependencies,
args.localJars,
args.repositories
Comment on lines +52 to +56
Copy link
Contributor

@ghostbuster91 ghostbuster91 May 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Very clever 👍

There is one downside though:
This is not ideal as coursier's resolution will just pick the latest versions preventing users from using anything older than what codegen was compiled against.

But maybe this is good enough trade-off given its low complexity? 🤔

Copy link
Member Author

@kubukoz kubukoz May 7, 2025

Choose a reason for hiding this comment

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

imo this is actually good 😅 if you used older things you might be risking that the old version is not going to work with the codegen anyway, i.e. it might not be compliant with more recent requirements in the smithy model etc.

)

val cl = new URLClassLoader(
cp.map(_.toIO.toURI.toURL).toArray,
null
)
}
res.foreach(out.println)

val mainClass = cl.loadClass("smithy4s.codegen.cli.Main")
val mainMethod = mainClass.getMethod(
"main",
classOf[Array[String]]
)

val newArgs = argsArray.filterNot(_.contains("--fork"))
// run main method
Console.err.println("invoking main")
mainMethod.invoke(null, newArgs)
()
} else {
// todo: repeat this and fork handling in other commands
System.setOut(System.err)
val res = Codegen.generateToDisk(args)
if (res.isEmpty) {
// Printing to stderr because we print generated files path to stdout
Console.err.println(
List(
"Nothing was generated. Make sure your targetting Smithy files or folders",
"that include Smithy definitions. Otherwise, you can also use",
"--dependencies to pull external JARs or use --local-jars to use",
"JARs located on your file system."
).mkString(" ")
)
}
res.foreach(out.println)
}
case Smithy4sCommand.DumpModel(args) =>
// todo: support fork mode
out.println(Codegen.dumpModel(args))

case Smithy4sCommand.Version =>
Expand All @@ -77,4 +110,42 @@ object Main {
}
}

private def resolveDependencies(
dependencies: List[String],
localJars: List[os.Path],
repositories: List[String]
): Seq[os.Path] = {
val maybeRepos = RepositoryParser.repositories(repositories).either
val maybeDeps = DependencyParser
.dependencies(
dependencies,
defaultScalaVersion = smithy4s.codegen.BuildInfo.scalaBinaryVersion
)
.either
val repos = maybeRepos match {
case Left(errorMessages) =>
throw new IllegalArgumentException(
s"Failed to parse repositories with error: $errorMessages"
)
case Right(r) => r
}
val deps = maybeDeps match {
case Left(errorMessages) =>
throw new IllegalArgumentException(
s"Failed to parse dependencies with errors: $errorMessages"
)
case Right(d) => d
}
val resolvedDeps: Seq[os.Path] =
if (deps.nonEmpty) {
val fetch = Fetch(FileCache())
.addRepositories(repos: _*)
.addDependencies(deps: _*)
fetch.run().map(os.Path(_))
} else {
Seq.empty
}
resolvedDeps ++ localJars
}

}
8 changes: 4 additions & 4 deletions modules/codegen-cli/test/src/smithy4s/codegen/cli/CommandParsingSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ object CommandParsingSpec extends FunSuite {
output = os.pwd,
resourceOutput = os.pwd,
skip = Set.empty,
discoverModels = false,
allowedNS = None,
excludedNS = None,
repositories = Nil,
dependencies = defaultDependencies,
transformers = Nil,
localJars = Nil,
smithyBuild = None
smithyBuild = None,
fork = false
)
)
)
Expand Down Expand Up @@ -88,7 +88,6 @@ object CommandParsingSpec extends FunSuite {
output = os.pwd / "target",
resourceOutput = os.pwd / "target" / "openapi",
skip = Set(FileType.Openapi, FileType.Scala),
discoverModels = false,
allowedNS = Some(Set("name1", "name2")),
excludedNS = None,
repositories = List("repo1", "repo2"),
Expand All @@ -98,7 +97,8 @@ object CommandParsingSpec extends FunSuite {
os.pwd / "lib1.jar",
os.pwd / "lib2.jar"
),
smithyBuild = Some(os.pwd / "smithy-build.json")
smithyBuild = Some(os.pwd / "smithy-build.json"),
fork = false
)
)
)
Expand Down
28 changes: 20 additions & 8 deletions modules/codegen-plugin/src/smithy4s/codegen/JsonConverters.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,20 @@ private[smithy4s] object JsonConverters {
}
)

// format: off
type GenTarget = List[PathRef] :*: os.Path :*: os.Path :*: Set[FileType] :*: Boolean:*: Option[Set[String]] :*: Option[Set[String]] :*: List[String] :*: List[String] :*: List[String] :*: List[PathRef] :*: Option[PathRef] :*: LNil
// format: on
type GenTarget =
List[PathRef] :*:
os.Path :*:
os.Path :*:
Set[FileType] :*:
Option[Set[String]] :*:
Option[Set[String]] :*:
List[String] :*:
List[String] :*:
List[String] :*:
List[PathRef] :*:
Option[PathRef] :*:
Boolean :*:
LNil

// `output` and `resourceOutput` are intentionally serialized as paths
// instead of PathRefs. This is to avoid hashing the directories that are generated by smithy4s anyway
Expand All @@ -79,42 +90,43 @@ private[smithy4s] object JsonConverters {
("output", ca.output) :*:
("resourceOutput", ca.resourceOutput) :*:
("skip", ca.skip) :*:
("discoverModels", ca.discoverModels) :*:
("allowedNS", ca.allowedNS) :*:
("excludedNS", ca.excludedNS) :*:
("repositories", ca.repositories) :*:
("dependencies", ca.dependencies) :*:
("transformers", ca.transformers) :*:
("localJars", ca.localJars.map(PathRef(_))) :*:
("smithyBuild", ca.smithyBuild.map(PathRef(_))) :*:
("fork", ca.fork) :*:
LNil
},
{
case (_, specs) :*:
(_, output) :*:
(_, resourceOutput) :*:
(_, skip) :*:
(_, discoverModels) :*:
(_, allowedNS) :*:
(_, excludedNS) :*:
(_, repositories) :*:
(_, dependencies) :*:
(_, transformers) :*:
(_, localJars) :*:
(_, smithyBuild) :*: LNil =>
(_, smithyBuild) :*:
(_, fork) :*:
LNil =>
CodegenArgs(
specs.map(_.underlying),
output,
resourceOutput,
skip,
discoverModels,
allowedNS,
excludedNS,
repositories,
dependencies,
transformers,
localJars.map(_.underlying),
smithyBuild.map(_.underlying)
smithyBuild.map(_.underlying),
fork
)
}
)
Expand Down
Loading
Loading