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

[24.1] Backport: [GR-54871] [JDK-8229959] Execute bootstrap methods for Proxy at image build time. #758

Closed zakkak closed 4 months ago

zakkak commented 5 months ago

What would you like to backport?

https://github.com/oracle/graal/pull/9184

Why?

Should fix https://github.com/quarkusio/quarkus/issues/41283 (to be confirmed). We've seen CI test failures related to it on the 24.1 release branch here: https://github.com/zakkak/mandrel/actions/runs/9685446687/job/26725539358

Are the changes being backported merged in upstream Graal?

jerboaa commented 5 months ago

@zakkak We should expedite the backport as this seems to cause a lot of test noise in CI: https://github.com/graalvm/mandrel/issues/742#issuecomment-2199651048

zakkak commented 5 months ago

I backported this along with a rebase of our mandrel/24.1 branch on the latest 24.1 upstream release branch. This is planned to be included upstream as well so I expect my backport to get dropped in a future rebase.

jerboaa commented 5 months ago

I backported this along with a rebase of our mandrel/24.1 branch on the latest 24.1 upstream release branch.

Thanks. Could you create a PR next time, please? This makes it more apparent when changes get included.

This is planned to be included upstream as well so I expect my backport to get dropped in a future rebase.

Sure. Sounds good.

zakkak commented 5 months ago

Thanks. Could you create a PR next time, please? This makes it more apparent when changes get included.

On one hand it does make it more apparent, but on the other hand if we are going to rebase the commit hashes referenced in the PR won't be relevant any more and in the case of an upstream backport where our backport PR will be dropped that won't be logged either.

I am OK with either approach though.

jerboaa commented 5 months ago

Thanks. Could you create a PR next time, please? This makes it more apparent when changes get included.

On one hand it does make it more apparent, but on the other hand if we are going to rebase the commit hashes referenced in the PR won't be relevant any more and in the case of an upstream backport where our backport PR will be dropped that won't be logged either.

My main concern is that we (I) don't notice changes going in. If I hadn't checked the release branch, I wouldn't have seen why you said you've backported it. It's also useful to see CI with a PR running before merging it. Finally, we can collect PRs in a milestone and reference it. For example it would show up in this issue. There would be a history that we've first backported it ourselves, then - potentially - reverted it and included the upstream PR instead.

I am OK with either approach though.

Please use PRs if you can. Also for "sync with upstream" kind of work. Thank you!

zakkak commented 5 months ago

Please use PRs if you can.

Noted.

Also for "sync with upstream" kind of work. Thank you!

This would only work for merges though, rebases and force pushes can't go through a PR AFAIK.

zakkak commented 4 months ago

This is in the process of being backported upstream as part of https://github.com/oracle/graal/pull/9236

zakkak commented 4 months ago

Backported with https://github.com/oracle/graal/pull/9236 as https://github.com/oracle/graal/pull/9236/commits/62505d5c3c20e519b1a15a23238a789089b6d0bc