Skip to content

chore: include java-storage-nio changes in GraalVM config #3710

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Mar 21, 2025

Conversation

diegomarquezp
Copy link
Contributor

Follow up from #3674
Context: Some local changes were not pushed to that PR before it was merged.
Main changes:

  • include src/test/.../native-image.properties in testlib jar
  • include java-storage-nio related classes for initialization at build time

Copy link

Copy link

Quality Gate Passed Quality Gate passed for 'java_showcase_integration_tests'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@@ -99,6 +99,7 @@
<include>com/google/api/gax/rpc/testing/**</include>
<include>com/google/api/gax/rpc/mtls/**</include>
<include>com/google/api/gax/util/**</include>
<include>**/native-image.properties</include>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this change required? If yes, why #3674 didn't include it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was meant to be included in #3674, but I did not push such change.
This change allows the native-image.properties resource file to be included in the testlib jar.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are the changes we made to test/resources/META-INF/native-image/com.google.api/gax/native-image.properties in #3674 going to work in handwritten libraries without being included in the testjar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, they wouldn't work without this <include> entry. Luckily there has not been a release since #3674 was merged, so next release will include that PR and this one (if merged)

Copy link
Collaborator

Choose a reason for hiding this comment

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

SG, that's what I was suspecting, thanks for confirming!

@diegomarquezp diegomarquezp merged commit a2ba9a8 into main Mar 21, 2025
52 of 53 checks passed
@diegomarquezp diegomarquezp deleted the centralize-graal-23-ii branch March 21, 2025 21:53
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
size: xs Pull request size is extra small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants