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

Add stable option for -H:IncludeResources and -H:ExcludeResources #7354

Closed
jerboaa opened this issue Sep 6, 2023 · 12 comments
Closed

Add stable option for -H:IncludeResources and -H:ExcludeResources #7354

jerboaa opened this issue Sep 6, 2023 · 12 comments

Comments

@jerboaa
Copy link
Collaborator

jerboaa commented Sep 6, 2023

Describe the issue
With a GraalVM dev build (24.0) when -H:IncludeResources=path/to/image.jpg is being used it gets flagged as an experimental option. For example, in my case it produces the following warnings on the native-image invocation:

Warning: The option '-H:IncludeResources=Grace_M._Hopper.jp2,MyFreeMono.ttf,MyFreeSerif.ttf' is experimental and must be enabled via '-H:+UnlockExperimentalVMOptions' in the future.
Warning: Please re-evaluate whether any experimental option is required, and either remove or unlock it. The build output lists all active experimental options, including where they come from and possible alternatives. If you think an experimental option should be considered as stable, please file an issue.

As per the suggestion, I'm filing this issue to consider this as an API option. Or otherwise, how would one add resources to the image in 23.1+ without running into the experimental options issue?

@jerboaa
Copy link
Collaborator Author

jerboaa commented Sep 6, 2023

/cc @fniephaus

@fniephaus
Copy link
Member

Hi @jerboaa, thanks for raising this.

-H:IncludeResources can be replaced with an appropriate resource-config.json file.

So

-H:IncludeResources=Grace_M._Hopper.jp2,MyFreeMono.ttf,MyFreeSerif.ttf

can be turned into the following resource-config.json file:

{
  "resources": {
    "includes": [
      {
        "pattern": "Grace_M._Hopper.jp2"
      },
      {
        "pattern": "MyFreeMono.ttf"
      },
      {
        "pattern": "MyFreeSerif.ttf"
      }
    ]
  }
}

I'm sure you can come up with a smarter pattern if you prefer that.

@zakkak
Copy link
Collaborator

zakkak commented Sep 6, 2023

@jerboaa also note that this is how Quarkus handles this (it generates a resource-config.json file). If you get the above message with Quarkus then it's probably due to a third party dependency passing the argument (e.g. by setting Args in native-image.properties)

@fniephaus
Copy link
Member

The build output tells you where the option is coming from:

 1 experimental option(s) unlocked:
 - '-H:IncludeResources' (origin(s): command line)

So in this case, it seems to be a user-provided command-line argument.

@jerboaa
Copy link
Collaborator Author

jerboaa commented Sep 6, 2023

It seems a step backwards in terms of usability? First I need to generate a json, and then pass it to the native-image build using some other option? It would be nice if the driver could handle that for me.

@fniephaus
Copy link
Member

We don't have any API options to register elements for reflection, JNI, etc. Why would we have that for resources? You don't need to add any additional option if the resource-config.json is in a META-INF/native-image/packagename/ location on your classpath or module path.

@fniephaus
Copy link
Member

fniephaus commented Sep 6, 2023

It seems a step backwards in terms of usability?

You can spin this both ways: with the config file, there is only one way to do the thing. We also provide tooling for the config files (e.g., for merging).

@jerboaa
Copy link
Collaborator Author

jerboaa commented Sep 6, 2023

Fair enough. Could I convince you to make this apparent in the build output? A link to a doc should suffice. I.e. to safe the next person from going down the rabbit whole of trying to figure out what the replacement is supposed to be.

@fniephaus
Copy link
Member

Could I convince you to make this apparent in the build output?

You don't have to convince me. 😉 The build output already prints alternative API options when available, but we didn't have time to implement support for more complex migration suggestions. The easiest solution in this case would be to deprecate IncludeResources and add the migration hint as a deprecationMessage. I'm not 100% sure this is what we actually want to do. Any thoughts?

@christianwimmer
Copy link

Note that " exclude resources" needs to go away completely, because excluding resources in one native-image.properties file that another file included breaks composeability of libraries. But that is not really related to the option vs. the ability to exclude resources via the configuration files.

@fniephaus
Copy link
Member

I'm going to close this for now. #7370 tracks the request in #7354 (comment).

@fniephaus
Copy link
Member

This will be addressed by #7720.

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

No branches or pull requests

4 participants