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

Bug#683871: RFS: pyskein/0.7.1-1 [ITP] -- Skein hash for Python3



On Wed, Aug 8, 2012 at 11:55 AM, Jakub Wilk <jwilk@debian.org> wrote:
> * Jason Gerard DeRose <jderose@novacut.com>, 2012-08-08, 06:40:
>
>>>> 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?
>
>
> Well, using that logic, it should be "debhelper (= 9.20120608)" because you
> don't know if it'll continue to work with the future versions... ;]
>
> Build-dependencies (and Python version declarations) should be as loose as
> possible, to ease backporting.

Okay, that makes sense. And now I *do* have a specific reason for the
>= version, see below :)

> Looking at debhelper changelog, the last change that affects your package
> appears to be:
>
> | debhelper (8.9.5) unstable; urgency=low
> |
> |   * dh_compress: Don't compress _sources documentation subdirectory
> |     as used by python-sphinx. Closes: #637492
>
>
>>> 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.
>
>
> Which reminds me: upstream provides a test suite. Please run it at build
> time.

I've contacted the upstream author for a bit of guidance on the tests.

And in the meantime, I'm doing my best to properly integrate them into
the build (work in progress).

>> 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.
>
>
> Firstly, it should be: setup.py clean -a (the -a part is quite important).
>
> Additionally, the current code won't work that well once we have more than
> one supported Python 3 version. You need to either have a for loop there, or
> rm -rf the whole build/ directory.

Based on what you said, `rm -rf build/` seems safer and simpler. I'm
also properly cleaning the doc build now, which I wasn't before. How
is this?

override_dh_auto_clean:
	rm -rf build/ doc_src/_build/

> If we are it, the package FTBFS if built twice in a row (not only because of
> missing -a):
> |  dpkg-source -b pyskein-0.7.1
> | dpkg-source: info: using source format `3.0 (quilt)'
> | dpkg-source: info: building pyskein using existing
> ./pyskein_0.7.1.orig.tar.gz
> | dpkg-source: warning: file
> pyskein-0.7.1/doc_src/_build/html/searchindex.js has no final newline
> (either original or modified version)
> | dpkg-source: error: cannot represent change to
> doc_src/_build/html/objects.inv: binary file contents changed
> | dpkg-source: error: add doc_src/_build/html/objects.inv in
> debian/source/include-binaries if you want to store the modified binary in
> the debian tarball
> | dpkg-source: warning: file pyskein-0.7.1/doc_src/_build/html/download.html
> has no final newline (either original or modified version)
> | dpkg-source: warning: file pyskein-0.7.1/doc_src/_build/html/genindex.html
> has no final newline (either original or modified version)
> | dpkg-source: warning: file pyskein-0.7.1/doc_src/_build/html/skein.html
> has no final newline (either original or modified version)
> | dpkg-source: warning: file pyskein-0.7.1/doc_src/_build/html/index.html
> has no final newline (either original or modified version)
> | dpkg-source: warning: file pyskein-0.7.1/doc_src/_build/html/scripts.html
> has no final newline (either original or modified version)
> | dpkg-source: warning: file pyskein-0.7.1/doc_src/_build/html/search.html
> has no final newline (either original or modified version)
> | dpkg-source: warning: file
> pyskein-0.7.1/doc_src/_build/html/threefish.html has no final newline
> (either original or modified version)
> | dpkg-source: warning: file pyskein-0.7.1/doc_src/_build/html/random.html
> has no final newline (either original or modified version)
> | dpkg-source: warning: file pyskein-0.7.1/doc_src/_build/html/stream.html
> has no final newline (either original or modified version)
> [snip - more similar warnings and errors]
> | dpkg-source: warning: executable mode 0775 of 'build/scripts-3.2/skeinsum'
> will not be represented in diff
> | dpkg-source: error: cannot represent change to
> build/lib.linux-i686-3.2/_skein.cpython-32mu.so: binary file contents
> changed
> | dpkg-source: error: add build/lib.linux-i686-3.2/_skein.cpython-32mu.so in
> debian/source/include-binaries if you want to store the modified binary in
> the debian tarball
> | dpkg-source: error: unrepresentable changes to source
> | dpkg-buildpackage: error: dpkg-source -b pyskein-0.7.1 gave error exit
> status 2
>
> Please fix the clean target. :)
>
>
>> Fixed, I removed this and am now only building the sphinx docs in
>> override_dh_auto_build.
>
>
> But that means the C code is built in the binary(-arch) target, rather than
> in build(-arch). The former should be normally used for stuff that requires
> (fake)root privileges.

Okay, I fixed this. I'm calling `setup.py build` here and am using
--executable=/usr/bin/python3 to fix the shebang. How is this?

override_dh_auto_build:
	sphinx-build -b html doc doc/_build/html
	set -ex; for python in $(shell py3versions -r); do \
		$$python setup.py build \
				--executable=/usr/bin/python3; \
	done

>>> 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?
>
>
> That shouldn't be necessary. distutils honours CFLAGS/CPPFLAGS/LDFLAGS set
> in environment. See also: https://wiki.debian.org/Hardening#dpkg-buildflags

Okay, after reading those docs, I got the impression that the most
modern/correct way to do this is probably to Build-Depend on debhelper
(>= 9.20120417) and use compat=9, so that's what I did.

In the build logs I can see that gcc is indeed getting called with
-D_FORTIFY_SOURCE=2.

What do you think of using compat=9 here? Is that the best approach?

>>> 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`.
>
>
> TIMTOWTDI!

As noted above, I choose the --executable=/usr/bin/python3 route. The
shebang is now:

#!/usr/bin/python3

Thank you again for all the time you've spent looking this over! I've
learned a lot, and I'll do my best to pass those lessons on :)

Aside from running the tests during the build, is there anything else
you spot that needs work?

> The way that dh_auto_build does it (for 2.* modules) is indeed to use
> /usr/bin/python interpreter instead of /usr/bin/python2.X when 2.X happens
> to be the default version. But it's very easy to get it wrong (see bugs
> #547510, #589759). Nobody sane would implement such logic directly in
> debian/rules, ever. :)
>
> The traditional solution is to fix the shebang by sed.
>
> The modern solution is to pass --executable=/usr/bin/python to the build
> command.[0]
>
> And finally, there's a “oooh, I'll use this new shiny thing” method:
> dh_python2 has now a --shebang option.
>
>
> [0] Documentation:
> http://docs.python.org/dev/distutils/setupscript#installing-scripts
>
> --
> Jakub Wilk
>
>
>
> --
> To UNSUBSCRIBE, email to debian-mentors-REQUEST@lists.debian.org
> with a subject of "unsubscribe". Trouble? Contact
> listmaster@lists.debian.org
> Archive: [🔎] 20120808175515.GA3933@jwilk.net">http://lists.debian.org/[🔎] 20120808175515.GA3933@jwilk.net
>


Reply to: