-
Notifications
You must be signed in to change notification settings - Fork 80
Generate dynamic access metadata and provide it to the classpath when passing "--emit build-report" as a build argument #795
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
base: master
Are you sure you want to change the base?
Conversation
…mit build-report"
…mpile-no-fork while emitting a Build Report
…adle plugins add the generated file to the classpath instead of a new option
… the maven and gradle plugins
|
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
| }); | ||
| } | ||
|
|
||
| public Path getRepositoryDirectory() { |
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.
❌ Should probably return Optional<Path> instead, so that we don't have to figure out if null is expected or not.
| private static final String PROVIDES_FOR = "providesFor"; | ||
|
|
||
| @Internal | ||
| public abstract Property<Configuration> getRuntimeClasspath(); |
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.
❌ This is not compatible with Gradle's configuration cache. We shouldn't use Configuration as input (probably a Property<ResolvedComponentResult>).
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've since moved to the usage of Property<ResolvedComponentResult> and SetProperty<ResolvedArtifactResult> (trying to follow the pattern you used when setting properties for the CollectReachabilityMetadata task to be safe).
| Set<File> classpathEntries = runtimeClasspathConfig.getFiles(); | ||
|
|
||
| Map<String, Set<String>> exportMap = buildExportMap( | ||
| runtimeClasspathConfig.getResolvedConfiguration().getFirstLevelModuleDependencies(), |
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.
❌ You should use a dependency graph as input instead. This isn't compatible with the configuration cache.
| * entry in the {@value #LIBRARY_AND_FRAMEWORK_LIST} file. | ||
| */ | ||
| private Set<String> readArtifacts(File inputFile) throws IOException { | ||
| Set<String> artifacts = new HashSet<>(); |
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.
Should probably use a LinkedHashSet so that the result is consistent across invocations.
| tasks, | ||
| deriveTaskName(binaryName, "generate", "DynamicAccessMetadata")); | ||
| imageBuilder.configure(buildImageTask -> { | ||
| if (buildImageTask.getOptions().get().getBuildArgs().get().stream() |
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.
❌ This is likely to fail, because you are reading the build arguments during configuration of a task, which is non deterministic (depends on when the build args are changed in a script). Either the reasoning has to be deferred to execution time, based on the actual arguments, or it should use a non eager transformation (not use .get() but .map).
native-gradle-plugin/build.gradle
Outdated
| } | ||
|
|
||
| dependencies { | ||
| implementation libs.openjson |
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.
❌ The Gradle plugin shouldn't have a direct dependency on openjson. The common utils should have one, but there's no reason the Gradle plugin should directly depend on it.
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've created a separate DynamicAccessMetadataUtils class where I do this serialization and JSON parsing (which I left in the commons.utils package), so now I don't add this direct dependency on openjson here anymore.
FYI: Not sure if the same applies for the native-maven-plugin, but in the build.gradle.kts for it, we already depend on both libs.utils and libs.openjson.
…tResult> instead of relying on Property<Configuration>
…class, that is used by both the Maven and Gradle plugin
In this PR we add the Maven mojo and Gradle task for generating and providing to the native image build classpath a
dynamic-access-metadata.jsonfile, when emitting a Build Report (when the--emit build-reportbuild argument is present) and the used release of thegraalvm-reachability-metadatarepository contains thelibrary-and-framework-list.jsonfile.The generated JSON file maps all of the artifacts present on the classpath that exist in the
library-and-framework-list.jsonto the list of their transitive dependencies (as they are marked as "metadata provided-for" by the parent artifact). It is later used by the dynamic access tab of the native image Build Report to mark these "provided-for" dependencies as being covered by native image and not "needing investigation". The JSON file follows the schema at: https://github.com/oracle/graal/blob/master/docs/reference-manual/native-image/assets/dynamic-access-metadata-schema-v1.0.0.json.Both the Maven mojo and the Gradle task can be ran standalone, but will automatically run when emitting a Build Report and running the native image build mojo/task.