-
Notifications
You must be signed in to change notification settings - Fork 214
fix alpm jsonschema #623
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: main
Are you sure you want to change the base?
fix alpm jsonschema #623
Conversation
Signed-off-by: Prabhu Subramanian <[email protected]>
Signed-off-by: Prabhu Subramanian <[email protected]>
pombredanne
left a comment
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.
Thanks. See some comments. This is not schema valid for now.
| }, | ||
| "name_definition": { | ||
| "note": "The name is the package name. It is not case sensitive and must be lowercased.", | ||
| "requirement": "required", |
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 a property defined in the schema for name, because a purl without a name is mostly harmless in all cases, so if you run make conf && make check you will get:
$ make check
-> Validate JSON schemas
ok -- validation done
The following files were checked:
schemas/purl-test.schema.json
schemas/purl-type-definition.schema.json
schemas/purl-types-index.schema.json
-> Validate JSON data files against the schemas
ok -- validation done
The following files were checked:
purl-types-index.json
Schema validation errors were encountered.
types/alpm-definition.json::$.qualifiers_definition[0]: Additional properties are not allowed ('examples' was unexpected)
types/alpm-definition.json::$.qualifiers_definition[1]: Additional properties are not allowed ('examples' was unexpected)
types/alpm-definition.json::$.qualifiers_definition[2]: Additional properties are not allowed ('examples' was unexpected)
make: *** [Makefile:53: checkjson] Error 1
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.
Actually that part was schema-valid
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.
If name is not mandatory we risk the proliferation of non actionable purls for given types. So some strictness is better. I consciously avoided regex, min lengths and other properties that was suggested by Gemini so had some filter but completely forgot to validate. Will fix the validation errors and push later this evening.
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 schema has been updated. The name is always required now https://github.com/package-url/purl-spec/blob/f0ee1f6ab00ea980c4965a34d7d00bd7e8fb2cb0/schemas/purl-type-definition.schema-1.1.json so this change is fine.
|
Edited and pushed changes from my phone! Ran validation with codespaces. |
|
All frontier models think that an alpm purl with a subpath is not valid! We are reinforcing the incorrect learning by only using tests with subpath: null. https://github.com/package-url/purl-spec/blob/main/tests/types/alpm-test.json |
pombredanne
left a comment
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.
Some bits for your review.
| { | ||
| "$schema": "https://packageurl.org/schemas/purl-type-definition.schema-1.0.json", | ||
| "$id": "https://packageurl.org/types/github-definition.json", | ||
| "$id": "https://packageurl.org/types/alpm-definition.json", |
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 valid and important change
| }, | ||
| "name_definition": { | ||
| "note": "The name is the package name. It is not case sensitive and must be lowercased.", | ||
| "requirement": "required", |
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 schema has been updated. The name is always required now https://github.com/package-url/purl-spec/blob/f0ee1f6ab00ea980c4965a34d7d00bd7e8fb2cb0/schemas/purl-type-definition.schema-1.1.json so this change is fine.
| ] | ||
| }, | ||
| "version_definition": { | ||
| "requirement": "required", |
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.
Versions are never required, so this change must be removed.
| "description": "The distribution name when using multiple distributions." | ||
| }, | ||
| { | ||
| "key": "repository_url", |
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 a standard qualifier so it not really needed here IMHO, but it does not hurt either... @johnmhoran what do you think?
First fix PR out of several.
Methodology
To arrive at the fixes, I trained up and utilised three separate LLMs. (Fixes are simply not possible with traditional linting and jsonschema validation tools.)
Human was involved in the loop constantly steering and restarting the conversation threads to reduce mistakes. The result is captured in this repo.
Once the enhanced version of PURL was created, Gemini-2.5-Pro was trained with this enhanced version and asked to fix the current version, strongly adhering to the current JSON schema specification.
This
methodologysection is fyi and will not be included in subsequent PRs.Fixes for alpm
requirement=requireddistroandrepository_urlqualifiers. The note clearly recommends these two qualifiers but they weren't included in the definition section.NOTE: creating purl4ml and each such PR is a time consuming activity. It will take a while for me to send all PRs due to my travel.