Reviewing
Differences between revisions 31 and 59 (spanning 28 versions)
|
Size: 15599
Comment:
|
Size: 115
Comment:
|
| Deletions are marked like this. | Additions are marked like this. |
| Line 1: | Line 1: |
| ||<tablestyle="float:right; font-size: 0.9em; width:40%; background:#F1F1ED; margin: 0 0 1em 1em;" style="padding:0.5em;">'''Contents'''[[BR]][[TableOfContents(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]. = 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] |
#DEPRECATED #REFRESH 2 http://wiki.ubuntu.com/UbuntuDevelopment/CodeReviews#Tips |
| Line 300: | Line 4: |
| ["CategoryMOTU"] | ["CategoryMOTURedirect"] |
["CategoryMOTURedirect"]
["CategoryMOTURedirect"]
MOTU/Packages/Reviewing (last edited 2008-08-06 16:32:37 by localhost)