Delegate striping of debug symbols to GraalVM/Mandrel when >23.0 #31294
Conversation
| * contrast to Quarkus it uses absolute paths which makes debugging harder especially for | ||
| * in-container builds. |
There was a problem hiding this comment.
I was too quick, sorry. There is no proof of this container issue. Even though the paths passed to objcopy are absolute, the links in the files end up being relative. So there seems to be no issue with upstream as far as I could tell. I'm not sure what the man page of objcopy alludes to, though. Anyway, perhaps we should use GraalVM's upstream version instead if it passes the failing integration tests?
There was a problem hiding this comment.
Still there is another difference between the GraalVM and the Quarkus approach. The former strips more info from the image while Quarkus strips only debug info. If we want to adopt the GraalVM way (which makes sense to me) we need to update Quarkus to do the same with older versions too.
Update: on second thought that's not necessary. We can allow for that difference between the versions.
There was a problem hiding this comment.
That's right. I agree that it should be OK to do slightly different debuginfo handling between 22.3 and 23.x.
zakkak
force-pushed
the
2023-02-20-debug-strip-fix
branch
from
019b5ab to
cf13dbe
Compare
zakkak
changed the title
core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildRunner.java
Outdated
Show resolved
Hide resolved
core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildRunner.java
Outdated
Show resolved
Hide resolved
@jerboaa I Added a new commit to address these two as well. Please have a look. |
|
Once reviewed by someone else as well and the CI passes I will squash the commits and merge. |
This comment has been minimized.
This comment has been minimized.
|
@gsmet Could you please help get this reviewed and integrated? Thanks! This issue is causing some noise in our mandrel CI. I've verified the fix works, fwiw. |
|
We have a job failing due to one of the commits being a fixup commit. Let's address it before merging :) |
Starting with GraalVM/Mandrel 23.0, native-image strips debug info in a separate file by default. Fixes quarkusio#31258
zakkak
force-pushed
the
2023-02-20-debug-strip-fix
branch
from
0b48cd7 to
ad468ce
Compare
zakkak
added
the
triage/waiting-for-ci
|
✔️ The latest workflow run for the pull request has completed successfully. It should be safe to merge provided you have a look at the other checks in the summary. |
quarkus-bot
bot
removed
the
triage/waiting-for-ci
|
Thanks all! |
Starting with GraalVM/Mandrel 23.0, native-image strips debug info in a separate file by default.
Fixes #31258
Fixes Karm/mandrel-integration-tests#138
cc @jerboaa