openSUSE:Factory review
The review by the openSUSE review team
We basically want to ensure that packaging follows the best practices captured already in Portal:Packaging and follow the high level guidelines the team has established.
General review
There are some process issues that are enforced:
- Does the package build on all supported architectures (x86 and x86_64 right now)? This is done via an automatic check.
- Does the package submission come from its devel project? This is done via an automatic check.
Changes file review
- Every submission has to have a changes file entry.
- For submissions that are updates, the changes file should refer to one or more bug reports or FATE entries.
- For submissions that are new versions, some sane reference to what is included/changed in the new version update. (See also: (Not) Referring to upstream logs)
- Changes must be in chronological order. (Automatic check)
- If changes are made to old entries, it should only be to fix things like bug numbers or typos. Old entries should not typically be removed or substantially changed.
- The changelog header format must be strictly observed, such as the format of the user, date, time, and '----' string.
- New changelog entries for a package should be consistent with previous entries.
- The addition of any given patch file needs to be mentioned in the .changes file. (Automatic check) Patch files should have documentation to them, e.g. in the form of [openSUSE:Packaging_Patches_guidelines in-patch descriptions] (Manual check).
- The removal of any given patch file needs to be mentioned in the .changes file (Automatic check), even if the removal is simply because upstream accepted the patch.
Specfile Review
- Check the usage of 'ExclusiveArch tags. In general, a package should build for all architectures, but if it does not make sense at all, this should be mentioned as comment in the spec file.
- Ensure that all the standard packaging best practices are followed openSUSE:Packaging_Patches_guidelines. For example:
- Reduce or eliminate the Warnings that are given about patches in the sources
- Verify license header
- Preamble has to have the basic set of tags, such as: name, version, release, URL, etc.
- Inspect version to ensure that it is sane and upgradeable
- Look for things that should not be in specfile anymore, such as:
- AutoReqProv: on, # norootforbuild, # usedforbuild, %clean section, etc.
- Look for things that should be replaced in specfiles, such as:
- %{?jobs:-j%jobs} → %{?_smp_mflags}
- %fdupes %buildroot → %fdupes %buildroot/%_prefix (otherwise one may end up with hardlinks across top-level dirs and partitions, causing installation failure)
- Look for proper OBSOLETES (see openSUSE:Package_dependencies)
- Ensure proper package naming, such as packaging of shared libs, avoiding colons, etc.
Accept or Reject
Should all violations immediately result in rejection of the package? Mostly "no." Some common sense needs to prevail, but sometimes, members of our team have different views of how violations should be handled. This needs to be refined and reconciled (with some time and patience).
Some ways to address violations:
- Decline submission. This may seem harsh, but usually just means "needs more work". If a package turns out to be a hopeless case (for example because of patents), we will convey to the submitter to not retry.
- Create your own branch, make the small change yourself, and resubmit/obsolete
- Send message to the submitter by way of the OBS comment system or by any other means. (Communication is always a good thing, and usually welcomed by the packager). Always explain why you have done what you have done, and what you expect to have done now. Ask the submitter to correct the violation.
Security review
New setuid binaries etc. need a review by the security team first. Add a security-team into the review queue, so they can check it. Atm it can be done via command-line only.
osc review add -G security-team ID
In general, security review has the highest priority, and we might err being to cautious.
Maintenance reviews
For maintenance packages, the review of patch-info files is an additional step to the normal review:
- Confirm that patchinfo data matches the changes in the sources and vice versa.
- Verify that the bug numbers and CVE references listed in the patchinfo match the bug numbers listed in the changes log, and vice versa. (Somewhat automatic, if the correct abbreviation is used).
- All changes must be behaviorally and ABI backward compatible. This means defaults must not change silently as part of the update, configs or major functionality of the package be splitted off onto other parts or similar. Shared library changes must be reviewed with their impact on the product without a full rebuild in mind.
- All submissions should have a consistent package naming (extended package names, aka foo.openSUSE_X.Y) and have source links pointing to openSUSE_X.Y:Update. This is necessary for release number tracking to work correctly, and required even for new packages.
- Check that at least one bugzilla reference is given. The bugzilla bug should give additional reason why the update is needed.
Hard rejections
These are the things for which we will always reject a submission
- Build failed
- Missing specfile, sources, or changes log
- No new changelog entry
- Things that rpmlint calls an "error". Sometimes also reject for warnings or a large amount of warnings.
- A changelog entry is horribly bad or missing required data
- We see suspicious stuff like suid being set or an obvious security violation. rpmlint usually finds these things, but we still watch out for them and reject them.
- If things like header files are in the wrong package, or other obvious violations of package policies. (There are many exceptions, but over time there are fewer exceptions.)
- Improper use of macros.
- A package that reverts to old problems that we've asked to have fixed in the past.
Best Effort
The openSUSE review team follows a "best effort approach" for general changes and might err - sometimes being too cautious, especially for security issues and sometimes take stuff in to get a build quickly out of the door.
Also, maintenance review might enforce stricter rules.
The team might also accept packages and create bug reports to cleanup packages later.
Review of delete requests
A delete request has to pass these minimal requirements:
- A valid reason is provided and was discussed beforehand on a public openSUSE mailinglist (not always the case)
- The package has no active maintainer
- Upstream development discontinued and the maintenance burden (patching security holes, bugs, etc.) isn't worth the effort
- It may be part of a package rename (i.e. there is a new submit request with the same package under a different name)
- The delete request should mention the other submit request number and both should be reviewed together
Guidelines by package type
Perl packages
Perl packages should be generated with the automated tool _cpanspec_. This ensures maximum compatibility and almost guarantees a successful review. You can ask *coolo* about this script.
http://en.opensuse.org/openSUSE:Packaging_Perl
Python packages
Ruby packages
Ruby packages should be generated with the automated tool _gem2rpm-opensuse_. This ensures maximum compatibility and almost guarantees a successful review. You can ask *darix* about this script.