Don't use `--add-modules` with GraalVM < 22.0.0.2

[zakkak]

Support for --add-modules was added in 22.0.0.2 https://github.com/oracle/graal/commit/f7b98697f1506ddb4cd317aa2b1b57748d03d986

Fixes #28475

[Sanne]

I'm in favour to bump the minimal version, but I'm not sure about "some things won't work very well without this" - as older GraalVM versions wouldn't run native-image on the module path, so it's possible that the (single) case in which I figured we need to use --add-modules isn't actually necessary on the older version (and it would work).

That might need testing; but indeed if we were to bump minimal GraalVM version we're more likely to spare some trouble.

[Sanne]

And FYI this is the only consumer for this builditem: https://github.com/quarkusio/quarkus/blob/141753f9d4307b8ba0a0ab14c9f0e74e03af90be/extensions/jdbc/jdbc-h2/deployment/src/main/java/io/quarkus/jdbc/h2/deployment/JDBCH2Processor.java#L60-L64

[gsmet]

Yeah, I was also considering https://github.com/quarkusio/quarkus/issues/28530 as a similar issue that could be fixed the same way. Of course we would have to decide which version is now the minimal version for 2.13 and main.

My reasoning is that it's not a good idea to have a big pile of version switches if we can easily avoid it. Now, if we can't or if it's not desirable, then this PR makes perfect sense and just dismiss my review :).

[quarkus-bot[bot]]

Failing Jobs - Building d779a4adf8964b3ede96a2367b9cba2d70bbed7c

Status	Name	Step	Failures	Logs	Raw logs
:heavy_check_mark:	JVM Tests - JDK 11
:heavy_check_mark:	JVM Tests - JDK 17
✖	JVM Tests - JDK 17 MacOS M1	Set up runner	:warning: Check →	Logs	Raw logs
:heavy_check_mark:	JVM Tests - JDK 18

[Sanne]

I'll let @zakkak decide. Personally I'd love to drop the old GraalVM versions but I'm not sure of the implications - either way I believe we could merge this one first, and then drop them... up to him :)

[zakkak]

+1 on dropping support for 21.3 as there will be no further 21.3.x releases moving forward (i.e. it's reaching its EOL on the 25th of October).

If Quarkus 2.14 is not going to be released before the 25th of October then I would suggest bumping the minimum to 22.2 and the current to 22.3 (once released).

If bumping the minimum to 22.2 (released on the 26th of July) seems too drastic we can bump it to 22.1 instead.

Regarding Quarkus 2.13 the plan is to support 22.3 there as well, but to be on the safe side I would prefer keeping it compatible with 21.3 as well.

cc @jerboaa @Karm

[jerboaa]

Regarding Quarkus 2.13 the plan is to support 22.3 there as well, but to be on the safe side I would prefer keeping it compatible with 21.3 as well.

cc @jerboaa @Karm

I don't think much testing has been carried out with Quarkus 2.13 and graal/mandrel 21.3, so I'm not convinced we need to keep it (or rather this fix is worth doing). There is a cost to actually ensure it (2.13 + 21.3) in addition to the other, more recent graal versions (22.x). We should have by-in from the quarkus devs on that. It looks like if we really have to, making it compatible to 21.3 would be fairly straight forward (at least on the surface).

IMO, minimal 22.2 and default 22.3 once it releases later this month would be my preference.

[zakkak]

I don't think much testing has been carried out with Quarkus 2.13 and graal/mandrel 21.3, so I'm not convinced we need to keep it (or rather this fix is worth doing).

FWIW this combination was actually being tested nightly till the release of Quarkus 2.13, so I would say that it has been tested quiet well up to a point. Nevertheless, I am OK with not fixing https://github.com/quarkusio/quarkus/issues/28530 (note that this issue that we are commenting in does not affect Quarkus 2.13)

[gsmet]

I'm +1 for making 22.2 the minimum version in 2.14.

We used to be far more aggressive about that and I think it's fair to push people to the latest and greatest of GraalVM given how fast it evolves.

[gsmet]

Making it draft as if all goes well, we won't merge this one. Keeping it around for potential emergency plan though.