Generate dynamic access metadata and provide it to the classpath when passing "--emit build-report" as a build argument#795
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.
❌ 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.
❌ 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.
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.
❌ 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.
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.
❌ 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.
❌ 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.
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
|
Hey @melix, I've refactored the code to align with your requested changes, so please take another look when you have the time. |
| } | ||
|
|
||
| @Internal | ||
| public abstract Property<ResolvedComponentResult> getRuntimeClasspathGraph(); |
There was a problem hiding this comment.
I don't think these are really internal, right? Internal means used internally but without influencing the inputs. If the graph changes, I assume that this has to be taken into account?
d4b8af7 to
7d13364
Compare
|
I was thinking that maybe instead of complicated logic to figure out if |
…1.3 to 0.11.4 [skip ci] Bumps [org.graalvm.buildtools:native-maven-plugin](https://github.com/graalvm/native-build-tools) from 0.11.3 to 0.11.4. Release notes *Sourced from [org.graalvm.buildtools:native-maven-plugin's releases](https://github.com/graalvm/native-build-tools/releases).* > 0.11.4 > ------ > > What's Changed > -------------- > > * Release 0.11.3 by [`@graalvmbot`](https://github.com/graalvmbot) in [graalvm/native-build-tools#797](https://redirect.github.com/graalvm/native-build-tools/pull/797) > * Bump version to 0.11.4-SNAPSHOT by [`@graalvmbot`](https://github.com/graalvmbot) in [graalvm/native-build-tools#799](https://redirect.github.com/graalvm/native-build-tools/pull/799) > * Generate dynamic access metadata and provide it to the classpath when passing "--emit build-report" as a build argument by [`@jormundur00`](https://github.com/jormundur00) in [graalvm/native-build-tools#795](https://redirect.github.com/graalvm/native-build-tools/pull/795) > * Fix relativlization bug by [`@melix`](https://github.com/melix) in [graalvm/native-build-tools#792](https://redirect.github.com/graalvm/native-build-tools/pull/792) > * Fix wrong property name in the maven plugin documentation by [`@jormundur00`](https://github.com/jormundur00) in [graalvm/native-build-tools#808](https://redirect.github.com/graalvm/native-build-tools/pull/808) > * Enhance Project Documentation and Testing by [`@vjovanov`](https://github.com/vjovanov) in [graalvm/native-build-tools#791](https://redirect.github.com/graalvm/native-build-tools/pull/791) > * Support compatibility mode in GraalVM by [`@vjovanov`](https://github.com/vjovanov) in [graalvm/native-build-tools#811](https://redirect.github.com/graalvm/native-build-tools/pull/811) > * Update reachability metadata to 0.3.33 by [`@graalvmbot`](https://github.com/graalvmbot) in [graalvm/native-build-tools#814](https://redirect.github.com/graalvm/native-build-tools/pull/814) > * Release 0.11.4 by [`@graalvmbot`](https://github.com/graalvmbot) in [graalvm/native-build-tools#815](https://redirect.github.com/graalvm/native-build-tools/pull/815) > > New Contributors > ---------------- > > * [`@jormundur00`](https://github.com/jormundur00) made their first contribution in [graalvm/native-build-tools#795](https://redirect.github.com/graalvm/native-build-tools/pull/795) > > **Full Changelog**: <graalvm/native-build-tools@0.11.3...0.11.4> Commits * [`cfa42b8`](graalvm/native-build-tools@cfa42b8) Merge pull request [#815](https://redirect.github.com/graalvm/native-build-tools/issues/815) from graalvm/release/0.11.4 * [`0db3ae0`](graalvm/native-build-tools@0db3ae0) Release 0.11.4 * [`ea4d521`](graalvm/native-build-tools@ea4d521) Merge pull request [#814](https://redirect.github.com/graalvm/native-build-tools/issues/814) from graalvm/update-metadata-to-0.3.33 * [`c773ff7`](graalvm/native-build-tools@c773ff7) Update reachability metadata to 0.3.33 * [`06737b9`](graalvm/native-build-tools@06737b9) Merge pull request [#811](https://redirect.github.com/graalvm/native-build-tools/issues/811) from graalvm/vj/support-compatibility-mode-preserve * [`1ba5ebc`](graalvm/native-build-tools@1ba5ebc) Avoid reaching hosted elements with -H:Preserve=all * [`ffb728f`](graalvm/native-build-tools@ffb728f) Merge pull request [#791](https://redirect.github.com/graalvm/native-build-tools/issues/791) from graalvm/vj/project-documentation-and-testing * [`c6ff0a2`](graalvm/native-build-tools@c6ff0a2) Merge pull request [#808](https://redirect.github.com/graalvm/native-build-tools/issues/808) from jormundur00/jormundur00/[gh-807](https://redirect.github.com/graalvm/native-build-tools/issues/807) * [`aafd2e6`](graalvm/native-build-tools@aafd2e6) Fix wrong property name in the maven plugin documentation * [`ae4b8eb`](graalvm/native-build-tools@ae4b8eb) Fix relativlization bug ([#792](https://redirect.github.com/graalvm/native-build-tools/issues/792)) * Additional commits viewable in [compare view](graalvm/native-build-tools@0.11.3...0.11.4) [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
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.
Fixes: #801.