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

[Mandrel 23.0] hibernate-orm-panache-kotlin native integration tests fail with 23.0 build and 23.1.2 sdk. #39322

Closed
jerboaa opened this issue Mar 11, 2024 · 13 comments

Comments

@jerboaa
Copy link
Contributor

jerboaa commented Mar 11, 2024

Describe the bug

When running the hibernate-orm-panache-kotlin native integration test with newly update Graal SDK (23.1.2) and a Mandrel 23.0 native image builder we see build failures of the like:

Error: Class-path entry file:///home/runner/work/mandrel/mandrel/quarkus/integration-tests/hibernate-orm-panache-kotlin/target/quarkus-integration-test-hibernate-orm-panache-kotlin-999-SNAPSHOT-native-image-source-jar/lib/org.graalvm.polyglot.polyglot-23.1.2.jar contains class org.graalvm.polyglot.management.Management. This class is part of the image builder itself (in file:///home/runner/work/mandrel/mandrel/graalvm-home/lib/jvmci/graal-sdk.jar) and must not be passed via -cp. This can be caused by a fat-jar that illegally includes svm.jar (or graal-sdk.jar) due to its build-time dependency on it. As a workaround, -H:+AllowDeprecatedBuilderClassesOnImageClasspath allows turning this error into a warning. Note that this option is deprecated and will be removed in a future version.
com.oracle.svm.core.util.UserError$UserException: Class-path entry file:///home/runner/work/mandrel/mandrel/quarkus/integration-tests/hibernate-orm-panache-kotlin/target/quarkus-integration-test-hibernate-orm-panache-kotlin-999-SNAPSHOT-native-image-source-jar/lib/org.graalvm.polyglot.polyglot-23.1.2.jar contains class org.graalvm.polyglot.management.Management. This class is part of the image builder itself (in file:///home/runner/work/mandrel/mandrel/graalvm-home/lib/jvmci/graal-sdk.jar) and must not be passed via -cp. This can be caused by a fat-jar that illegally includes svm.jar (or graal-sdk.jar) due to its build-time dependency on it. As a workaround, -H:+AllowDeprecatedBuilderClassesOnImageClasspath allows turning this error into a warning. Note that this option is deprecated and will be removed in a future version.
	at org.graalvm.nativeimage.builder/com.oracle.svm.core.util.UserError.abort(UserError.java:73)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageClassLoaderSupport.reportBuilderClassesInApplication(NativeImageClassLoaderSupport.java:819)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.ImageClassLoader.loadAllClasses(ImageClassLoader.java:105)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.buildImage(NativeImageGeneratorRunner.java:296)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.build(NativeImageGeneratorRunner.java:612)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.start(NativeImageGeneratorRunner.java:134)
	at org.graalvm.nativeimage.builder/com.oracle.svm.hosted.NativeImageGeneratorRunner.main(NativeImageGeneratorRunner.java:94)

See:
https://github.com/graalvm/mandrel/actions/runs/8211076023/job/22460083047#step:12:356

Related: #39260 and #37423

It's not clear if we need to exclude the org.graalvm.polyglot:polyglot artefact when producing the native sources jar (revert #37423) or it's a Mandrel 23.0 packaging/build bug.

@jerboaa jerboaa added the kind/bug Something isn't working label Mar 11, 2024
Copy link

quarkus-bot bot commented Mar 11, 2024

/cc @FroMage (panache), @Karm (mandrel), @galderz (mandrel), @geoand (kotlin), @gsmet (hibernate-orm), @loicmathieu (panache), @yrodiere (hibernate-orm), @zakkak (mandrel,native-image)

@zakkak
Copy link
Contributor

zakkak commented Mar 11, 2024

It's not clear if we need to exclude the org.graalvm.polyglot:polyglot artefact when producing the native sources jar (revert #37423) or it's a Mandrel 23.0 packaging/build bug.

I think we need to exclude it but only when building with Mandrel 23.0, this is related to #39319 (let's see how that will conclude as well...)

@jerboaa
Copy link
Contributor Author

jerboaa commented Mar 11, 2024

Same happens with oidc, security-webauthn and smallrye-graphql native integration tests.

@zakkak
Copy link
Contributor

zakkak commented Mar 11, 2024

To clarify, this got triggered by #39260 because in 23.1 graal-sdk is essentially a wrapper that brings in org.graalvm.polyglot:polyglot (along with org.graalvm.sdk:collections, org.graalvm.sdk:nativeimage, and org.graalvm.sdk:word). In 23.1 this does not cause an issue because org.graalvm.polyglot:polyglot is only shipped as a standalone and not shipped along with GraalVM itself, while in 23.0 org.graalvm.polyglot:polyglot is part of the release itself (so it's not allowed to be on the classpath as well).

@gsmet
Copy link
Member

gsmet commented Mar 11, 2024

@zakkak that makes the case for backporting #39260 more complicated.

@gemmellr
Copy link
Contributor

gemmellr commented Mar 11, 2024

I hit similar elsewhere. The extension had a compile scope dependency on graal-sdk (formerly on svm, which quarkus-bom had at provided scope), and so failed to build with the JDK17 native builder image that is based on 23.0.3.0. Changing the graal-sdk dep to provided scope resolved things.

https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/Java.2017.20native.20build.20compatibility.20expectation.20for.203.2E8.2Ex.2B.3F/near/425894538

@zakkak
Copy link
Contributor

zakkak commented Mar 12, 2024

@gsmet @jerboaa #39372 resolves this specific issue, but I expect it to still trigger when users explicitly add org.graalvm.polyglot:polyglot as a dependency. I am afraid there's no way to support having org.graalvm.polyglot:polyglot explicitly added other than conditionally removing it from the classpath depending on the Mandrel version, as mentioned in #39322 (comment).

Do we want/need to go that way to ensure Truffle works on Q main with both 23.1 and 23.0?
WDYT?

@jerboaa
Copy link
Contributor Author

jerboaa commented Mar 12, 2024

No strong feelings either way. I guess since we know polyglot.jar is there for 23.0 we'd be wise to exclude it from the classpath there.

@jerboaa
Copy link
Contributor Author

jerboaa commented Mar 13, 2024

I'd say we just do the conditional removal of org.graalvm.polyglot:polyglot for 23.0 on the classpath.

@zakkak
Copy link
Contributor

zakkak commented Mar 13, 2024

I'd say we just do the conditional removal of org.graalvm.polyglot:polyglot for 23.0 on the classpath.

Done in #39397

@zakkak
Copy link
Contributor

zakkak commented Mar 13, 2024

An alternative would be to add -H:+AllowDeprecatedBuilderClassesOnImageClasspath when using Mandrel 23.0, as noted in the error message:

As a workaround, -H:+AllowDeprecatedBuilderClassesOnImageClasspath allows turning this error into a warning.

I am thinking that this might be better than just removing the dependency from the classpath, as it will notify the user that there is something they should probably be aware of.

WDYT? cc @jerboaa @gsmet

@jerboaa
Copy link
Contributor Author

jerboaa commented Mar 13, 2024

An alternative would be to add -H:+AllowDeprecatedBuilderClassesOnImageClasspath when using Mandrel 23.0, as noted in the error message:

As a workaround, -H:+AllowDeprecatedBuilderClassesOnImageClasspath allows turning this error into a warning.

I am thinking that this might be better than just removing the dependency from the classpath, as it will notify the user that there is something they should probably be aware of.

WDYT? cc @jerboaa @gsmet

Hmm, some users might end up with a 23.1 polyglot on the class-path and 23.0 from the builder, which in theory could produce weird and hard-to-debug results.

@zakkak
Copy link
Contributor

zakkak commented Mar 27, 2024

This is now fixed by #39442

Users explicitly depending on polyglot and using Mandrel 23.0 will need to use the maven scope provided.

@zakkak zakkak closed this as completed Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants