Nov 11, 2011
For package to be included in default Fedora repositories, it has to
go through process called package review. If you've done a few package
reviews you know big chunks of this process are repeated ad-nausea in
every review.
There have been quite a few tools aimed at automating and
simplifying this process. However they all had one major flaw. They
have been designed for reviewing specific class of packages, be it
Perl, Python or generic C/C++ packages. Few us us decided to change
this.
We used Tim Lauridsen's FedoraReview
package as a base for our work and started adding new features and
tweaks. Current work has a website on
fedorahosted where you'll find all important information.
Full feature list would be quite long, but I'll list a few
major things:
- Bugzilla integration
- Mock integration
- JSON api for external plugins (more info further down)
- Several automated tests
The tool runs all checks/tests on spec file and rpms and writes
output into a text file. Snippet of the output looks like this:
Package Review ============== Key: - = N/A x = Check ! = Problem ? = Not evaluated ==== Generic ==== ... [ ]: MUST License field in the package spec file matches the actual license. [ ]: MUST License file installed when any subpackage combination is installed. [!]: MUST Package consistently uses macros (instead of hard-coded directory names). Using both %{buildroot} and $RPM_BUILD_ROOT ... [x]: SHOULD Spec use %global instead of %define. ... ==== Java ==== [!]: MUST If package uses "-Dmaven.local.depmap" explain why it was needed in a comment [!]: MUST If package uses "-Dmaven.test.skip=true" explain why it was needed in a comment Issues: [!]: MUST Package consistently uses macros (instead of hard-coded directory names). Using both %{buildroot} and $RPM_BUILD_ROOT [!]: MUST If package uses "-Dmaven.local.depmap" explain why it was needed in a comment [!]: MUST If package uses "-Dmaven.test.skip=true" explain why it was needed in a comment
We display only relevant results. In other words if there are no
post/postun scriptlets there is no reason to include sanity output
checking in the template. This will make more and more sense as we
add more checks.
JSON API
So how is it that different people will be able to write additional
plugins for this review tool? We provide a relatively simple JSON
api through stdin/stdout.
So to create a new check plugin you create a script or program in
your language of choice. There is only one requirement:
- Programming language has to have JSON format support
{ "supported_api": 1, "pkgname": "package name", "version": "package version", "release": "package release", "srpm":"path/to/srpm", "spec":{ path: "path/to/spec", "text": "spec text with expanded macros"}, "rpms":[ "path/to/rpm", ...], "rpmlint": "rpmlint output", "build_dir": "/path/to/src/directory/after/build" }When your plugin is done with checks it returns following message by writing it to stdout:
{ "command": "results", "supported_api": 1, "checks": [ { "name": "CheckName", "url": "URL to guidelines usually", "group": "Group for this test.(Java, Perl, etc.)", "text": "Check description that shows on review template", "deprecates":["DeprecatedTest", ...] "type": "MUST"|"SHOULD", "result": "pass"|"fail"|"inconclusive", "extra_output": "text", }, ... ] }If the plugin closes stdout without writing anything there, it means there were no relevant automated tests to run and no non-automated tests to include in template for manual evaluation. This is useful so we don't include for example Perl-related test output for Java packages and vice-versa.
Roadmap
While the tool is already usable and soon to be packaged in Fedora, there are still quite a few things we want to improve:- Add more functions to API (currently there is just get_section)
- Automate all automatable tests currently available
- Get rid of redundant tests (don't duplicate rpmlint)
- Add more tests of course!
- Maybe add templating support?