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

Update documentation about using GraalVM configuration files #36491

Merged

Conversation

zakkak
Copy link
Contributor

@zakkak zakkak commented Oct 16, 2023

Starting with Mandrel 23.1 and GraalVM for JDK 21 the use of
-H:ResourceConfigurationFiles and -H:ReflectionConfigurationFiles is
marked as experimental and requires the use of
-H::±UnlockExperimentalVMOptions to avoid warnings (see
oracle/graal#7190), while in the future (planned for the next release)
the presence of this option will be mandatory when using experimental
options (see oracle/graal#7370).

Furthermore, as described in oracle/graal#7487 Mandrel and GraalVM will also adopt glob patterns in the future (planned for the next release) and gradually phase out the ability to use exclude patterns.

The aim of this change is to inform Quarkus users that they are
responsible for keeping such configuration files up to date and to make
clear that they need to start using the
-H::±UnlockExperimentalVMOptions option.

@github-actions
Copy link

github-actions bot commented Oct 16, 2023

🙈 The PR is closed and the preview is expired.


[source,properties]
----
quarkus.native.additional-build-args =-H:ResourceConfigurationFiles=resources-config.json
Copy link
Member

Choose a reason for hiding this comment

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

Is this option entirely gone or it's just that you need to stick to conventions and let GraalVM detect them?

Same question for the other option below.

Asking because IIRC, using these files could be useful/more flexible but maybe that's not the case anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this option entirely gone or it's just that you need to stick to conventions and let GraalVM detect them?

The latter. There is no plan to remove these options in the foreseeable future, but users should not be encouraged to use them as they can change or get removed anytime.

Asking because IIRC, using these files could be useful/more flexible but maybe that's not the case anymore?

For resources, this files are going to move to "glob patterns" any way, meaning that even if someone relies on the added flexibility at some point they won't be able to use them.

Regarding reflection I believe that io.quarkus.runtime.annotations.RegisterForReflection covers most cases, and if there are some not covered we could work in extending it instead of asking users to create their own config files.

As a more defensive approach we could keep the docs and add a warning that the way the config files are used is planned to change and users should refrain from using them.

Copy link
Member

Choose a reason for hiding this comment

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

My proposal would have been to keep the content, remove the mention of the option and enforce the file name.

But I can see your point too. So I let you decide what you prefer. If you prefer your PR as is, let me know and I'll merge 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.

My proposal would have been to keep the content, remove the mention of the option and enforce the file name.

That's not going to work, at least not without changing the way it works now. Currently Quarkus copies all json config files in the jars it generates, but only uses them in the native-image if the user explicitly defines the said options. My understanding is that this is done to prevent unintended bloat.

But I can see your point too. So I let you decide what you prefer. If you prefer your PR as is, let me know and I'll merge it.

I have no strong opinion. The options I see are:

  1. This PR, were we drop the support of custom configs entirely.

  2. Leave the documentation as is and adding a note mentioning that this is an "on your own option" and whoever uses it is responsible to make sure the configuration files are compatible with the Mandrel or GraalVM version they use, as well as for making sure they pass any additional flags to avoid warnings and build errors.

  3. Allow for custom configurations that we integrate in the json configuration files that Quarkus generates anyway, this will remove the need for using ResourceConfigurationFiles and ReflectionConfigurationFiles but will complicate the code handling the corresponding annotations and the generation of the json config files.

I think I prefer 2. Let me work on it and submit another PR.

Starting with Mandrel 23.1 and GraalVM for JDK 21 the use of
`-H:ResourceConfigurationFiles` and `-H:ReflectionConfigurationFiles` is
marked as experimental and requires the use of
`-H::±UnlockExperimentalVMOptions` to avoid warnings (see
oracle/graal#7190), while in the future (planned for the next release)
the presence of this option will be mandatory when using experimental
options (see oracle/graal#7370).

Furthermore, as described in oracle/graal#7487 Mandrel and GraalVM will also adopt glob patterns in the future (planned for the next release) and gradually phase out the ability to use exclude patterns.

The aim of this change is to inform Quarkus users that they are
responsible for keeping such configuration files up to date and to make
clear that they need to start using the
`-H::±UnlockExperimentalVMOptions` option.
@zakkak zakkak force-pushed the 2023-10-16-update-resources-reflect-config branch from 643fdae to 975871c Compare October 16, 2023 10:29
@zakkak zakkak changed the title Remove documentation encouraging the use of json config files Update documentation about using GraalVM configuration files Oct 16, 2023
@zakkak zakkak requested a review from gsmet October 16, 2023 10:30
@gsmet gsmet merged commit ede3417 into quarkusio:main Oct 16, 2023
5 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.6 - main milestone Oct 16, 2023
@zakkak zakkak deleted the 2023-10-16-update-resources-reflect-config branch October 16, 2023 19:40
@parasjain27031994
Copy link

Hi @zakkak ,

Regarding reflection I believe that io.quarkus.runtime.annotations.RegisterForReflection covers most cases, and if there are some not covered we could work in extending it instead of asking users to create their own config files.

We currently use Apache commons-beanutils to read some properties reflectively, and for it we define reflection-config.json to register attributes for reflection. Now considering the above statement could you point to some documentation on how to achieve the same thing using @RegisterForReflaction annotation, because in this case I do not have access to the class directly.

Also, @gsmet , we noticed one thing, if we define native image configuration in application.yaml file

quarkus: native: additional-build-args: -H:ReflectionConfigurationFiles=reflection-config.json

And we also pass native image confguration as build argument, it seems to not consider what is defined in the properties file. Is this behavior expected ?

./mvnw clean package -Pnative -Dquarkus.native.additional-build-args=-march=x86-64-v2 -Dmaven.test.skip

@zakkak
Copy link
Contributor Author

zakkak commented May 23, 2024

We currently use Apache commons-beanutils to read some properties reflectively, and for it we define reflection-config.json to register attributes for reflection. Now considering the above statement could you point to some documentation on how to achieve the same thing using @RegisterForReflaction annotation, because in this case I do not have access to the class directly.

But you know the class names you are interested in, right? If so you can use something like the following:

@RegisterForReflection(classNames = { "net.zakkak.MyClass", "net.zakkak.MyClass2", "io.quarkus.AnotherClass" })
public class MyReflectionRegistrationDummyClass {}

Is this behavior expected ?

Yes, unfortunately the CLI overrides the values passed in the properties file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

3 participants