Reviewing
Size: 13618
Comment:
|
Size: 18052
Comment:
|
Deletions are marked like this. | Additions are marked like this. |
Line 1: | Line 1: |
## page was renamed from MOTU/Packages/ReviewingGuide ## page was renamed from MOTU/ReviewingGuide ## page was renamed from MOTUReviewingGuide |
|
Line 85: | Line 88: |
and check the diff has no output. | and check the diff has no output (i.e. the folders are identical). |
Line 116: | Line 119: |
This diff should only contain files from the debian/ directory. | This diff should only contain files from the debian/ directory. If it is not the case, the packager has to make everything possible to make the diff as small as possible, (sometimes it's not possible to have only things in debian/, especially with automake transitions). |
Line 246: | Line 249: |
This file is aimed to replace the {{{export DH_COMPAT=4}}} line in debian/rules. If that file exist (which is a good thing), you should check that there is no such thing as {{{export DH_COMPAT=4}}} in debian/rules, that might cause a conflict. |
This file is aimed at replacing the {{{DH_COMPAT}}} option in debian/rules. If that file exists, you should check that there is no such thing as {{{export DH_COMPAT=4}}} in debian/rules, that might cause a conflict. |
Line 257: | Line 260: |
Description lines are limited to 80 characters, and should have a space as first character. | ==== Build-Depends ==== attachment:IconsPage/IconWarning3.png Using the auto-update CDBS trick for Build-Depends is '''highly evil''' and a cause of immediate reject on the Debian NEW queue. This should be checked in debian/rules aswell. Check that the debhelper version in this field fits with the level in debian/compat. ==== Build-Depends-Indep ==== This field is not mandatory. attachment:IconsPage/IconWarning3.png There has been discussions on whether it should be used or not. Using it for binary dependencies is prohibited. See [http://lists.debian.org/debian-devel/2003/12/msg01762.html this thread] about this subject. ==== Architecture ==== Check that the architecture is good. A program that needs to be compiled in the build is usually {{{any}}}, unless it doesn't build on some platforms. A set of scripts (python, perl, bash, php, etc.) or artwork is usually {{{all}}}. This architecture will generate a lintian output if Build-Depends-Indep is not used. However, given the issues generated by Build-Depends-Indep, this output can be safely ignored most of the time. Exceptions to these rules are to be discussed if unclear. ==== Description ==== Description lines are limited to 80 characters, and should have a space as first character. Although it is required to have lines smaller than 80 characters, lines of less than 70 characters make the Description field very long, which is not convenient for users either, so it is good to make sure lines are close to 80 characters without overriding this limit. |
Line 261: | Line 288: |
The description is the way the users will know about the package and decide wether they will install it or not. It must be aimed at the target users for the package. For a guide to good Description fields, see [http://people.debian.org/~walters/descriptions.html]. == GUI specificities == === desktop files === In Ubuntu, we want to follow the [http://freedesktop.org Free Desktop] Policy as much as possible. In particular, we want to check how the desktop files look, and relate it to the [http://standards.freedesktop.org/desktop-entry-spec/latest/ desktop-entry specification]. It is recommended to use GenericName entries in desktop files. If there is no desktop file in a program, it should be added in debian/, installed by the package, and sent back to upstream. attachment:IconsPage/IconWarning3.png As usual, we don't want to modify the source. If the desktop file has to be modified, the packager should use patches. === po & pot files in Kubuntu === In Kubuntu packages, tarballs providing a po/ folder should be patched following these instructions : [:KubuntuPackagingGuide#head-11219c7a7bbbccadc452aaf496f0df95e41b8fba], so the program can be added to [Rosetta] for translation. == REVU reviewing == [:REVU] includes special tools to help reviewing packages. It focuses on NEW packages to be added to the development version of Ubuntu. [:MOTU]s can ask for an ssh access to tiber.tauware.de to launch some commands that help reviewing packages. The main command available on tiber is : {{{ pbuilder-dapper update # update the dapper pbuilder revu-build <file.dsc> }}} This will pbuild the package in Dapper and generate a set of reports that can be viewed on the REVU page for the package. attachment:IconsPage/IconTip.png Tiber stores uploads in /var/revu/revu1-incoming/$package-$date/ folders. All the files present in these folders are shown in the list on the package page on REVU. If you have an ssh account on tiber, you can generate additional reports that will be viewable by other reviewers and packagers on REVU. I have modified revu-build and created a set of scripts to be used on REVU, that can be found [http://revu.tauware.de/~raphink/revu-tools here] on in [http://revu.tauware.de/~raphink/revu-tools_0.1.tar.gz this tarball]. |
|
Line 269: | Line 335: |
* [http://people.debian.org/~mpalmer/sponsorship_checklist.html Matthew Palmer's advice on sponsorship] * [:/Tips] : Reviewing tips, some of which are not merged here yet = Authors = * Initial author : [:RaphaelPinson] ---- ["CategoryMOTU"] CategoryDocumentation |
ContentsBRTableOfContents(3) |
Introduction: Who is this for?
This guide is aimed at both reviewers/sponsors and packagers in Ubuntu and other Debian systems. To the former I hope it can provide a rather strong document to base their reviews on. To the latter it is a base to check their own packages before submitting them.
attachment:IconsPage/IconWarning3.png This guide is not a policy in any way, but rather a proposal for efficient and complete reviews. The complete policy regarding Debian packages can be found [http://www.debian.org/doc/debian-policy/ here].
Prerequisites
It seems impossible to imagine reviewing a work without being able to do it. Therefore, knowing Debian packaging is a condition to understanding this guide. Please refer to the [http://www.debian.org/doc/manuals/maint-guide/index.en.html Debian New Maintainer's Guide] for an introduction to packaging techniques.
Reviewing Debian packages
Pre-existence of the package
It is the duty of packagers to check if the program they are about to package has not already been packaged by someone else. However, the reviewer should also check it.
Is the package in Ubuntu?
The first thing to check is whether the package is or has been in Ubuntu. Search for the presence of the package in any distribution on [http://packages.ubuntu.com].
If the package is in the Ubuntu archives:
- Is it a package in main/restricted or in universe/multiverse?
if it is in the current development version, is it an update? Are we in [https://wiki.ubuntu.com/UpstreamVersionFreeze UVF]?
if it is not in the current development version, has it been nuked? Why? Refer to the bug tracking system on [http://launchpad.net/malone Malone] to find out why.
If the package is not in the Ubuntu archives
check if it is on [http://revu.tauware.de REVU].
Is the package in Debian?
If the package is not in Ubuntu, we should see if it is in Debian. Search for the presence of the package in any distribution on [http://packages.debian.org].
If the package is in the Debian archives:
if it is in the current development version, should it be merged in Ubuntu? Are we in [https://wiki.ubuntu.com/UpstreamVersionFreeze UVF]?
if it is not in the current development version, has it been nuked? Why? Refer to the [http://bugs.debian.org Debian BTS] to find out why.
If the package is not in the Debian archives:
check if it is in the [http://ftp-master.debian.org/new.html NEW queue].
check if it is on [http://sponsors.debian.net/ Debian Sponsors].
Are there unofficial versions of the package?
Check other sources for the presence of the package.
The [http://apt-get.org AptGetOrg] website is a useful interface gathering lots of unofficial repositories for Debian packages.
- Check the upstream website for a Debian package with sources.
The tarball
The general rule for the tarball is to not modify the upstream tarball.
Is it the latest version?
Check on the upstream website that the current version is the latest version available. If it is not, is the packager aware of it and if so why did he/she choose to package this version?
tar.gz tarballs
If it is a tar.gz (or tgz) tarball, then the tarball should be simply renamed into the orig.tar.gz. Compare the md5 sums of the upstream tarball and the orig.tar.gz one and check that they are identical. If they are not, there must be a good reason, fully explained in debian/changelog (e.g. the removal of the CVS/ dir).
non-tar.gz tarballs
If it is not a tar.gz tarball, then obviously it will have to be modified to make the orig.tar.gz tarball. You should still check for the indentity of the orig.tar.gz with the upstream tarball.
For example, if the upstream tarball is a tar.bz2, you can use the following (assuming app-version.tar.bz2 is the upstream tarball and app_version.orig.tar.gz is the orig.tar.gz):
tar -xvf app-version.tar.bz2 app-version-upstream/ tar -xvf app_version.orig.tar.gz app-version/ diff -ru app-version-upstream app-version
and check the diff has no output (i.e. the folders are identical).
attachment:IconsPage/IconHint2.png The easiest way to make a proper orig.tar.gz from a tar.bz2 tarball is:
bunzip2 app-version.tar.bz2 gzip -9 app-version.tar mv app-version.tar.gz app_version.orig.tar.gz
When is it ok to change the tarball?
For more infos on this topic, see [:CommonPackagingMistakes/ChangingTheOrigTarball]
Build test
Pbuilder test
Before checking anything else in the package, it can be useful to know if the package actually builds in the latest unstable environment.
Using [https://wiki.ubuntu.com/PbuilderHowto Pbuilder] seems the best way to test it.
Debuild test
Once we know the package builds in a pbuilder, it can be good to check the diff generated when building the package.
You can run debuild && debuild -S and check for files in the diff, for example by grepping for 'orig' in the diff file.
This diff should only contain files from the debian/ directory. If it is not the case, the packager has to make everything possible to make the diff as small as possible, (sometimes it's not possible to have only things in debian/, especially with automake transitions).
attachment:IconsPage/IconHint2.png Since this has to be run in a recent unstable environment and requires to install dependencies you might not want to keep, using a [https://wiki.ubuntu.com/DebootstrapChroot chroot] is a good idea to achieve this task.
License
Licenses are a big issue on Debian systems. The policy regarding licenses is very strict, for good reasons.
Determining the license of the program
attachment:IconsPage/IconTip.png According to the FSF policy, the license of the program should be mentioned in each source file of the code. The COPYING file at the root of the upstream tarball has no value if the source files don't refer to it. In the case of a GPL license, the source files should contain the following layout as headers (see http://www.gnu.org/copyleft/gpl.html#SEC4 for more details) :
one line to give the program's name and an idea of what it does. Copyright (C) yyyy name of author This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 2 of the License, or (at your option) any later version. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with this program; if not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
If the license is not clear enough, the package should be refused and the packager told to ping upstream about this issue.
Checking if the license is GPL-compatible
Debian systems are open-source software. We care particularly about the fact that software is GPL-compatible.
If the license used for the package is not an FSF one (see [http://www.gnu.org/licenses/licenses.html] for a list), check it's a GPL-compatible one ([http://www.gnu.org/philosophy/license-list.html]).
Diving into debian/
After checking all these major points, let us dive into the debian/ directory.
debian/copyright
Once you know what license is used by upstream, check that debian/copyright refers to it properly. In the case of FSF licenses, the 3 paragraphs of the short version of the license should be quoted, followed by a Debian-specific paragraph.
For example, for a program licensed under GPL v.2 or later :
This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 2 of the License, or (at your option) any later version. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with this package; if not, write to the Free Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, N: MA 02110-1301, USA. On Debian GNU/Linux systems, the complete text of the GNU General Public License can be found in `/usr/share/common-licenses/GPL'.
The debian/copyright file should also contain the copyright for the program and, if necessary, for the manuals or other things added by the packager. This copyright must be of the following form : Copyright (C) yyyy name of author
attachment:IconsPage/IconNote.png Note that /usr/share/common-licenses/GPL always links to the latest version of the GPL on Debian systems. Therefore it should be used for programs licensed under GPL v.x or later. Programs licensed under GPL v.x should link specifically to /usr/share/common-licenses/GPL-x !
debian/changelog
debian/changelog is easy to review, as it's often properly generated by dh_make. However, there are some important points to be checked.
Package version
The package version should follow the upstream versioning most of the time. However, it is important to point out that in some cases, the package should change the version number, for upgrading purposes, linked to the way dpkg compares versions.
For example, 0.2.3-beta1 is newer than 0.2.3 for dpkg, but not for upstream! To fix this issue, our example can be numbered 0.2.2.99+0.2.3-beta1, which will be considered older than 0.2.3.
attachment:IconsPage/IconHint2.png It is useful to use dpkg --compare-versions to check how dpkg handles different versions of a package, e.g. :
dpkg --compare-versions 0.2.3-beta1 lt 0.2.3 && echo 'y' || echo 'n'
will return n, so we're sure it's not right.
Also check the use of -X or -XubuntuY in the package version:
- -X are for packages either synced from Debian or Ubuntu-specific
- -XubuntuY are for either packages either merged from Debian, upgraded from a Debian sync, or new in Ubuntu. In this last case, it should be numbered -0ubuntu1.
attachment:IconsPage/IconNote.png dh_make usually numbers the package with -1 by default for a first release, so check that closely.
attachment:IconsPage/IconHint2.png Packages on ["REVU"] are uploaded several times and changed. It is tempting for the packager to use dch and increase the version number above -0ubuntu1. It can be useful to number the package as -0ubuntu0.X in this case and have X updated in each upload, then switching to -0ubuntu1 once the package is ready to go.
Distribution
dh_make sets the distribution to 'unstable' by default in debian/changelog. While this is fine for Debian packages, packages in Ubuntu should be packaged for the latest unstable distribution, currently 'dapper'.
Commentaries
Commentaries in the changelog must be short yet informative. They must contain all the necessary informations so that a packager can take the package and understand what they are about later on.
When patches are used in the package, they should be quoted in the changelog, one entry per patch. Quoting them namely allows to grep the changelog for them.
If a change in the package closes a bug somewhere, it must be referred to, with e.g. (Closes : Malone #XXXX).
Lines length is limited to 80 characters in the commentaries. This is not policy, but it's quite logical, so that the file can be read easily in a 80-columns-wide console.
If the tarball was repackaged for some reason (tar.bz2 tarball, CVS/ dir, etc.), it is a good idea to mention it in the changelog.
Multiple entries in new packages
A new package (i.e. not in Ubuntu yet) should have only one changelog entry, mentioning the initial release. As long as a package has not hit the official repositories, it must not mention any changes made to the package, that actually don't exist in the distribution. This is an important point for ["REVU"] packages.
debian/compat
This file is aimed at replacing the DH_COMPAT option in debian/rules. If that file exists, you should check that there is no such thing as export DH_COMPAT=4 in debian/rules, that might cause a conflict. This file must contain only one number, as an indicator of the debhelper compat level of the package.
The current recommended debhelper compat level is 5.
Check in debian/control that the debhelper version in Build-Depends corresponds to the chosen compat level.
debian/control
Build-Depends
attachment:IconsPage/IconWarning3.png Using the auto-update CDBS trick for Build-Depends is highly evil and a cause of immediate reject on the Debian NEW queue. This should be checked in debian/rules aswell.
Check that the debhelper version in this field fits with the level in debian/compat.
Build-Depends-Indep
This field is not mandatory.
attachment:IconsPage/IconWarning3.png There has been discussions on whether it should be used or not. Using it for binary dependencies is prohibited. See [http://lists.debian.org/debian-devel/2003/12/msg01762.html this thread] about this subject.
Architecture
Check that the architecture is good. A program that needs to be compiled in the build is usually any, unless it doesn't build on some platforms. A set of scripts (python, perl, bash, php, etc.) or artwork is usually all. This architecture will generate a lintian output if Build-Depends-Indep is not used. However, given the issues generated by Build-Depends-Indep, this output can be safely ignored most of the time. Exceptions to these rules are to be discussed if unclear.
Description
Description lines are limited to 80 characters, and should have a space as first character. Although it is required to have lines smaller than 80 characters, lines of less than 70 characters make the Description field very long, which is not convenient for users either, so it is good to make sure lines are close to 80 characters without overriding this limit.
If the description contains indented lists, they must be indented with a space.
The description is the way the users will know about the package and decide wether they will install it or not. It must be aimed at the target users for the package.
For a guide to good Description fields, see [http://people.debian.org/~walters/descriptions.html].
GUI specificities
desktop files
In Ubuntu, we want to follow the [http://freedesktop.org Free Desktop] Policy as much as possible. In particular, we want to check how the desktop files look, and relate it to the [http://standards.freedesktop.org/desktop-entry-spec/latest/ desktop-entry specification]. It is recommended to use GenericName entries in desktop files.
If there is no desktop file in a program, it should be added in debian/, installed by the package, and sent back to upstream.
attachment:IconsPage/IconWarning3.png As usual, we don't want to modify the source. If the desktop file has to be modified, the packager should use patches.
po & pot files in Kubuntu
In Kubuntu packages, tarballs providing a po/ folder should be patched following these instructions : [:KubuntuPackagingGuide#head-11219c7a7bbbccadc452aaf496f0df95e41b8fba], so the program can be added to [Rosetta] for translation.
REVU reviewing
[:REVU] includes special tools to help reviewing packages. It focuses on NEW packages to be added to the development version of Ubuntu.
[:MOTU]s can ask for an ssh access to tiber.tauware.de to launch some commands that help reviewing packages.
The main command available on tiber is :
pbuilder-dapper update # update the dapper pbuilder revu-build <file.dsc>
This will pbuild the package in Dapper and generate a set of reports that can be viewed on the REVU page for the package.
attachment:IconsPage/IconTip.png Tiber stores uploads in /var/revu/revu1-incoming/$package-$date/ folders. All the files present in these folders are shown in the list on the package page on REVU. If you have an ssh account on tiber, you can generate additional reports that will be viewable by other reviewers and packagers on REVU.
I have modified revu-build and created a set of scripts to be used on REVU, that can be found [http://revu.tauware.de/~raphink/revu-tools here] on in [http://revu.tauware.de/~raphink/revu-tools_0.1.tar.gz this tarball].
Useful links
[http://www.debian.org/doc/manuals/maint-guide/index.en.html Debian New Maintainer's Guide]
[http://www.debian.org/doc/debian-policy/ Debian Policy]
[http://ftp-master.debian.org/REJECT-FAQ.html Reject FAQ on the Debian ftp-master website]
[http://people.debian.org/~mpalmer/sponsorship_checklist.html Matthew Palmer's advice on sponsorship]
- [:/Tips] : Reviewing tips, some of which are not merged here yet
Authors
Initial author : [:RaphaelPinson]
["CategoryMOTU"] CategoryDocumentation
MOTU/Packages/Reviewing (last edited 2008-08-06 16:32:37 by localhost)