Skip to content
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

Fix resource registration for native compilation #40184

Merged

Conversation

zakkak
Copy link
Contributor

@zakkak zakkak commented Apr 22, 2024

NativeImageResourceBuildItem and ServiceProviderBuildItem contain a path
of the resource we should include and not a pattern or a glob. As a
result, Pattern.quote is the right method to use in order to produce a
pattern that would match the path.

The patch also remove the wrong addition of the paths "as is" to the
includes json array.

Fix up of b7f49dd (#31185)

NativeImageResourceBuildItem and ServiceProviderBuildItem contain a path
of the resource we should include and not a pattern or a glob. As a
result, `Pattern.quote` is the right method to use in order to produce a
pattern that would match the path.

The patch also remove the wrong addition of the paths "as is" to the
includes json array.

Fix up of b7f49dd
@gastaldi gastaldi added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Apr 22, 2024
Copy link
Contributor

@mkouba mkouba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that we should also clarify the javadoc of NativeImageResourceBuildItem so that it's clear what exactly does resource mean there and how it's handled later on in the NativeImageResourceConfigStep...

@zakkak
Copy link
Contributor Author

zakkak commented Apr 22, 2024

I believe that we should also clarify the javadoc of NativeImageResourceBuildItem so that it's clear what exactly does resource mean there and how it's handled later on in the NativeImageResourceConfigStep...

Done in 24507ee

This comment has been minimized.

@zakkak zakkak force-pushed the 2024-04-22-fix-double-resource-registration branch 2 times, most recently from 98552ea to 24507ee Compare April 22, 2024 12:27
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense for main and 3.10.
I will send an email to quarkus-dev to check it won't break people before backporting further.

This comment has been minimized.

@zakkak zakkak force-pushed the 2024-04-22-fix-double-resource-registration branch from 24507ee to 757537b Compare April 22, 2024 17:45
Copy link

quarkus-bot bot commented Apr 22, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 757537b.

✅ 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.

You can consult the Develocity build scans.

@mkouba mkouba merged commit 94fceeb into quarkusio:main Apr 23, 2024
51 checks passed
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Apr 23, 2024
@quarkus-bot quarkus-bot bot added this to the 3.11 - main milestone Apr 23, 2024
@gsmet gsmet modified the milestones: 3.11 - main, 3.10.0 Apr 23, 2024
@gsmet gsmet modified the milestones: 3.10.0, 3.8.5 May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants