Don't include GraalVM dependencies in lib directory #28650

zakkak · 2022-10-17T23:16:54Z

Since GraalVM dependencies are provided by GraalVM itself there is no need to include them in the lib directory of thin jars or in uber jars.

Supersedes #28647

CI run: https://github.com/graalvm/mandrel/actions/runs/3269063301

geoand · 2022-10-18T06:05:26Z

core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/JarResultBuildStep.java

@@ -429,6 +430,9 @@ private static boolean includeAppDep(ResolvedDependency appDep, Optional<Set<Art
        if (!appDep.isJar()) {
            return false;
        }
+        if (appDep.getGroupId().equals("org.graalvm.nativeimage") || appDep.getGroupId().equals("org.graalvm.sdk")) {


Do we need to only do this for a specific GraalVM version or will this work for all versions we support?

It should work for all versions.

Tested with 23.0-dev in https://github.com/graalvm/mandrel/actions/runs/3269063301

And being tested with 22.2 in this PR.

22.3 should be no different.

zakkak · 2022-10-18T08:33:32Z

I see a lot of timeout errors in the JVM tests. I don't see how these could relate to this change, but I am also hesitant to believe it's coincidence. Should we re-run the tests? (Update: nevermind I can do it myself :) )

geoand · 2022-10-18T08:35:12Z

Yeah, best re-run them

geoand · 2022-10-18T09:39:11Z

It seems like the test failures are genuine

zakkak · 2022-10-18T11:51:44Z

It seems like the test failures are genuine

Yes, apparently there are some dependencies on graal-sdk in JVM mode. I altered the PR to only skip the dependencies in native builds.

New CI run with oracle/graal#5232 : https://github.com/graalvm/mandrel/actions/runs/3273144262

In preparation for oracle/graal#5232

gsmet · 2022-10-18T15:13:42Z

@zakkak is it required for GraalVM 22.3 (and so should be backported) or is it 22.4+?

zakkak · 2022-10-18T15:22:03Z

No, this will only be required for GraalVM 23.0 (there will be no 22.4 it goes up to YY.3, i.e. four releases per year)

gsmet · 2022-10-18T15:35:21Z

core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/JarResultBuildStep.java

+            }
+            List<String> removedArtifacts = classLoadingConfig.removedArtifacts.get();
+            removedArtifacts.add("org.graalvm.nativeimage:svm");
+            removedArtifacts.add("org.graalvm.sdk:graal-sdk");


Can't say I'm excited about us actually updating the config. But I suppose it shouldn't be a huge problem in practice.

I went this way to reuse the existing code and avoid introducing more parameters and checks that would essentially do the same thing. As this is not urgent please let me know if you can think of any better alternatives.

quarkus-bot · 2022-10-18T18:55:59Z

Failing Jobs - Building `2ae0c0e`

Status	Name	Step	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✖	JVM Tests - JDK 11 Windows	`Build`	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 17
✖	JVM Tests - JDK 17 MacOS M1	`Build`	Failures	Logs	Raw logs
✔️	JVM Tests - JDK 18

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 Windows #

- Failing: extensions/opentelemetry/deployment 
! Skipped: integration-tests/micrometer-prometheus integration-tests/opentelemetry integration-tests/opentelemetry-grpc and 5 more

📦 extensions/opentelemetry/deployment

✖ io.quarkus.opentelemetry.deployment.instrumentation.RestClientOpenTelemetryTest.urlWithoutAuthentication line 112 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: expected: <CLIENT> but was: <SERVER>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)

⚙️ JVM Tests - JDK 17 MacOS M1 #

- Failing: extensions/vertx/deployment 
! Skipped: extensions/agroal/deployment extensions/amazon-lambda-http/deployment extensions/amazon-lambda-rest/deployment and 340 more

📦 extensions/vertx/deployment

✖ io.quarkus.vertx.DuplicatedContextTest.testThatBlockingEventConsumersAreCalledOnDuplicatedContext - More details - Source on GitHub

(RECIPIENT_FAILURE,8185) java.util.concurrent.CompletionException: java.io.IOException: Broken pipe
	at io.vertx.core.eventbus.Message.fail(Message.java:141)
	at io.quarkus.vertx.runtime.VertxRecorder$3$1$1.handle(VertxRecorder.java:122)

zakkak · 2022-10-19T08:36:21Z

Test failures seem unrelated to the PR.

@gsmet we would need a fix sooner than later since oracle/graal#5232 is now merged upstream and is causing CI failures (when testing with 23.0-dev), please advise on how to proceed.

jerboaa · 2022-10-20T12:29:34Z

@gsmet we would need a fix sooner than later since oracle/graal#5232 is now merged upstream and is causing CI failures (when testing with 23.0-dev), please advise on how to proceed.

Yes, please. Our integration tests are all red because of this and are at risk of missing other breakage because of it.

gsmet · 2022-10-20T13:08:12Z

I merged it.

jerboaa · 2022-10-21T08:17:01Z

Thank you!

quarkus-bot bot added the area/core label Oct 17, 2022

zakkak marked this pull request as ready for review October 18, 2022 06:02

zakkak requested review from gsmet, geoand and Sanne October 18, 2022 06:02

geoand reviewed Oct 18, 2022

View reviewed changes

geoand approved these changes Oct 18, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

zakkak force-pushed the 2022-10-18-skip-graal-deps-from-libs branch from 8d5ddbd to 2b28b05 Compare October 18, 2022 11:49

geoand approved these changes Oct 18, 2022

View reviewed changes

zakkak mentioned this pull request Oct 18, 2022

Mark graal-sdk and svm dependencies as "provided" #28647

Closed

zakkak added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 18, 2022

zakkak mentioned this pull request Oct 18, 2022

[GR-40187] Dectect invalid use of SVM specific classes on image class- or module-path. oracle/graal#5232

Merged

zakkak force-pushed the 2022-10-18-skip-graal-deps-from-libs branch 2 times, most recently from 0c57ee7 to 574645b Compare October 18, 2022 14:05

Don't include GraalVM dependencies in lib directory for native builds

2ae0c0e

In preparation for oracle/graal#5232

zakkak force-pushed the 2022-10-18-skip-graal-deps-from-libs branch from 574645b to 2ae0c0e Compare October 18, 2022 14:08

gsmet reviewed Oct 18, 2022

View reviewed changes

This was referenced Oct 19, 2022

[CI] Quarkus main fails with Java 17 mandrel build of graal/master on Linux graalvm/mandrel#399

Closed

[CI] Quarkus main fails with Java 17 mandrel build of graal/master on Windows graalvm/mandrel#400

Closed

gsmet added triage/backport-2.13 and removed triage/backport-2.13 labels Oct 20, 2022

gsmet merged commit 92d9151 into quarkusio:main Oct 20, 2022

quarkus-bot bot added this to the 2.14 - main milestone Oct 20, 2022

quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 20, 2022

zakkak deleted the 2022-10-18-skip-graal-deps-from-libs branch October 20, 2022 13:12

zakkak mentioned this pull request Jan 9, 2023

Adds perf smoke tests, report and fixes incompatibilities Karm/mandrel-integration-tests#127

Merged

Karm mentioned this pull request Feb 8, 2023

HotSpot mode NoClassDefFoundError: org/graalvm/nativeimage/ImageInfo #30998

Closed

zakkak mentioned this pull request Sep 11, 2023

Initial test of updated workflow with local graalvm artefacts graalvm/mandrel#565

Merged

2 tasks

jerboaa mentioned this pull request Sep 12, 2023

[GraalVM 23.1+] Several quarkus native tests fail with locally installed GraalVM 23.1+ maven artefacts #35872

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't include GraalVM dependencies in lib directory #28650

Don't include GraalVM dependencies in lib directory #28650

zakkak commented Oct 17, 2022

geoand Oct 18, 2022

zakkak Oct 18, 2022

geoand Oct 18, 2022

This comment has been minimized.

zakkak commented Oct 18, 2022 •

edited

Loading

geoand commented Oct 18, 2022

This comment has been minimized.

geoand commented Oct 18, 2022

zakkak commented Oct 18, 2022

gsmet commented Oct 18, 2022

zakkak commented Oct 18, 2022 •

edited

Loading

gsmet Oct 18, 2022

zakkak Oct 18, 2022

quarkus-bot bot commented Oct 18, 2022

zakkak commented Oct 19, 2022

jerboaa commented Oct 20, 2022

gsmet commented Oct 20, 2022

jerboaa commented Oct 21, 2022

Don't include GraalVM dependencies in lib directory #28650

Don't include GraalVM dependencies in lib directory #28650

Conversation

zakkak commented Oct 17, 2022

geoand Oct 18, 2022

Choose a reason for hiding this comment

zakkak Oct 18, 2022

Choose a reason for hiding this comment

geoand Oct 18, 2022

Choose a reason for hiding this comment

This comment has been minimized.

zakkak commented Oct 18, 2022 • edited Loading

geoand commented Oct 18, 2022

This comment has been minimized.

geoand commented Oct 18, 2022

zakkak commented Oct 18, 2022

gsmet commented Oct 18, 2022

zakkak commented Oct 18, 2022 • edited Loading

gsmet Oct 18, 2022

Choose a reason for hiding this comment

zakkak Oct 18, 2022

Choose a reason for hiding this comment

quarkus-bot bot commented Oct 18, 2022

Failing Jobs - Building 2ae0c0e

Failures

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/opentelemetry/deployment

⚙️ JVM Tests - JDK 17 MacOS M1 #

📦 extensions/vertx/deployment

zakkak commented Oct 19, 2022

jerboaa commented Oct 20, 2022

gsmet commented Oct 20, 2022

jerboaa commented Oct 21, 2022

zakkak commented Oct 18, 2022 •

edited

Loading

zakkak commented Oct 18, 2022 •

edited

Loading

Failing Jobs - Building `2ae0c0e`