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

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



Hi Daniel,

thanks a lot for all the valuable feedback.

I will dive into it later.

Do you have any further recommendations on finding a sponser (after
streamlining the package)?

THanks a lot and kind regards
Cornelius

Am Sonntag, den 11.10.2015, 00:18 +0200 schrieb Daniel Stender:
> 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
> 


Attachment: signature.asc
Description: This is a digitally signed message part


Reply to: