[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#801213: RFS: python-privacyidea/2.7-1 [ITP]



-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Hi Cornelius,

I can't sponsor an upload of your work, but here are the results of my review
of your package I was doing for my DD application process (I am CCing this email
to the application manager and the archives for this purpose).

Here are some collected suggestions for improvement,

1) debian/changelog: the target distribution can't be "jessie". Jessie is the current
stable release, and new packages are not going to be added to that. If there's no
special reason to want to get uploaded into experimental you are heading for unstable,
codename "sid". But "unstable" is the suite resp. target name, which is going to be
used here [a].

   [a] https://www.debian.org/doc/manuals/developers-reference/ch05.en.html#distribution

2) debian/changelog: you have release information in the changelog entry of your
package, but debian/changelog is reserved for changes of the Debian version of
the package [a]. For there are no changes on the package yet, "Initial release"
is all what's happening with this package.

   [a] http://www.debian.org/doc/debian-policy/ch-source.html#s-dpkgchangelog

3) debian/changelog: the ITP bug usually gets closed by the initial upload through
a "Closes:" in the deb/changelog, which should be added to the "Initial release"
line [a].

   [a] https://lintian.debian.org/tags/new-package-should-close-itp-bug.html

4) debian/watch: uscan scan fails because the watch line is missing the actual
Perl regex pattern matching the tarball. But direct scanning of Pypi for upstream
tarballs is deprecated, anyway [a]. The Python team maintains a great Pypi
redirector, which allows easy installation of a proper watch file [b]:
$ curl -o debian/watch http://pypi.debian.net/privacyidea/watch

   [a] https://lintian.debian.org/tags/debian-watch-file-unsupported-pypi-url.html

   [b] https://wiki.debian.org/Python/LibraryStyleGuide#debian.2Fwatch

5) debian/control: better is to use the current debhelper (>= 9~), that also needs
a bump of the compat level to "9" in debian/compat [a].

   [a] https://lintian.debian.org/tags/package-uses-old-debhelper-compat-version.html

6) debian/control: the "X-Python-Version" element doesn't belong in the binary package
section, but above into the source package section [a]

   [a] https://www.debian.org/doc/packaging-manuals/python-policy/ch-module_packages.html#s-specifying_versions

7) debian/control: though not mandatory nor recommended by the policy [a], if the
"Homepage:" field is present in the source package section, it's easy to browse to
it from the package tracker page.

   [a] https://www.debian.org/doc/debian-policy/ch-controlfields.html

8) debian/control: the build-dependency against python-setuptools is redundant, and
the versioning against >= 0.6b3 is obsolete, even oldstable has 0.6.24.

9) debian/rules: since the tarball ships a testsuite, the tests should be run during
build time (for the package has a lot of dependencies, it would be great if also a
DEP-8 compatible test setup for Debian CI could be installed, even if you are upstream
[a,b]).

   [a] http://anonscm.debian.org/gitweb/?p=autopkgtest/autopkgtest.git;a=blob_plain;f=doc/README.package-tests.rst;hb=HEAD

   [b] http://meetings-archive.debian.net/pub/debian-meetings/2014/debconf14/webm/debci_and_the_Debian_Continuous_Integration_project.webm

10) debian/control: there are some errors in the package descriptions like that
"python" isn't capitalized, "authenticaion" etc. (please recheck and see the related
Lintian complaints). The description text looks like it's just a copy of the program's
documentation, and copyright statements should not be included here [a].
   
   [a] https://www.debian.org/doc/debian-policy/ch-binary.html#s-descriptions

11) debian/control: you've got Breaks and Replaces against a package "privacyidea",
but which isn't in the archive. I would guess that's a non-official package around
somewhere? I'm not sure if a use like that could be discussed, but another use (maybe
you mean the upstream version) would be unwanted, definitely [a].

   [a] https://www.debian.org/doc/debian-policy/ch-relationships.html

12) debian/rules: "python_distutils" is usually put by py2dsc/stdeb as buildsystem
but you might want to use "pybuild" (that's commonly used for new packages). That
also needs a build-dep on "dh-python" in debian/control (that's recommended anyway
when you run the dh sequencer "--with python2", see the complaint in the build log).

13) in python-privacyidea and privacyidea-pam, you have a five executable scripts in
/usr/bin without manpages. There is a "should have" in the policy on this issue [a],
but missing manpages are considered being a bug. And if missing manpages adds to a couple
of other issues like that (like sub standard descriptions) the chance of being rejected
by the FTP team rises [b].

   [a] https://www.debian.org/doc/debian-policy/ch-docs.html#s12.1

   [b] https://ftp-master.debian.org/REJECT-FAQ.html (see "Manpages")

14) debian/copyright: this files is the complete license text of the AGPL-3.0, but
although this is also being marked as optional in the policy [a], it would be better
if it would follow the (machine parse-able) DEP-5 copyright file format [b] instead.

   [a] https://www.debian.org/doc/debian-policy/ch-docs.html#s-copyrightformat

   [b] http://dep.debian.net/deps/dep5/

15) you have some third-party files in the source package which follow a
different license, like privacyidea/static/contrib/js/jquery-1.11.3.js (MIT). All
licenses and copyright holders must be listed in debian/copyright (you have to make
it DEP-5). Misses are a common reason for new packages to get rejected by the
FTP masters [a].

   [a] https://ftp-master.debian.org/REJECT-FAQ.html (see "License III").

16) debian/*.copyright: the individual copyright files for different binaries should
be dropped in favour of a DEP-5 debian/copyright, the same for all the binaries. I've
rechecked, the policy says: "a copy of the file which will be installed in
/usr/share/doc/package/copyright should be in debian/copyright in the source
package" [a].

   [a] https://www.debian.org/doc/debian-policy/ch-docs.html#s-copyrightfile

17) debian/control: the "python-" prefix for the main binary "python-privacyidea" could
be dropped, that's mend for packages with public modules used by other packages [a]
(there might be different opinions on if you should keep it like it is). But the build-dep
on python-all should be replaced with "python", for that package doesn't need
to get build against any/all supported Python versions, but only the default one.

   [a] https://www.debian.org/doc/packaging-manuals/python-policy/ch-module_packages.html#s-package_names

18) maybe you would like to build the Sphinx documentation in doc/ into a
privacyidea-docs package. As far as I see it doesn't make much sense to run this application
offline, but it would be more comfortable to have the documentation that way, instead of
only on the web.

19) maintscripts: you have two symlinks in debian/, privacyidea-apache2.postinst and
privacyidea-nginx.postinst which are dead (it's "../deploy/debian-ubtuntu/" instead of
"../debian-ubuntu/"). They should be replaced by the actual scripts. Policy 6.1:
"... they must be proper executeable files" [a] (and not symlinks). The others aren't
executable here, either.

   [a] https://www.debian.org/doc/debian-policy/ch-maintainerscripts.html

20) debian/privacyidea-radius.postinst: there's a Lintian complaint of running
the "service" command for a restart-after-upgrade, which isn't wanted for maintainer
scripts.

21) the manpage ./usr/share/man/man1/privacyidea-server.1.gz has a Lintian complaint
on "manpage-has-errors from-man" [a], and furthermore there is the problem that this
manpage is in section "1", which is reserved for binaries resp. executables, but this
is a general documentation. That's pre generated out of the Sphinx docs, maybe you
would like to put this out in plain text and put it into /usr/share/docs/, if not going
for 18), instead (and manpages aren't handled by debian/install, either).

   [a] https://lintian.debian.org/tags/manpage-has-errors-from-man.html

22) you've got important Lintian complaints on privacy breaches (fetching data from
external websites on runtime) in some of the Javascript libraries in
privacyidea/static/contrib/js/ (angular.js and ui-bootstrap-tpls-0.13.0.js),
this must be patched out.

23) There are some more Lintian complaints on things copied into the binaries, which aren't
wanted (image-file-in-usr-lib, extra-license-file), please make your package Lintian clean!

24) there are some more issues on the source tarball (contains .pyc bytecode files, prebuild
Javascript objects), and maybe you would like to remove docs/sphinx_rtd_theme.zip from the
source package (and use the package python-sphinx-rtd-theme). You might want to use "Files-Excluded:"
in debian/copyright [a] to created a reduced Debian tarball via uscan.

   [a] https://wiki.debian.org/UscanEnhancements

I'm sure that privacyIDEA would be a great addition to the Debian archive (I'm going to watch
the talk on Datenspuren 2014 [a] after I've sended this), and I would really like to see that
package pass the sponsoring-requests stage and that you'll find a sponsor.

    [a] https://www.youtube.com/watch?v=fvKPQMpvYnw

Best,
Daniel Stender

- -- 
4096R/DF5182C8
46CB 1CA8 9EA3 B743 7676 1DB9 15E0 9AF4 DF51 82C8
LPI certified Linux admin (LPI000329859 64mz6f7kt4)
http://www.danielstender.com/blog/

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCgAGBQJWGY7DAAoJEBXgmvTfUYLIt2wP/ihnOnD/vUv975wyHH7rKWSu
F+o3/RcCymWr3bLXbksLtOXDhKto+oYdEqifSdNI02Hf58YCK21AjTNdQGb2GOg1
gLMgYKCBJVob2+4iU/iaeuoVm+2VKp47WhABe7yBxerjJOHqqXZgX+2jdE6JRpBT
aada78UP1ljlQTQ6PQqu47Ni8c8Btyj6eTE6341RMzXWxRDdufT6F+/PDnP2Z0al
3hFRd0JYON9voThDSMW0zs851a+yyQF4ek+323zH8f6qFfiAl5XfjrdUhjuTwnkU
/bxHxHHSC2aoRn5d/ylrGHEGsL5aKHs8VlYVYP3WrCCALcD4bxKqaXVdGeo/DirF
mLl7zolZRwor0ZYTu086wcBsiEgr6XRjZv6b8kMnDrH/zBpcFnnHUIIKA27UcBFd
EGsYmc1k5GVXooXD1NQawdNw6A+GTA3RM+mcQPncoomcFgtx67Qjt+zCDHwi7dgy
mzi8ZTsb8wmWAi5d/ZPNvAwY/t+7yxk1qJnECZT1pwE7MdNO8j3ySqJKPVzP6z8E
sUE4QhnCOupwH4t21kDkGj/9sKqNisj1DKs9TlkqfolQE4wWNhmTSbHBIaPAGjMI
9b4MUFr3XZS8INdFtMPwNWs8WZvh9pPopucqGh2LVqvBpVcGuoT5wC0knFibXdUf
r5f9Ngs2DPv8Iv94gTMT
=HGKw
-----END PGP SIGNATURE-----


Reply to: