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
Streamline native image config generation / Update native image config #5516
Conversation
Motivation: - Generating the native image config currently requires more than one manual steps not well-documented. - We didn't update our native image config recently. Modifications: - Updated `README.md` - `nativeImageConfig` task now merges the previously generated config, so that we don't need to merge it manually. - Added a new option `-Pscratch` that builds the config from scratch, which is disabled by default. - `nativeImageConfig` task now removes the comment patterns in `resource-config.json` automatically. - Added a few more filters to remove unnecessary entries in the generated native image config. - Updated the native image config Result: - Only one manual step left until automating native image config generation - Up-to-date native image config
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.
Looks nice!
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.
Thanks!
[ | ||
{ | ||
"type":"agent-extracted", | ||
"classes":[ |
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.
Do we need a break line here?
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.
These files are automatically generated, so I think it doesn't really matter as long as it's auto-formatted. This PR actually added an auto-formatting step, which is why we see a lot of diffs in this file.
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.
Left a super nit/optional comment! Thanks for the improvement @trustin ! 🙇 👍 🙇
@@ -26,6 +26,18 @@ plugins { | |||
`jvm-toolchains` | |||
} | |||
|
|||
// If `-Pscratch` is specified, do not source the previously generated config at core/src/main/resources/META-INF/native-image | |||
// otherwise, the previously generated config will be merged into the newly generated config. | |||
val shouldGenerateFromScratch = project.findProperty("scratch").let { |
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.
Question) Would hasProperty
be more simple (and require less explanation)?
val shouldGenerateFromScratch = project.findProperty("scratch").let { | |
val shouldGenerateFromScratch = project.hasProperty("scratch").let { |
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.
I wanted to make sure our build explicitly rejects the flags like -Pscratch=false
, which would otherwise have been treated as true
.
Motivation:
Modifications:
README.md
nativeImageConfig
task now merges the previously generated config, so that we don't need to merge it manually.-Pscratch
that builds the config from scratch, which is disabled by default.nativeImageConfig
task now removes the comment patterns inresource-config.json
automatically.Result: