search

graalvm / mandrel

Mandrel is a downstream distribution of the GraalVM community edition. Mandrel's main goal is to provide a native-image release specifically to support Quarkus.
Other
397 stars 15 forks source link

[23.1] Backports resolving OpenJDK 21.0.4 updates compatibility issues #738

Closed zakkak closed 6 months ago

zakkak commented 6 months ago

Resolves issues introduced by the JDK 21 backports of:

It does so by backporting the following:

and applying a fix on top:

jerboaa commented 6 months ago

@zakkak This supersedes https://github.com/graalvm/mandrel/pull/736 right?

zakkak commented 6 months ago

@zakkak This supersedes #736 right?

Right.

simonis commented 6 months ago

Sorry for being late on this, I'm now back to work on this topic. Thanks for updating the changes required for 21.0.4 compatibility. I think we still want to get these changes into the upstream https://github.com/graalvm/graalvm-for-jdk21-community-backports as well, so I'll update my old #736 with the additional changes from this PR.

@zakkak, @jerboaa what are your concrete plans on this PR? I think ideally they should be done in the https://github.com/graalvm/graalvm-for-jdk21-community-backports repository and merged from there. But I'm not sure if any of us currently has commit rights for https://github.com/graalvm/graalvm-for-jdk21-community-backports. If that's not the case and if we don't get them until the end of May (May, 28th will be the last merge from jdk21u-dev to jdk21u) I think it's fine to proceed with these changes in the Mandrel repository first.

What do you think?

jerboaa commented 6 months ago

@zakkak, @jerboaa what are your concrete plans on this PR?

@simonis JDK 21.0.4 ea builds are already running in our CI and we need to get this in ASAP in order to catch other issues with newer EA builds. While it's ideal to get this into the community repos, the issue of maintenance isn't yet resolved and there is a chance that it'll be taking longer than we'd want. Nevertheless we'll sync it there once the new maintainers are in place. For 21.0.5 onwards we'll directly go to the community repo and merge downstream. If the community maintenance question is resolved earlier even better! Does that sound OK?

jerboaa commented 6 months ago

Getting the maintenance repo issue resolved by Tuesday next week seems very optimistic.

zakkak commented 6 months ago

Getting the maintenance repo issue resolved by Tuesday next week seems very optimistic.

I agree. @jerboaa please review when possible. Even if we don't end up merging the exact changes upstream we can fix this retroactively (as done in the past).

jerboaa commented 6 months ago

@zakkak I've reviewed this on a per-commit basis for context. It would also be good if the commit messages referred to the relevant JDK bug (where it doesn't).

zakkak commented 6 months ago

For better per commit tracking I'd suggest to divide 79eb001 into the corresponding changes that got cherry-picked. This allows for an easier 1-to-1 mapping of the backports.

In particular these cherry-picks should get amended:

* [1e48201](https://github.com/graalvm/mandrel/commit/1e482015f98677adaf7b4251edb67d591d587a02)

* [be359a8](https://github.com/graalvm/mandrel/commit/be359a85ee267ad6db7f37834bd08724d152ab42)

* [95d0057](https://github.com/graalvm/mandrel/commit/95d005723430fbcc465cf7392e02c170bf77e2f5)

Good, I will try to do that.

zakkak commented 6 months ago

Good, I will try to do that.

Done, I also amended the commit messages to indicate that I adopted the conditions on top of the corresponding cherry picks. Let's see what the CI thinks as well.

zakkak commented 6 months ago

3601c1a should mention https://bugs.openjdk.org/browse/JDK-8312498 in the commit message as that is the change which required the graal changes. Otherwise, looks good.

Done. I will merge once the CI completes.

jerboaa commented 6 months ago

Done. I will merge once the CI completes.

Thanks, sure!