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.

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

ci: update native-image testing to GraalVM 21 #983

Merged
merged 2 commits into from
Oct 4, 2023

Conversation

kkriske
Copy link
Contributor

@kkriske kkriske commented Sep 29, 2023

Update the native-image GitHub Actions flow to test against GraalVM 21 instead of GraalVM 20.
The change of the org.graalvm.sdk:graal-sdk to org.graalvm.sdk:nativeimage is a rename described here: https://www.graalvm.org/release-notes/JDK_21#refactoring-graalvm-sdk

@kkriske kkriske mentioned this pull request Sep 29, 2023
@gotson
Copy link
Collaborator

gotson commented Oct 3, 2023

@kkriske before i can look into this i may need you to help me on https://github.com/xerial/sqlite-jdbc/actions/runs/6391166703/job/17345891110

i added slf4j-api as a dependency, and it seems to be breaking the native build.

@kkriske
Copy link
Contributor Author

kkriske commented Oct 3, 2023

@gotson You can try and follow the exception, and add --trace-class-initialization=org.slf4j.LoggerFactory and --trace-class-initialization=ch.qos.logback.classic.Logger to the native-image arguments. That should give more insights. Since it's logger related it's probably got to do something with static logger fields in build-time classes.

I'm currently on a work trip so it'll be a week before I can look at it more closely.

@gotson
Copy link
Collaborator

gotson commented Oct 4, 2023

@gotson You can try and follow the exception, and add --trace-class-initialization=org.slf4j.LoggerFactory and --trace-class-initialization=ch.qos.logback.classic.Logger to the native-image arguments. That should give more insights. Since it's logger related it's probably got to do something with static logger fields in build-time classes.

I'm currently on a work trip so it'll be a week before I can look at it more closely.

Thanks, i think i managed to fix it via 89dbda1

hopefully i did the right thing ! CI is now green.

@gotson gotson merged commit 1229f00 into xerial:master Oct 4, 2023
24 checks passed
@kkriske
Copy link
Contributor Author

kkriske commented Oct 4, 2023

hopefully i did the right thing ! CI is now green.

config in maven just for tests should be fine. I would argue against adding RuntimeClassInitialization.initializeAtBuildTime(LoggerFactory.class); to the feature. You shouldn't directly configure anything outside your own codebase, this can conflict with the existing configuration of an end user (or from logback itself if they decide to bundle one) which would prevent them from using sqlite-jdbc.
It's especially dangerous to mark external things for build-time initialization because you're not in charge of what code will be run (maybe the end user overrides the logback version) and this could have unintended consequences.

In the end the user will also get the same build error you got, and they'll have to fix it in the way they want.
I'll take a look next week if there's anything I can do to prevent the error it alltogether.

@kkriske kkriske deleted the test-against-graalvm-21 branch October 8, 2023 12:36
@kkriske
Copy link
Contributor Author

kkriske commented Oct 8, 2023

@gotson I've put my take on the build issue in this PR: #985

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants