Enable std-database in native image #12068
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Not working ATM, breaks even BASE stdlib tests. Culprit appears to be `sqlite-jdbc` client
sqlite-jdbc's NI properties file defines `--enable-url-protocols=jar`. That's experimental and breaks completely our `HostClassloader`. We exclude their config but include their feature to correctly extract shared native library.
Excel_Spec leads to segfault, potentially related to reading/writing files.
|
Now, or . randomly. Which feels like this is still related to some configuration. |
hubertp
added
the
CI: No changelog needed
|
Further narrowed it down to Excel .xls. |
Apparently `poi` requires `java.awt` and it was segfaulting in a mysterious way when it was missing.
|
Here was a hint: Spoiler alert, |
|
And we may have some issues with |
std-bits/table/src/main/resources/META-INF/native-image/org/enso/table/resource-config.json
Outdated
Show resolved
Hide resolved
| @@ -188,7 +188,8 @@ add_specs suite_builder = | |||
|
|
|||
| (table.at "enso_dates" == table.at "py_dates").to_vector . should_equal [True, True] | |||
|
|
|||
| group_builder.specify "should work with polyglot values coming from JS" <| | |||
| pending_js_missing = if Polyglot.is_language_installed "js" then Nothing else "Can't run JavaScript tests, language `js` is not installed." | |||
| group_builder.specify "should work with polyglot values coming from JS" pending=pending_js_missing <| | |||
There was a problem hiding this comment.
I see. JavaScript is no longer in the bin/enso and thus these tests have to be pending.
Yes we may not be the only ones trying to avoid dependency on AWT:
Another possible Quarkiverse goodies: Btw. there seems to be some fancy Quarkus build system. |
|
I've merged this branch with develop: and executed: that's good, isn't it? |
4 tasks
Yes, I resolved all issues yesterday, it seems. Just testing now. |
|
Interesting. Now I'm not surprised I had so many issues with it.
|
hubertp
requested review from
jdunkerley,
radeusgd,
GregoryTravis,
AdRiley,
marthasharkey and
4e6
as code owners
|
Re |
The added dependency on |
JaroslavTulach
approved these changes
Jan 23, 2025
| @@ -3718,7 +3720,7 @@ lazy val `engine-runner` = project | |||
| val NI_MODULES = | |||
| "org.graalvm.nativeimage,org.graalvm.nativeimage.builder,org.graalvm.nativeimage.base,org.graalvm.nativeimage.driver,org.graalvm.nativeimage.librarysupport,org.graalvm.nativeimage.objectfile,org.graalvm.nativeimage.pointsto,com.oracle.graal.graal_enterprise,com.oracle.svm.svm_enterprise" | |||
| val JDK_MODULES = | |||
| "jdk.localedata,jdk.httpserver,java.naming,java.net.http" | |||
| "jdk.localedata,jdk.httpserver,java.naming,java.net.http,java.desktop" | |||
There was a problem hiding this comment.
Including java.desktop is undesirable. What can we do to remove it?
- Use quarkus version?
- Replace
java.awt.Colorusage with a substitution? - Report a bug for later?
There was a problem hiding this comment.
I would prefer the last option. Configuration is rather fragile and it is easier to improve it incrementally than push it in one go.
There was a problem hiding this comment.
Go on, get this in. Otherwise we need to deal #12073 (comment) forever. Once this change is in, we can polish it more.
There was a problem hiding this comment.
quarkus-poi doesn't work out-of-the-box. It seems it relies on the fact that one builds it for redhat base image:
https://github.com/quarkiverse/quarkus-poi?tab=readme-ov-file#docker
| "name":"org.enso.table.parsing.IdentityParser" | ||
| }, | ||
| { | ||
| "name":"org.enso.table.parsing.NumberParser" |
There was a problem hiding this comment.
Good. languageserver shouldn't care about std-table classes.
| "name": "org.enso.base.time.FormatterKind" | ||
| }, | ||
| { | ||
| "name": "org.enso.base.time.Time_Of_Day_Utils" |
There was a problem hiding this comment.
Classes from org.enso.base package belong to std-base. Good.
std-bits/table/src/main/resources/META-INF/native-image/org/enso/table/jni-config.json
Outdated
Show resolved
Hide resolved
std-bits/table/src/main/resources/META-INF/native-image/org/enso/table/reflect-config.json
Outdated
Show resolved
Hide resolved
| ] | ||
| }, | ||
| { | ||
| "name": "org.openxmlformats.schemas.spreadsheetml.x2006.main.impl.WorksheetDocumentImpl", |
There was a problem hiding this comment.
This is pretty log file!
5 tasks
enso/build_tools/build/src/engine/context.rs Line 684 in 8cc8c4f
|
hubertp
added
the
CI: Clean build required
|
It appears to fail now with some weird errors that I can't reproduce locally: |
CI fails with a useless `java.lang.Exception` which is not reproducible locally. Attempting to blindly add a few methods/fields, hoping that it will fix it.
I think the huge stack trace you see is not an error, it is just a log message that was leftover. This is getting fixed in #12139. |
You are probably right, yes. The stacktraces were so common and so big that it hid issues with non-Linux OS. |
|
And, as suspected, I'm hitting the awt problem on MacOS. |
java.awt support on MacOS is still missing therefore bundling/configuring resources for it only makes sense on Linux/Windows. That was unsatisafactory. This change uses native image's substitution mechanism to avoid any awt library loading. The latter will not be functional when one attempts to use it but our implementation does not rely on it at all.
|
I used substitution after all because of lack of support for awt on MacOS - our builds were failing.This is acceptable as our implementation does not rely on that part at all. |
JaroslavTulach
approved these changes
Jan 26, 2025
| @TargetClass(java.awt.GraphicsEnvironment.class) | ||
| final class ReplacementGraphicsEnvironment { | ||
| @Substitute | ||
| public static boolean isHeadless() { return true; } |
There was a problem hiding this comment.
Yup, this one is straightforward.
| import com.oracle.svm.core.annotate.Substitute; | ||
|
|
||
| @TargetClass(java.awt.Toolkit.class) | ||
| final class ReplacementToolkit { |
There was a problem hiding this comment.
Is that all to get rid of dependency on AWT?
There was a problem hiding this comment.
For our use-cases, yes. Note that this renders AWT pretty much useless but that's OK. I suspect NI eliminates most if not all java.desktop code. java.beans (in DataFormatter) still requires java.desktop and I couldn't get rid of that dependency the same way; this time because of log4j 🤦 .
No. It needs it to perform the substitution at build-time, sadly. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Description
enso --run test/Table_Testspasses without failures.Used the opportunity to cleanup resource configs a bit and update them for each of standard libraries (not yet complete but almost).
Closes #12006 and #12018
Important Notes
We include
java.desktopmodule now, which is unfortunate but necessary forjava.beansandjava.awt.But we also use NI substitution which workarounds current
java.awtlimitations.More importantly, Language Server's NI continues to work.
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.