search

Karm / mandrel-integration-tests

Integration tests for GraalVM and its Mandrel distribution. Runs Quarkus, Helidon and Micronaut applications and small targeted reproducers. The focus is solely on native-image utility and compilation of Java applications into native executables.
Apache License 2.0
5 stars 3 forks source link

[23.1] apps/versions test depends on HomeFinderFeature no longer available #182

Closed jerboaa closed 10 months ago

jerboaa commented 10 months ago

File GraalVM20OrLater.java in apps/versions uses org.graalvm.home.Version which is part of the polyglot.jar file not part of a default Mandrel install after the graal-sdk split. We should remove this test or disable it on 23.1. This fails the versions integration test with:

 2023-09-01 16:25:41.040 INFO  [o.g.t.i.u.Commands$ProcessRunner] (run) Command: [native-image, --features=org.graalvm.home.HomeFinderFeature, -jar, target/version.jar, target/version]
========================================================================================================================
GraalVM Native Image: Generating 'version' (executable)...
========================================================================================================================
For detailed information and explanations on the build output, visit:
https://github.com/oracle/graal/blob/master/docs/reference-manual/native-image/BuildOutput.md
------------------------------------------------------------------------------------------------------------------------

[1/8] Initializing...                                                                                    (0.0s @ 0.06GB)
Error: Feature org.graalvm.home.HomeFinderFeature class not found on the classpath. Ensure that the name is correct and that the class is on the classpath.

See: https://github.com/graalvm/mandrel/actions/runs/6050869361/job/16422363604#step:11:17886

zakkak commented 10 months ago

That's going to be an issue on the Qurkus side as well, see https://github.com/quarkusio/quarkus/blob/f3d8f88b6d7bca7317720779c8ead67932280e49/core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/GraalVM.java#L141

jerboaa commented 10 months ago

I'm not sure this will be a quarkus issue. polyglot.jar will be available via the graal-sdk umbrella maven artefact and, AFAIK, this is only being used at native-sources build step. It's not needed at native-image invoke time, no? This is a bit confusing... We haven't seen any quarkus related failures related to that (other than https://github.com/quarkusio/quarkus/issues/35507). More thoughts?

zakkak commented 10 months ago

Correct, Quarkus doesn't need this at native-image build time, but still needs it while preparing the application for native compilation, which means that if we switch from org.graalvm.sdk:graal-sdk to org.graalvm.sdk:nativeimage as discussed in https://github.com/oracle/graal/discussions/7224 we will then also need org.graalvm.polyglot:polyglot.

I wonder if it would be worth making Version public API and moving it to another package. I guess it's something useful for other frameworks as well.

jerboaa commented 10 months ago

It looks like more discussion needed. To me it seems like we should strive to keep the needing API surface of GraalVM in Quarkus as low as possible, so rewriting a Version class as part of Quarkus could be an option too... IMO, the last option seems most appealing to me currently since the GraalVM version is supposed to be an internal property anyway.

jerboaa commented 10 months ago

I've created https://github.com/quarkusio/quarkus/issues/35714 to build awareness on the GraalVM side.

jerboaa commented 10 months ago

I'm going to disable this test on 23.1+ as HomeFinderFeature is no longer critical in current quarkus. Quarkus still uses org.graalvm.home.Version.parse(), but I suggest to remove that dep on the quarkus side going forward. For now, it's relying on the maven artefacts, which will supply org.graalvm.home.Version from org.graalvm.polyglot:polyglot by means of using the org.graalvm.sdk:graal-sdk umbrella artefact.