--jvm tries to find Java executable system-wide. #11500

Akirathan · 2024-11-06T16:53:58Z

Pull Request Description

Fixes --jvm option, given to the native image. This was failing on my machine, because when given --jvm option, the runner was trying to find the java executable from the distribution manager's runtime (on my system located in ~/.local/share/enso/runtime) and it used the first runtime found. But the first runtime on my system is JDK 17.

The --jvm option now tries to:

Find a JDK from the distribution manager that has the same version as the JDK used for building the engine.
- If there is not an exact version match, it tries to find a runtime from distribution manager that is newer.
- If none, fallback to system-wide search
System-wide search tries to find java from $JAVA_HOME and from $PATH. But this is just a fallback.

Important Notes

Added test to Engine CI jobs that pass --jvm argument to a native image of engine-runner
- ea3af5f
runtime-version-manager sbt project migrated to a JPMS module
engine-runner now depends on runtime-version-manager.
Removed unnecessary stuff in runtime-version-manager dealing with outdated gu Graal Updater utility.
Extracted GraalVersionManager from RuntimeVersionManager

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.
If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
or the Snowflake database integration, a run of the Extra Tests has been scheduled.
- If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

JaroslavTulach

The --jvm option now tries to find java executable from JAVA_HOME and from PATH. If that is not found, the distribution manager's runtimes are used as a fallback.

I'd say that this is the wrong order. The target audience of Enso users is unlikely to have proper JVM installed in the system. Searching for a system JVM is good for us, developers, but not for our target audience.

trying to find the java executable from the distribution manager's runtime (on my system located in ~/.local/share/enso/runtime) and it used the first runtime found. But the first runtime on my system is JDK 17.

It is good we have such a failing example! What is the layout of your Enso JVM runtimes? That is an important information for your #11274 bug report.

Anyway, the enso launcher should:

search managed runtimes (in ~/.local/share/enso/runtime) first
ignore the outdated ones - e.g. having JVM spec version lower than 21 (right now)
select the exact match, if found
select acceptable match (e.g. newer JDK), if found - probably with some warning printed
if no match found among managed runtimes, fallback to system JVM - with some warning printed, if the JDK isn't exactly the one we expect

Would the above steps work for your system setup, Pavel?

Akirathan · 2024-11-07T08:15:32Z

Searching for a system JVM is good for us, developers, but not for our target audience.

That is a good point.

It is good we have such a failing example! What is the layout of your Enso JVM runtimes? That is an important information for your #11274 bug report.

$ ls -l ~/.local/share/enso/runtime
drwxrwxr-x 1 pavel pavel 612 Nov  1  2023 graalvm-ce-java17.0.7-23.0.0
drwxrwxr-x 1 pavel pavel 114 Nov 28  2023 graalvm-ce-java21.0.1-23.1.0
drwxrwxr-x 1 pavel pavel 114 Feb 29  2024 graalvm-ce-java21.0.2-23.1.2
drwxrwxr-x 1 pavel pavel 114 Apr 16  2024 graalvm-ce-java21.0.2-24.0.0

Would the above steps work for your system setup, Pavel?

Certainly. Sounds like a good plan. I will see what I can do.

JaroslavTulach · 2024-11-07T09:06:29Z

$ ls -l ~/.local/share/enso/runtime
drwxrwxr-x 1 pavel pavel 612 Nov  1  2023 graalvm-ce-java17.0.7-23.0.0
drwxrwxr-x 1 pavel pavel 114 Nov 28  2023 graalvm-ce-java21.0.1-23.1.0
drwxrwxr-x 1 pavel pavel 114 Feb 29  2024 graalvm-ce-java21.0.2-23.1.2
drwxrwxr-x 1 pavel pavel 114 Apr 16  2024 graalvm-ce-java21.0.2-24.0.0

It would be great, if we selected the same JDK as used for the build (however that's graalvm-community-openjdk-21.0.2+13.1 and I don't see it) and then try any JDK21 (probably the highest - e.g. graalvm-ce-java21.0.2-24.0.0) maybe with a warning...

Sounds like a good plan. I will see what I can do.

DistributionManager shall do most of the selection logic. CCing @radeusgd to give us an expert guidance.

…m runtime-version-manager

radeusgd · 2024-11-07T16:38:08Z

engine/runner/src/main/java/org/enso/runner/JavaFinder.java

+  /**
+   * Tries to find {@code java} executable on the system. If a system-wide JDK is not found, tries
+   * to find it in the {@link DistributionManager distribution} runtimes.
+   *
+   * @return null if cannot be found. Otherwise, returns the absolute path to the executable, or
+   *     simply {@code java} if it is on the {@code PATH}.
+   */
+  static String findJavaExecutable() {
+    var javaHome = System.getenv("JAVA_HOME");
+    if (javaHome != null) {
+      var java = new File(javaHome, "bin/java").getAbsoluteFile();
+      if (java.exists()) {
+        return java.getAbsolutePath();
+      }
+    }
+    if (isJavaOnPath()) {
+      return "java";
+    }
+    var javaInRuntime = findJavaExecutableInDistributionRuntimes();
+    if (javaInRuntime != null) {
+      return javaInRuntime.toAbsolutePath().toString();
+    }
+    return null;
+  }


I think the precedence should actually look into distribution first and only afterwards use system JVM.

The whole purpose of ensoup was to manage JVMs related to the engine versions, so that the engine is run with the right compatible JVM and not a random version that may be incompatible with it (I guess it used to be more important before Truffle 'got unchained').

I think ideally we should first to run the JVM version required by the engine if it's found in the distribution. Then we can fallback to using the logic of searching JAVA_HOME and Path.

Ideally I'd also check the JVM version and log a warning if the version required by engine does not match the version that has been found.

JVMs related to the engine versions

This version is stored in the engine's manifest - see for example built-distribution/enso-engine-0.0.0-dev-windows-amd64/enso-0.0.0-dev/manifest.yaml.

It contains fields: graal-vm-version and graal-java-version.
You can see lib/scala/runtime-version-manager/src/main/scala/org/enso/runtimeversionmanager/components/Manifest.scala for the logic of loading the manifest and extracting the GraalVMVersion from it.

We have a GraalCEReleaseProvider which translates this GraalVMVersion into a strategy for downloading it from GitHub - but you probably don't need this.

The function graalDirectoryForVersion in RuntimeVersionManager takes GraalVMVersion and gives you back the directory name under which this JVM version will be sitting in the $ENSO_DATA_DIRECTORY/runtime (installed by ensoup).

I guess you could factor out the Manifest and graalDirectoryForVersion utilities to a common package and you could use it to figure out the JVM version tied to the engine.

Btw. the manifest also contains a jvm-options field. I'd be very happy if we could keep the (sometimes changing) options in a single place.

I guess if you want to check if the JVM version is correct you need to call java --version and somehow parse its output and compare with graal-java-version.

e.g. I get

openjdk 21.0.2 2024-01-16 OpenJDK Runtime Environment GraalVM CE 21.0.2+13.1 (build 21.0.2+13-jvmci-23.1-b30) OpenJDK 64-Bit Server VM GraalVM CE 21.0.2+13.1 (build 21.0.2+13-jvmci-23.1-b30, mixed mode, sharing)

I guess I want to extract the second word from the first line. At least for now that seems to work. But we can update the logic whenever the version format of expected JVM changes - unexpected format just means it's not the JVM we expect :)

Possibly better way than parsing output of java -version is to run a small program on the JVM:

public static void main(String... args) { System.out.prinltn(System.getProperty("java.vm.version")); // etc. }

and just read the output line by line without the need to parse words.

Cool - that indeed sounds better, I didn't think of that

Akirathan · 2024-11-07T17:20:17Z

As of 586fc73:

I have extracted GraalVersionManager from RuntimeVersionManager
- Among other things, GraalVersionManager is able to list, and sort, all the installed runtimes in the distribution (in my case, it is inside ~/.local/share/enso/runtime)
engine-runner depends on runtime-version-manager
- For that, I had to convert runtime-version-manager project to JPMS module.
Removed unnecessary stuff in runtime-version-manager dealing with outdated gu Graal Updater utility.
JavaFinder knows the exact version that was used for building -

enso/engine/runner/src/main/java/org/enso/runner/JavaFinder.java

Line 58 in 1455b02

var versionUsedForBuild =
- And tries to find the runtime from the distribution with the exact version, or newer version

Only when there is no newer runtime found in the managed distribution runtimes, JAVA_HOME and system java is tried.

...ion-manager/src/main/java/org/enso/runtimeversionmanager/components/GraalVersionManager.java

radeusgd

Looks great!

build/build/src/engine/context.rs

This reverts commit f14333d.

This reverts commit fb25d57.

Akirathan · 2024-11-14T17:16:13Z

Windows CI does not have java.exe on PATH - https://github.com/enso-org/enso/actions/runs/11840014687/job/32992849858?pr=11500#step:7:2539 . Removing this test from Windows again in 049b991

GitHub
--jvm tries to find Java executable system-wide. · enso-org/enso@fb25d57
Enso Analytics is a self-service data prep and analysis platform designed for data teams. - --jvm tries to find Java executable system-wide. · fb25d57

radeusgd · 2024-11-14T22:33:33Z

Windows CI does not have java.exe on PATH - enso-org/enso/actions/runs/11840014687/job/32992849858?pr=11500#step:7:2539 . Removing this test from Windows again in 049b991

GitHub**--jvm tries to find Java executable system-wide. · enso-org/enso@fb25d57**Enso Analytics is a self-service data prep and analysis platform designed for data teams. - --jvm tries to find Java executable system-wide. · fb25d57

What about JAVA_HOME/bin?

hubertp

I'm a bit surprised this fails Windows PR check. What's the reason?

build/build/src/engine/context.rs

hubertp · 2024-11-18T22:37:35Z

engine/runner/src/main/java/org/enso/runner/JavaFinder.java

+      logger.warn("Falling back to java on PATH: {}", javaExe);
+      return javaExe;
+    }
+    logger.warn("No JDK found on PATH. Cannot start the runtime.");


Shouldn't you be checking ENSO_JVM_PATH as well?

hubertp · 2024-11-18T22:40:03Z

engine/runner/src/main/java/org/enso/runner/JavaFinder.java

+    return null;
+  }
+
+  private static boolean isOnWindows() {


Could just use OS.isWindows() instead? It's already a dependency, right?

hubertp · 2024-11-18T22:40:55Z

engine/runner/src/main/java/org/enso/runner/JavaFinder.java

+    try {
+      ProcessBuilder processBuilder;
+      if (isOnWindows()) {
+        processBuilder = new ProcessBuilder("java.exe", "-h");


nit:

Suggested change

processBuilder = new ProcessBuilder("java.exe", "-h");

processBuilder = new ProcessBuilder("java.exe", "-v");

hubertp · 2024-11-18T22:41:05Z

engine/runner/src/main/java/org/enso/runner/JavaFinder.java

+      if (isOnWindows()) {
+        processBuilder = new ProcessBuilder("java.exe", "-h");
+      } else {
+        processBuilder = new ProcessBuilder("java", "-h");


Suggested change

processBuilder = new ProcessBuilder("java", "-h");

processBuilder = new ProcessBuilder("java", "-v");

JaroslavTulach · 2024-11-19T09:00:13Z

Congratulation to getting thru CI checks.

Fixes `--jvm` option, given to the native image. This was failing on my machine, because when given `--jvm` option, the runner was trying to find the `java` executable from the distribution manager's runtime (on my system located in `~/.local/share/enso/runtime`) and it used the first runtime found. But the first runtime on my system is JDK 17. The `--jvm` option now tries to: - Find a JDK from the distribution manager that has the same version as the JDK used for building the engine. - If there is not an exact version match, it tries to find a runtime from distribution manager that is *newer*. - If none, fallback to system-wide search - System-wide search tries to find `java` from `$JAVA_HOME` and from `$PATH`. But this is just a fallback. # Important Notes - Added test to Engine CI jobs that pass `--jvm` argument to a native image of engine-runner - ea3af5f - `runtime-version-manager` sbt project migrated to a JPMS module - `engine-runner` now depends on `runtime-version-manager`. - Removed unnecessary stuff in `runtime-version-manager` dealing with outdated `gu` Graal Updater utility. - Extracted [GraalVersionManager](https://github.com/enso-org/enso/blob/1455b025cb8e5141409ea96b75577e8884265a70/lib/scala/runtime-version-manager/src/main/java/org/enso/runtimeversionmanager/components/GraalVersionManager.java) from [RuntimeVersionManager](https://github.com/enso-org/enso/blob/d2e8994700fd36228f7873f040668013e643190a/lib/scala/runtime-version-manager/src/main/scala/org/enso/runtimeversionmanager/components/RuntimeVersionManager.scala)

--jvm tries to find Java executable system-wide.

ca2be99

Akirathan added the CI: No changelog needed Do not require a changelog entry for this PR. label Nov 6, 2024

Akirathan self-assigned this Nov 6, 2024

Akirathan requested review from 4e6, JaroslavTulach and hubertp as code owners November 6, 2024 16:53

enso-bot bot mentioned this pull request Nov 6, 2024

Rewrite more of _mega passes_ to _mini passes_ #11326

Closed

JaroslavTulach reviewed Nov 7, 2024

View reviewed changes

JaroslavTulach mentioned this pull request Nov 7, 2024

enso --jvm fails with InvalidModuleDescriptorException #11274

Closed

Akirathan added 6 commits November 7, 2024 15:44

Remove outdated GraalVMComponentUpdater and related functionality fro…

19b0c3d

…m runtime-version-manager

runtime-version-manager does not depend on akka-http

b91121f

Extract GraalVersionManager from RuntimeVersionManager

d2e8994

engine-runner depends on runtime-version-manager

b38928b

JavaFinder uses GraalVersionManager to find the java executable

b9792ab

Generate javaVersion into version-output

6b54fcc

radeusgd reviewed Nov 7, 2024

View reviewed changes

Akirathan added 4 commits November 7, 2024 18:00

GraalVMVersion is comparable

7149cf3

Add some tests to SemVer parsing

d6bf5a7

JavaFinder tries to find newer runtime than the one used for building

1455b02

Fix version-output versions mismatch

586fc73

Update docs

dd357ca

enso-bot bot mentioned this pull request Nov 8, 2024

Two BindingsMap$ResolvedImport aren't equal and raise exception on the CI #11171

Closed

Akirathan added 2 commits November 8, 2024 10:44

runtimeSearchPath does not have to be a directory

25d8110

Fix some build failures in tests

c356a0c

Akirathan added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Nov 8, 2024

radeusgd reviewed Nov 8, 2024

View reviewed changes

...ion-manager/src/main/java/org/enso/runtimeversionmanager/components/GraalVersionManager.java Outdated Show resolved Hide resolved

radeusgd approved these changes Nov 8, 2024

View reviewed changes

Akirathan added 4 commits November 13, 2024 14:19

Merge branch 'develop' into wip/akirathan/11274-jvm-cmdlineopt-ni

b89db13

Make JavaFinder Windows friendly

fd52bbf

GraalVersionManager is final

46022f2

Add test for --jvm arg

ea3af5f

Akirathan requested review from jdunkerley, AdRiley and Frizi as code owners November 13, 2024 15:09

enso-bot bot mentioned this pull request Nov 13, 2024

Java Values are displayed differently in the Chrome Devtools debugger #7890

Closed

Akirathan added 3 commits November 14, 2024 12:25

Log found JDK

df1e2ec

Add timeout to waiting for subprocess

ed31ffc

Update licenses - akka-http was removed from launcher

9fb23d7

Akirathan requested review from GregoryTravis and marthasharkey as code owners November 14, 2024 11:35

Akirathan added 2 commits November 14, 2024 13:19

When testing --jvm in CI, don't override ENSO_DATA_DIRECTORY env var

45b7889

Run --jvm test only on Linux

f14333d

Akirathan commented Nov 14, 2024

View reviewed changes

build/build/src/engine/context.rs Show resolved Hide resolved

Akirathan added 3 commits November 14, 2024 16:11

On Windows, java.exe is tried to be found on PATH

184c245

Revert "Run --jvm test only on Linux"

fb25d57

This reverts commit f14333d.

Reapply "Run --jvm test only on Linux"

049b991

This reverts commit fb25d57.

Akirathan added the CI: Ready to merge This PR is eligible for automatic merge label Nov 18, 2024

hubertp approved these changes Nov 18, 2024

View reviewed changes

mergify bot merged commit 9a49a02 into develop Nov 18, 2024
42 checks passed

mergify bot deleted the wip/akirathan/11274-jvm-cmdlineopt-ni branch November 18, 2024 22:44

JaroslavTulach mentioned this pull request Dec 9, 2024

ensoup broken due to 2024.5.1.rc3 #11794

Closed

JaroslavTulach mentioned this pull request Jan 3, 2025

Native Language Server integration with PM #11880

Merged

3 tasks

JaroslavTulach mentioned this pull request Feb 18, 2025

Native enso bundle contains GraalVM, but --jvm cannot find it #12302

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

--jvm tries to find Java executable system-wide. #11500

--jvm tries to find Java executable system-wide. #11500

Akirathan commented Nov 6, 2024 •

edited

Loading

JaroslavTulach left a comment •

edited

Loading

Akirathan commented Nov 7, 2024

JaroslavTulach commented Nov 7, 2024

radeusgd Nov 7, 2024

radeusgd Nov 7, 2024 •

edited

Loading

radeusgd Nov 7, 2024

JaroslavTulach Nov 8, 2024

radeusgd Nov 8, 2024

Akirathan commented Nov 7, 2024

radeusgd left a comment

Akirathan commented Nov 14, 2024 •

edited by unfurl-links bot

Loading

radeusgd commented Nov 14, 2024

hubertp left a comment

hubertp Nov 18, 2024

hubertp Nov 18, 2024

hubertp Nov 18, 2024

hubertp Nov 18, 2024

JaroslavTulach commented Nov 19, 2024

	processBuilder = new ProcessBuilder("java.exe", "-h");
	processBuilder = new ProcessBuilder("java.exe", "-v");

--jvm tries to find Java executable system-wide. #11500

--jvm tries to find Java executable system-wide. #11500

Conversation

Akirathan commented Nov 6, 2024 • edited Loading

Pull Request Description

Important Notes

Checklist

JaroslavTulach left a comment • edited Loading

Choose a reason for hiding this comment

Akirathan commented Nov 7, 2024

JaroslavTulach commented Nov 7, 2024

Choose a reason for hiding this comment

radeusgd Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Akirathan commented Nov 7, 2024

radeusgd left a comment

Choose a reason for hiding this comment

Akirathan commented Nov 14, 2024 • edited by unfurl-links bot Loading

radeusgd commented Nov 14, 2024

hubertp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JaroslavTulach commented Nov 19, 2024

Akirathan commented Nov 6, 2024 •

edited

Loading

JaroslavTulach left a comment •

edited

Loading

radeusgd Nov 7, 2024 •

edited

Loading

Akirathan commented Nov 14, 2024 •

edited by unfurl-links bot

Loading