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

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



Hi Cornelius,

I recommend reading
<https://www.debian.org/doc/manuals/maint-guide/index.en> if you
haven't. The guide mentions suggestions 1), 2) and 3) mentioned by
Daniel and more.

Cheers,
Alex

On 11/10/2015, Cornelius Kölbel <cornelius.koelbel@netknights.it>
wrote:
> 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
>>
>
>
>


Reply to: