Reviewing

Differences between revisions 24 and 25
⇤ ← Revision 24 as of 2006-01-25 23:02:50 →
Size: 14164
Editor: c529d3992
Comment:
← Revision 25 as of 2006-01-25 23:16:24 → ⇥
Size: 14985
Editor: c529d3992
Comment:
Deletions are marked like this. Additions are marked like this.
Line 257: Line 257:
==== the Description field ==== ==== Build-Depends ====


==== Build-Depends-Indep ====

This field is not mandatory. 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 ====
Line 266: Line 282:

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:

If the package is not in the Ubuntu archives

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 the package is not in the Debian archives:

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.

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.

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

Build-Depends-Indep

This field is not mandatory. 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

MOTU/Packages/Reviewing (last edited 2008-08-06 16:32:37 by localhost)