On Sun, Aug 5, 2012 at 3:30 AM, Jakub Wilk <jw...@debian.org> wrote:
>
> * Jason Gerard DeRose <jder...@novacut.com>, 2012-08-04, 18:26:
>>
>> dget -x
>> http://mentors.debian.net/debian/pool/main/p/pyskein/pyskein_0.7.1-1.dsc
Thanks for taking the time to look this over and give me such detailed
feedback! Notes on my corrections follow:
> Let me see:
>
>> Build-Depends: debhelper (>= 8.9),
>
>
> Out of curiosity, why >= 8.9?
When I started working on this, 8.9 was the version in Debian testing.
But as I don't know if the current package actually works with 8.9, I
changed this to >= 9.20120608, the current version in Debian testing.
Is that correct?
>
>> python3 (>= 3.2),
>> python3-dev (>= 3.2),
>
>
> You don't need to build-depend on both, python3-dev would be enough.
> Shouldn't
> that be python3-all-dev though?
Fixed, now Build-Depends on python3-all-dev (>= 3.2.3)
> Also, why 3.2? Upstream README says “you need Python 3.0 or later”.
I did this because Debian testing doesn't contain Python 3.1 or 3.0,
plus I personally have only extensively tested this package under 3.2.
>> X-Python-Version: 3.2
>
>
> This is wrong, X-P-V is for 2.X versions only.
This was a hack to prevent setup.py from getting called with Python2.
Seems the (hopefully) correct fix was to override_dh_auto_clean.
Is my override_dh_auto_clean hack acceptable? From a few examples I
found, this seems to be how people are currently dealing with
Python3-only packages.
>> X-Python3-Version: 3.2
>
>
> Shouldn't that be ">= 3.2"?
Fixed, now I have X-Python3-Version: >= 3.2
>> Depends: python3 (>= 3.2),
>
>
> Don't hardcode the dependency, use ${python3:Depends} instead.
Fixed.
>> Description: PySkein implementation of Skein cryptographic hash algorithm
>
>
> Maybe s/PySkein/Python/?
Agreed, fixed.
>> algorithm, one of the finalists in the NIST SHA-3 Competition. While
>> originally based on the optimized version of the reference implementation
>> by
>> Doug Whiting, PySkein is now feature complete and offers a pythonic
>> interface,
>
>
> Is the fact it was based on $something in the past really important enough
> to put it in the package description?
Agreed, I removed this sentence.
>> all released as free software under the GNU General Public License. Its
>> highlights are:
>
>
> If it wasn't free software, it wouldn't be allowed in Debian. No need to
> mention that in the description.
I removed this sentence also.
>> Simple interface following the hash algorithms in the Python standard
>> library
>> (like hashlib.sha1 or hashlib.sha256)
>> .
>> All features of the Skein specification (flexible digest sizes, MAC
>> generation, tree hashing, and various other arguments)
>> .
>> High performance through optimized C implementation (7.1 cycles/byte for
>> sequential hashing and 4.2 cycles/byte for tree hashing on two cores,
>> measured
>> on an Athlon 64 X2)
>> .
>> Threefish, the tweakable block cipher used in Skein, available for
>> encryption
>> and decryption on its own
>
>
> This looks like an itemized lists, except that it doesn't have bullets.
> Looks odd to me.
I didn't realize you could do bullet points in the long description.
It now uses bullet points and I think it looks much nicer.
> You may want to get your descriptions reviewed by debian-l10n-english@ldo.
Okay, thank you.
> The copyright file doesn't document license/copyright status of
> doc/_static/jquery.js. It also doesn't say say where the upstream sources
> were obtained; see Policy §12.5.
I updated the copyright file to use the machine-readable copyright-format/1.0.
It now includes a Source and a Files section for doc/_static/jquery.js
> debian/python3-skein.install is empty. Remove it.
Done.
>> Abstract: Documentation for PySkein in HTML form.
>
>
> One of the features of doc-base is that you can have the same document in
> multiple formats. Mentioning the format in the Abstract field seems weird to
> me.
Fixed
>> for pyvers in $(shell py3versions -vr); do \
>
>
> Missing “set -e”; see Policy §4.6.
Fixed. I updated this using an example in the Python/AppStyleGuide.
>> LC_ALL=en_US.UTF-8 python$$pyvers setup.py install \
>> --install-layout=deb \
>> --root $(CURDIR)/debian/python3-skein; \
>
>
> IIRC “LC_ALL=en_US.UTF-8” is too work around issue 9561. This bug was
> fixed in Python 3.2.3 RC 1, so you might want to just bump version in
> Build-Depends and drop this work-around. Or you may want to use the C.UTF-8
> locale, which is provided by libc-bin (>= 2.13-1).
Fixed. I now Depend on python3-all-dev (>= 3.2.3) and I removed the
hacks I used to work around issue 9561.
>> dh_auto_build
>
>
> Uh, this looks wrong. dh_auto_* doesn't support building Python 3 modules.
> In fact, I see this in the build log:
Fixed, I removed this and am now only building the sphinx docs in
override_dh_auto_build.
> | Can't exec "pyversions": No such file or directory at
> /usr/share/perl5/Debian/Debhelper/Buildsystem/python_distutils.pm line 120.
> | Use of uninitialized value $python_default in substitution (s///) at
> /usr/share/perl5/Debian/Debhelper/Buildsystem/python_distutils.pm line 121.
> | Use of uninitialized value $python_default in substitution (s///) at
> /usr/share/perl5/Debian/Debhelper/Buildsystem/python_distutils.pm line 122.
>
> The package will FTBFS once #683551 is fixed…
>
> Lintian reports:
>
> W: python3-skein-doc: embedded-javascript-library
> usr/share/doc/python3-skein-doc/html/_static/jquery.js
> W: python3-skein-doc: embedded-javascript-library
> usr/share/doc/python3-skein-doc/html/_static/underscore.js
>
> You may want to use dh_sphinxdoc to fix these.
Fixed. Now I'm using --with=sphinxdoc and python3-skein-doc Depends on
${sphinxdoc:Depends}.
> Lintian also emits:
>
> W: python3-skein: hardening-no-fortify-functions
> usr/lib/python3/dist-packages/_skein.cpython-32mu.so
Apologies, but I don't know where to start on this.
How do I enable this? Does this require a patch to the upstream source code?
Any suggestions?
> blhc confirms it's a true positive:
>
> CPPFLAGS missing (-D_FORTIFY_SOURCE=2): gcc -pthread -DNDEBUG -g -fwrapv
> -O2 -Wall -Wstrict-prototypes -g -fstack-protector --param=ssp-buffer-size=4
> -Wformat -Werror=format-security -fPIC -Isrc -I/usr/include/python3.2mu -c
> src/threefish.c -o build/temp.linux-i686-3.2/src/threefish.o
> CPPFLAGS missing (-D_FORTIFY_SOURCE=2): gcc -pthread -DNDEBUG -g -fwrapv
> -O2 -Wall -Wstrict-prototypes -g -fstack-protector --param=ssp-buffer-size=4
> -Wformat -Werror=format-security -fPIC -Isrc -I/usr/include/python3.2mu -c
> src/_skeinmodule.c -o build/temp.linux-i686-3.2/src/_skeinmodule.o
>
> Last but not least, lintian also emits:
>
> E: python3-skein: python-script-but-no-python-dep usr/bin/skeinsum
>
> This is because /usr/bin/skeinsum has #!/usr/bin/python3.2 shebang, but
> the package depends on python3, which of course doesn't guarantee that
> /usr/bin/python3.2 exists. You probably want to make this shebang
> unversioned.
What's the best way to make the shebang unversioned? I believe calling
setup.py with `python3` rather than `python3.2` would do it, but that
means not looping through `py3versions -r`.
Any suggestions?
> --
> Jakub Wilk
Thanks again for your time!
>
> --
> To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org
> with a subject of "unsubscribe". Trouble? Contact
> listmas...@lists.debian.org
> Archive: http://lists.debian.org/20120805093030.ga1...@jwilk.net
>
--
To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive:
http://lists.debian.org/caczosvhmvwox7pk9+w3qunwqvxccypz8draqxjqynedu2...@mail.gmail.com
