-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Process custom image classpath after parsing options #3749
Conversation
682856d
to
d026897
Compare
* Custom image classpath processing can be impacted by exclude-config. * Moved the code out of -jar parsing, to avoid the need for specific parameter order. * Without this change, --exclude-config needs to be passed before -jar for it to have an effect. * Added verbose messages for easier debugging of native-image binary.
d026897
to
ac0f115
Compare
The CI failure seems unrelated. |
@galderz thanks for your contribution, I have assigned a reviewer to this PR |
Hi @galderz , the fact that
We cannot modify the behaviour for
Due to our left-to-right evaluation users are able to override the args coming from that Your change would break that behaviour. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #3749 (comment)
Closing this PR, If still there is any change is required, please raise a new PR |
@olpaw Thanks for the clarification. Are there any tests that verify the behaviour you mention? |
There are no explicit test of this. But our whole way of building images (especially with mx) relies on that. If you run e.g. Also the |
@olpaw On Spring side, we need to use If confirmed, I understand we can't change the global behavior of "ordering matters", but could we at least work on a fix for this specific issue to allow Happy to create a dedicated issue if that makes sense. cc @gradinac |
@sdeleuze, how about creating a subdir that contains
then use
Since config-only-cp-entry is the first classpath entry it's |
Possible also good to know that
|
@sdeleuze did the approach I suggested in #3749 (comment) work for you? |
@olpaw It's been a while since I worked on this, but the change here was no longer necessary once we understood the reason for the limitations. IIRC ordering parameters the expected way was enough. |
@olpaw I think for now the plan is to workaround that at Native Build Tools level, see graalvm/native-build-tools#185. |
@sdeleuze FYI I fixed this on master last week (so it will be in 22.2). Now if you use |
@olpaw Awesome thanks a lot! |
Closes #3742