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.

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

Add org.jline as module to component directory #8496

Merged
merged 8 commits into from
Dec 13, 2023
Merged

Add org.jline as module to component directory #8496

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
008d185
Add org.jline as module to component directory
Akirathan Dec 7, 2023
7d1feed
Add --add-modules option to runner in project-manager
Akirathan Dec 8, 2023
04ad8b8
fmt
Akirathan Dec 8, 2023
b23313e
Do not delegate loading of org.jline classes to system class loader
Akirathan Dec 13, 2023
5428f1f
Remove --add-modules org.jline option from the launchers
Akirathan Dec 13, 2023
a6711e0
Set thread context classLoader for jline's initialization.
Akirathan Dec 13, 2023
515010e
Remove jline from component modules
Akirathan Dec 13, 2023
79b836f
fmt
Akirathan Dec 13, 2023
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
2 changes: 1 addition & 1 deletion distribution/bin/enso
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ for opt in "$@"; do
done


exec java --module-path $COMP_PATH $EXTRA_OPTS $JAVA_OPTS -m org.enso.runtime/org.enso.EngineRunnerBootLoader "$@"
exec java --module-path $COMP_PATH --add-modules org.jline $EXTRA_OPTS $JAVA_OPTS -m org.enso.runtime/org.enso.EngineRunnerBootLoader "$@"
exit
2 changes: 1 addition & 1 deletion distribution/bin/enso.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ if /I %%A==--dump-graphs (
set EXTRA_OPTS=%EXTRA_OPTS% -Dgraal.Dump=Truffle:1
)
)
java --module-path %comp-dir% -Dpolyglot.compiler.IterativePartialEscape=true %EXTRA_OPTS% %JAVA_OPTS% -m org.enso.runtime/org.enso.EngineRunnerBootLoader %*
java --module-path %comp-dir% --add-modules org.jline -Dpolyglot.compiler.IterativePartialEscape=true %EXTRA_OPTS% %JAVA_OPTS% -m org.enso.runtime/org.enso.EngineRunnerBootLoader %*
Copy link
Member

Choose a reason for hiding this comment

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

It'd be better to avoid these add-modules modifications. If we are sure we want to use org.jline as a module in enso, then I'd suggest to modify module-info.java to require static org.jline. Then the standard JVM launcher shall pick the org.jline module up, when it is available.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. Let's avoid --add-modules.

exit /B %errorlevel%
5 changes: 3 additions & 2 deletions engine/runtime/src/main/java/org/enso/ClassLoaderConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ public class ClassLoaderConstants {
* consistent.
*/
public static final List<String> CLASS_DELEGATION_PATTERNS =
List.of("org.graalvm", "java", "org.slf4j", "ch.qos");
List.of("org.graalvm", "java", "org.slf4j", "ch.qos", "org.jline");

public static final List<String> RESOURCE_DELEGATION_PATTERNS = List.of("org.slf4j", "ch.qos");
public static final List<String> RESOURCE_DELEGATION_PATTERNS =
List.of("org.slf4j", "ch.qos", "org.jline");
/**
* Path to the {@code runner.jar} fat jar. This must not be on the system's module-path, because
* the JVM would not be able to boot.
Expand Down
2 changes: 2 additions & 0 deletions ...runtime-version-manager/src/main/scala/org/enso/runtimeversionmanager/runner/Runner.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@ class Runner(
manifestOptions ++ environmentOptions ++ commandLineOptions
if (shouldInvokeViaModulePath) {
jvmArguments = jvmArguments :++ Seq(
"--add-modules",
"org.jline",
"--module-path",
engine.componentDirPath.toAbsolutePath.normalize.toString,
"-m",
Expand Down
4 changes: 3 additions & 1 deletion project/JPMSUtils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import java.nio.file.{
object JPMSUtils {
val slf4jVersion = "2.0.9"
val logbackClassicVersion = "1.3.7"
val jlineVersion = "3.23.0"

/** The list of modules that are included in the `component` directory in engine distribution.
* When invoking the `java` command, these modules need to be put on the module-path.
Expand All @@ -36,7 +37,8 @@ object JPMSUtils {
GraalVM.modules ++ GraalVM.langsPkgs ++ GraalVM.toolsPkgs ++ Seq(
"org.slf4j" % "slf4j-api" % slf4jVersion,
"ch.qos.logback" % "logback-classic" % logbackClassicVersion,
"ch.qos.logback" % "logback-core" % logbackClassicVersion
"ch.qos.logback" % "logback-core" % logbackClassicVersion,
"org.jline" % "jline" % jlineVersion
)

/** Filters modules by their IDs from the given classpath.
Expand Down