Add check if correct schemas are present in the reachability metadata repository provided to buildtools#821
Conversation
… repo provided to buildtools
0bba771 to
7101367
Compare
7101367 to
08f31ec
Compare
|
|
||
| private FileSystemRepository newRepositoryFromDirectory(Path path, LogLevel logLevel) { | ||
| if (Files.isDirectory(path)) { | ||
| SchemaValidationUtils.validateSchemas(path); |
There was a problem hiding this comment.
Do we have GraalVM home at this point?
There was a problem hiding this comment.
This happens pretty early on and we don't have the knowledge of GraalVM home here.
| "Note: Since the repository is enabled by default, you can disable it manually in your pom.xml file (see this: " + | ||
| "https://graalvm.github.io/native-build-tools/latest/maven-plugin.html#_configuring_the_metadata_repository)"); | ||
| } else { | ||
| SchemaValidationUtils.validateSchemas(repoPath); |
There was a problem hiding this comment.
Why don't we have a single place where we validate?
There was a problem hiding this comment.
I moved the validation to inside of the FileSystemRepository constructor, so we can now do it in one place for both of the plugins. This required me to add a dependency from the common/graalvm-reachability-metadata project to the common/utils project, but that seems like a reasonable addition to me, as I see no reason to not have a dependency on Utils.
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "name": "library-and-framework-list-schema-v1.0.0", | |||
| "note": "placeholder for tests" | |||
There was a problem hiding this comment.
Since we now require schemas, we have to also add them to all the "custom" repositories that the tests use. As not to have to always copy the exact schema content (as I don't see a reason to), I just left a placeholder text.
To avoid even more confusion, I changed the content of these schemas to just being [].
| // Validate required files in the schemas directory (accepting minimal versions or newer) | ||
| List<String> missing = new ArrayList<>(); | ||
| String prefix = repoRoot.relativize(schemasDir).toString(); | ||
| for (RequiredSchema required : REQUIRED_SCHEMAS) { |
There was a problem hiding this comment.
What happens if we add a new schema in the metadata repo? We should match the two sets I believe and if there is an extra version, or the version in the repo is bigger you should update the build tools.
There was a problem hiding this comment.
Counting the number of schemas provided by the metadata repo seems reasonable to me (since changing the number of packaged schemas SHOULD require buildtools changes), so I added that.
I don't think we should necessarily fail if the schema version provided by the metadata repo is higher than the one in build tools: adding some field in an index.json which is not used by buildtools (for example, the field denoting metadata has been generated by AI, which we plan to add) should not necessitate buildtools updates (as it has no effect on them). If we were to go the EXACT match route, we would have to update buildtools every time any small change happens to any of the schemas, which I don't think is maintainable. A solution I came up with was to just keep the minimal version in buildtools, and fail if the metadata provided schemas are lower than the minimal required. Buildtools releases generally come with hardcoded metadata repo versions, so users using an old buildtools release with new metadata versions should do that at their own peril IMO.
There was a problem hiding this comment.
then we need to make the changes to the schemas forward compatible right so we need to have a way to tell to the build tools you need to update. Are we going to be backwards compatible in the repo?
There was a problem hiding this comment.
Per our offline discussion, I changed it so we now check for EXACT matches in the major version of the schemas. If either the metadata repo or the buildtools-requires have a higher major version, we now fail the builds and leave the appropriate update suggestion in the error message (so now we have a message for every different scenario of version mismatches).
| this.rootDirectory = rootDirectory; | ||
| } | ||
| SchemaValidationUtils.validateSchemas(rootDirectory); | ||
| this.moduleIndex = new FileSystemModuleToConfigDirectoryIndex(rootDirectory); |
| @@ -0,0 +1 @@ | |||
| {} No newline at end of file | |||
There was a problem hiding this comment.
Why even have a file, especially a file with a missing end-of-line?
There was a problem hiding this comment.
These /resources/repos directories are "mock" metadata repositories used in testing. As we now require the metadata repositories to have certain schemas packaged with them (and we check this), we need to add this "mock" schema directory inside of them.
The missing end-of-line got mixed in somehow with AI generation, I've added them now.
43ff9bd to
e557605
Compare
| /** | ||
| * Represents a required schema by base name with an exact required major version. | ||
| */ | ||
| private record RequiredSchema(String baseName, int requiredMajor) { |
…1.4 to 0.11.5 [skip ci] Bumps [org.graalvm.buildtools:native-maven-plugin](https://github.com/graalvm/native-build-tools) from 0.11.4 to 0.11.5. Release notes *Sourced from [org.graalvm.buildtools:native-maven-plugin's releases](https://github.com/graalvm/native-build-tools/releases).* > 0.11.5 > ------ > > What's Changed > -------------- > > * Bump version to 0.11.5-SNAPSHOT by [`@graalvmbot`](https://github.com/graalvmbot) in [graalvm/native-build-tools#817](https://redirect.github.com/graalvm/native-build-tools/pull/817) > * Make environment variables set in the pom.xml of the native-maven-plugin be visible in the image builder by [`@jormundur00`](https://github.com/jormundur00) in [graalvm/native-build-tools#819](https://redirect.github.com/graalvm/native-build-tools/pull/819) > * Add check if correct schemas are present in the reachability metadata repository provided to buildtools by [`@jormundur00`](https://github.com/jormundur00) in [graalvm/native-build-tools#821](https://redirect.github.com/graalvm/native-build-tools/pull/821) > * Add fallback for jarless artifacts in the native-maven-plugin by [`@jormundur00`](https://github.com/jormundur00) in [graalvm/native-build-tools#824](https://redirect.github.com/graalvm/native-build-tools/pull/824) > * Fix JUnit 6 not working correctly with JDK 21 by expanding the initialize-at-build-time-list by [`@jormundur00`](https://github.com/jormundur00) in [graalvm/native-build-tools#832](https://redirect.github.com/graalvm/native-build-tools/pull/832) > * Remove the usage of the global metadata/index.json from the nbt plugins by [`@jormundur00`](https://github.com/jormundur00) in [graalvm/native-build-tools#829](https://redirect.github.com/graalvm/native-build-tools/pull/829) > * Revert "Remove the usage of the global metadata/index.json from the nbt plugins" by [`@jormundur00`](https://github.com/jormundur00) in [graalvm/native-build-tools#836](https://redirect.github.com/graalvm/native-build-tools/pull/836) > * Use JDK 21 Graal in the CI by [`@jormundur00`](https://github.com/jormundur00) in [graalvm/native-build-tools#839](https://redirect.github.com/graalvm/native-build-tools/pull/839) > * Support `-H:+CompatibilityMode` in build tools by [`@vjovanov`](https://github.com/vjovanov) in [graalvm/native-build-tools#822](https://redirect.github.com/graalvm/native-build-tools/pull/822) > * Update reachability metadata to 0.3.34 by [`@graalvmbot`](https://github.com/graalvmbot) in [graalvm/native-build-tools#842](https://redirect.github.com/graalvm/native-build-tools/pull/842) > > **Full Changelog**: <graalvm/native-build-tools@0.11.4...0.11.5> Commits * [`ffd094d`](graalvm/native-build-tools@ffd094d) Release 0.11.5 * [`1d7c2f7`](graalvm/native-build-tools@1d7c2f7) Merge pull request [#842](https://redirect.github.com/graalvm/native-build-tools/issues/842) from graalvm/update-metadata-to-0.3.34 * [`ba1c2e8`](graalvm/native-build-tools@ba1c2e8) Update reachability metadata to 0.3.34 * [`790fa05`](graalvm/native-build-tools@790fa05) Merge pull request [#822](https://redirect.github.com/graalvm/native-build-tools/issues/822) from graalvm/vj/compatibility-mode * [`0631241`](graalvm/native-build-tools@0631241) Implement Compatibility Mode detection * [`1844654`](graalvm/native-build-tools@1844654) Use JDK 21 Graal in the CI ([#839](https://redirect.github.com/graalvm/native-build-tools/issues/839)) * [`6315677`](graalvm/native-build-tools@6315677) Revert "Remove the usage of the global metadata/index.json from the nbt plugi... * [`fe065ce`](graalvm/native-build-tools@fe065ce) Remove the usage of the global metadata/index.json from the nbt plugins ([#829](https://redirect.github.com/graalvm/native-build-tools/issues/829)) * [`94b5b54`](graalvm/native-build-tools@94b5b54) Fix JUnit 6 not working correctly with JDK 21 by expanding the initialize-at-... * [`086cfdf`](graalvm/native-build-tools@086cfdf) Add fallback for jarless artifacts in the native-maven-plugin ([#824](https://redirect.github.com/graalvm/native-build-tools/issues/824)) * Additional commits viewable in [compare view](graalvm/native-build-tools@0.11.4...0.11.5) [](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 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 introduce validation that correct schemas (that are of a certain major version) are present in the reachability metadata repository provided to the buildtools. With this check, we can warn the user if the format of the metadata repository they are using is outdated and will not work with the current buildtools.
As the changes within the schemas might not require changes in the buildtools repo, we only check if the major version matches, as those version changes represent changes that are not compatible with previous buildtools releases (e.g., adding a new field to a schema that isn't used by buildtools doesn't require buildtools changes).
This PR is a prerequisite for changing the
index.jsonparsing done by the buildtools, as not having this check could lead to projects who use older metadata snapshots breaking without knowing why.Fixes: #820