-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix: Make GraalVmProcessor Arguments Optional
#3772
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
Conversation
Fixes #3771 This PR makes the `-Alog4j.graalvm.groupId` and `-Alog4j.graalvm.artifactId` arguments optional. * If **no arguments** are provided, metadata is stored in: ``` META-INF/native-image/log4j-generated ``` Previously an error was thrown. * If **arguments are provided**, files go to: ``` META-INF/native-image/log4j-generated/<groupId>/<artifactId> ``` Previously `META-INF/native-image/<groupId>/<artifactId>` was used. The new path prevents collisions with user-provided metadata.
Modifies the `GraalVmProcessor` to use the hashcode of the descriptor as folder name in the case the user does not provide the recommended compiler options.
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.
Pull Request Overview
This PR makes the GraalVM metadata processor arguments optional, updates the default storage path to include a log4j-generated prefix, and refactors how reflect-config JSON is generated and emitted.
- Changed
GraalVmProcessorto compute reflect-config once into a byte array and derive a fallback folder name via a hash. - Updated
getReachabilityMetadataPathto return a warning (not error) and use the newMETA-INF/native-image/log4j-generatedhierarchy. - Expanded tests to cover new path logic, warning emission, and metadata contents.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/changelog/.2.x.x/3771_graalvm-params.xml | Added changelog entry for optional GraalVM args |
| GraalVmProcessor.java | Made processor arguments optional, added byte-buffered metadata, new path logic, and warning for missing options |
| FakePlugin.java / FakeAnnotations.java | Added test fixtures for plugins and annotation visitors |
| GraalVmProcessorTest.java | Refactored test helper, added parameterized path tests and warning check |
Comments suppressed due to low confidence (2)
log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/processor/GraalVmProcessor.java:229
- [nitpick] Consider adding a JavaDoc comment for this overloaded
getReachabilityMetadataPathmethod to explain thefallbackFolderNameparameter and how the fallback hash is computed, improving maintainability for future readers.
String getReachabilityMetadataPath(
log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/processor/GraalVmProcessor.java:230
- The use of the
@Nullableannotation requires an import (e.g.,org.jspecify.annotations.Nullableorjavax.annotation.Nullable), but no such import is present. Add the correct import to avoid a compile error.
@Nullable String groupId, @Nullable String artifactId, String fallbackFolderName) {
...e/src/main/java/org/apache/logging/log4j/core/config/plugins/processor/GraalVmProcessor.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
...e/src/main/java/org/apache/logging/log4j/core/config/plugins/processor/GraalVmProcessor.java
Show resolved
Hide resolved
Fixes #3771 This PR makes the `-Alog4j.graalvm.groupId` and `-Alog4j.graalvm.artifactId` arguments optional. * If **no arguments** are provided, metadata is stored in: ``` META-INF/native-image/log4j-generated/<content-derived-value> ``` Previously an error was thrown. * If **arguments are provided**, files go to: ``` META-INF/native-image/log4j-generated/<groupId>/<artifactId> ``` Previously `META-INF/native-image/<groupId>/<artifactId>` was used. The new path prevents collisions with user-provided metadata. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Fixes #3771
This PR makes the
-Alog4j.graalvm.groupIdand-Alog4j.graalvm.artifactIdarguments optional.If no arguments are provided, metadata is stored in:
Previously an error was thrown.
If arguments are provided, files go to:
Previously
META-INF/native-image/<groupId>/<artifactId>was used. The new path prevents collisions with user-provided metadata.