[Neurodebian-devel] Debian packages for Stimfit

Mon Jan 31 15:45:15 UTC 2011

Hi Christoph,

Great to see stimfit packaging converging.  Here is my review (sorry, list came
up a bit lengthy; I hope I would not scary you).

On Mon, 31 Jan 2011, Christoph Schmidt-Hieber wrote:
> Fixed. As you suggested, both python-stfio and stimfit are now built from the same source. Uploaded as 0.10.7 to the launchpad ppa.

* where could I fetch your GPG key you used to sign .dsc?
I've failed to find it around (PPA has different one A9205C2F)

$> gpg --recv-key 9F7AC0CC
gpg: requesting key 9F7AC0CC from hkp server pgp.mit.edu
gpgkeys: key 9F7AC0CC not found on keyserver
gpg: no valid OpenPGP data found.
gpg: Total number processed: 0

$> gpg --recv-key  --keyserver http://keyserver.ubuntu.com:11371 9F7AC0CC
gpg: requesting key 9F7AC0CC from http server keyserver.ubuntu.com
gpgkeys: no key data found for http://keyserver.ubuntu.com:11371/
gpg: no valid OpenPGP data found.
gpg: Total number processed: 0

* I guess there is no need to ship  /usr/lib/python-stfio/ at all --
static library is of no use (or do you want to expose interface and make
library public?), and .so seems to be duplicated under dist-packages as well

$> dpkg -c python-stfio_0.10.7-0ubuntu2_amd64.deb | grep -e '\(so\|libs\)'
-rw-r--r-- root/root      1037 2011-01-31 06:13 ./usr/lib/python-stfio/libstfio.la
-rw-r--r-- root/root   6582434 2011-01-31 06:13 ./usr/lib/python-stfio/libstfio.a
-rw-r--r-- root/root   2730756 2011-01-31 06:13 ./usr/lib/python-stfio/libstfio.so
-rw-r--r-- root/root   2730756 2011-01-31 06:13 ./usr/lib/python2.6/dist-packages/stfio/_stfio.so

* as above it builds only for the default version of python.  Would you
  consider building for all supported?  it is desired but not mandated... just wanted 
  to know

* better not to ship .la files at all
  http://wiki.debian.org/ReleaseGoals/LAFileRemoval

* during build there is report of
dpkg-shlibdeps: warning: debian/stimfit/usr/lib/python2.6/dist-packages/stimfit/_stf.so contains an unresolvable reference to symbol _ZN9Recording9SetFitEndEi: it's probably a plugin.

is that indeed a plugged in via some custom loading of some dynamic library? I see it undefined:

$> objdump -t /usr/lib/python2.6/dist-packages/stimfit/_stf.so | grep _ZN9Recording9SetFitEndEi 
0000000000000000         *UND*  0000000000000000              _ZN9Recording9SetFitEndEi
$> objdump -t /usr/lib/stimfit/libstf.so | grep _ZN9Recording9SetFitEndEi  
0000000000000000         *UND*  0000000000000000              _ZN9Recording9SetFitEndEi

 and there is more of them:
 dpkg-shlibdeps: warning: 48 other similar warnings have been skipped (use -v to see them all).

* all lintian warnings should be addressed:

neurodebian at lego:/tmp/stimfit$ lintian stimfit_0.10.7-0ubuntu2_amd64.changes
E: stimfit: copyright-contains-dh_make-todo-boilerplate
W: stimfit: binary-without-manpage usr/bin/stimfit
W: stimfit: executable-not-elf-or-script ./usr/lib/python2.6/dist-packages/stimfit/charlie.py
W: stimfit: executable-not-elf-or-script ./usr/lib/python2.6/dist-packages/stimfit/natools.py
W: stimfit: executable-not-elf-or-script ./usr/lib/python2.6/dist-packages/stimfit/ivtools.py
W: stimfit: executable-not-elf-or-script ./usr/lib/python2.6/dist-packages/stimfit/minidemo.py
W: stimfit: executable-not-elf-or-script ./usr/lib/python2.6/dist-packages/stimfit/hdf5tools.py
W: stimfit: executable-not-elf-or-script ./usr/lib/python2.6/dist-packages/stimfit/mintools.py
E: stimfit: unstripped-binary-or-object ./usr/bin/stimfit
E: stimfit: unstripped-binary-or-object ./usr/lib/python2.6/dist-packages/stimfit/_stf.so
E: stimfit: unstripped-binary-or-object ./usr/lib/stimfit/libstf.so
E: python-stfio: copyright-contains-dh_make-todo-boilerplate
W: python-stfio: wrong-section-according-to-package-name python-stfio => python
W: python-stfio: executable-not-elf-or-script ./usr/lib/python2.6/dist-packages/stfio/hdf5tools.py
E: python-stfio: unstripped-binary-or-object ./usr/lib/python-stfio/libstfio.so
E: python-stfio: unstripped-binary-or-object ./usr/lib/python2.6/dist-packages/stfio/_stfio.so

  - stripping -- you've forgotten to add dh_strip
    you might like to create also stimfit-dbg package which would absorb all debug symbols, so you just
   call
   dh_strip --dbg-package=stimfit-dbg

   that would help people to get meaningful backtraces if they are to file a bugreport

   N.B. you can avoid running into 'missing calls to dh_' scripts by switching
   to use debhelper 7 format of debian/rules, where you specify only
   customizations

  - manpage... since you have some command line options -- it is better to provide such one

    you might like to use help2man tool which would allow you to use output of
    --version and --help to create a manpage for a binary.  We use it for our packages quite often:

    $> grep -l help2man */debian/rules 
    ants/debian/rules
    libgdf/debian/rules
    mipav/debian/rules
    mlpy/debian/rules
    neurodebian/debian/rules
    psychopy/debian/rules
    pynifti/debian/rules
    pyoptical/debian/rules

   N.B. most (if not all) of those repositories should be available from
   http://git.debian.org/  under pkg-exppsy/ (historical name for our project)

  - debian/copyright

    needs to become a bit more extended, with proper copyright/license
    header and entries -- now many things are missing (see below)

   Also I would recommend to adopt DEP5 format for that file:

   http://dep.debian.net/deps/dep5/

   many (if not all) of above packages of ours carry such copyright files

   also I see some files which might be problematic:

./src/core/filelib/axon/Common/axodebug.h
//    Copyright (c) 1993-1997 Axon Instruments.
//    All rights reserved.

and no license, I have not spotted it among ./src/core/filelib/axon files
-- so strictly speaking it is not allowed to be used or distributed...  where
it came from and what is the license? (should be listed in debian/copyright).
The same for any other file for which you are not the copyright holder/author
(besides common free ones part of build infrastructure like  ltmain.sh):

 ./src/core/levmar/* GPL (v2 or later) 
  [Copyright: 2004-05 Manolis Lourakis (lourakis at ics.forth.gr)]

 NB it seems that at least two more projects carry copy of it -- hugin and
freemat... may be eventually it could be made a separate package to reduce
maintenance effort

* isn't stimfit using stfio anyhow?  I do not see any reference but I thought
it would be logical... may be worth adding python-stfio to at least Suggests of
stimfit?

* is test.h5  which you use in ./src/stfswig/unittest_stfio.py  too big
  to be included at least into version control repository?

   ideally unittests should run during build (invoked from debian/rules) so
   functionality for the built package gets tested.  It is most of the time better
   to fail while building than then interact with angry users ;-)

  would you mind sharing some way some other samples files to test
  functionality of stimfit/python-stfio ?

* are there any other tests available besides the above?

* since stimfit ships python module, to be friendly to Debian python
  guys, might be useful to add 

* since stimfit is a gui application, it would be great to register it within
  the system by providing .desktop file to be shipped into
  /usr/share/applications so people could invoke it from the GNOME or KDE
  menu.

  NB ideally there should also be debian/stimfit.menu and call to
  dh_installmenu to register stimfit within Debian menu system which would
  provide support for environments which do not care about .desktop files

  here you could find some examples:

$> ls -ld */debian/*desktop 
-rw------- 1 yoh yoh  244 Oct  4 12:05 afni/debian/afni.desktop
-rw-r--r-- 1 yoh yoh  208 Jul 15  2009 caret/debian/caret.desktop
drwxr-sr-x 2 yoh yoh 4096 Jul  1  2010 fsl/debian/desktop/
-rw-r--r-- 1 yoh yoh  330 Jun 11  2009 fslview/debian/fslview.desktop
-rw-r--r-- 1 yoh yoh  300 Jan 14 13:05 psychopy/debian/psychopy.desktop
-rw------- 1 yoh yoh  222 Jul  2  2010 voxbo/debian/vbview2.desktop

$> ls -ld */debian/*menu       
-rw------- 1 yoh yoh  210 Oct  4 12:05 afni/debian/afni.menu
-rw-r--r-- 1 yoh yoh  221 Jul 15  2009 caret/debian/caret.menu
-rw------- 1 yoh yoh 3527 Jul  1  2010 fsl/debian/fsl.menu
-rw-r--r-- 1 yoh yoh  244 Jun 11  2009 fslview/debian/fslview.menu
-rw------- 1 yoh yoh  107 Jul  6  2010 kbibtex/debian/menu
-rw------- 1 yoh yoh  240 Oct  5 17:35 mipav/debian/mipav.menu
-rw------- 1 yoh yoh  330 Sep 16  2009 odin/debian/menu
-rw------- 1 yoh yoh  330 Sep 16  2009 odin/debian/odin.menu
-rw-r--r-- 1 yoh yoh  139 Jan 14 13:05 psychopy/debian/menu
-rw------- 1 yoh yoh  207 Jul  2  2010 voxbo/debian/voxbo.menu

or just look into /usr/share/menu/ and /usr/share/applications

-- 
=------------------------------------------------------------------=
Keep in touch                                     www.onerussian.com
Yaroslav Halchenko                 www.ohloh.net/accounts/yarikoptic