[Bug 476346] Review Request: python-polybori - Framework for Boolean Rings

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=476346





--- Comment #13 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp>  2009-03-24 14:37:22 EDT ---
Some notes:

* %define -> %global
  - Current packaging guidelines suggest to use %global instead
    of %define, especially when defining nested macros, see:
    https://fedoraproject.org/wiki/Packaging/Python#System_Architecture
    https://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define
    (The latter one is still under "Drafts" category, but this is already
    accepted by FESCo)

* %debug_package
-----------------------------------------------------------------
# and apparently
# also stops checking other libs once it hits the first static lib
-----------------------------------------------------------------
  - The problem is not there. The real issue is that the installed
    shared libraries are already stripped:
-----------------------------------------------------------------
Symlinking from libgroebner-0.5.0.so to
/builddir/build/BUILDROOT/python-polybori-0.5-3.fc11.1.x86_64/usr/lib64/libgroebner.so
g++ -o polybori/libpolybori-0.5.0.so.0.0.0 -s -shared
-Wl,-soname,libpolybori-0.5.0.so.0 ....
-----------------------------------------------------------------
    note that "-s" option is passed to g++.
    Perhaps modifying "LDFLAGS_LINUX" in polybori/Makefile.in will
    solve this issue.

* License
  - Seems GPLv2+. Would you check this?

* Requires
  - Would you check the dependency "i.e. Requires" for -devel subpackage?
     ! For example, polybori/groebner/groebner_alg.h contains:
-----------------------------------------------------------------
    12  #include <polybori.h>
    13  #include "groebner_defs.h"
    14  #include "pairs.h"
    15  #include <boost/dynamic_bitset.hpp>
-----------------------------------------------------------------
      This means -devel subpackage should have "Requires: boost-devel"
      (here I am not saying about BuildRequires).
-----------------------------------------------------------------
# rpm -ql python-polybori-devel | xargs grep -h 'include ' | sort | uniq
-----------------------------------------------------------------
      will show some useful information.
    ! And this output shows that polybori/CDDManager.h contains:
-----------------------------------------------------------------
   102  #define CDDManager_h_
   103  #include "cacheopts.h"
   104  // load basic definitions
   105  #include "pbori_defs.h"
-----------------------------------------------------------------
      Would you check if cacheopts.h can be really removed?

  - Also please check the dependency for main package.
    ! For example, polybori/nf.py contains:
-----------------------------------------------------------------
    81      global mat_counter
    82      mat_counter=mat_counter+1
    83      import Image, ImageDraw
-----------------------------------------------------------------
      This may mean that python-polybori should have "Requires:
python-imaging".

-----------------------------------------------------------------
# rpm -ql python-polybori | grep -v /usr/share/doc | LANG=C xargs grep -h
'import ' | grep -v Binary | sort | uniq
-----------------------------------------------------------------
      will show some useful information.

    ! Also
-----------------------------------------------------------------
$ LANG=C ipbori
/usr/bin/ipbori: line 66: ipython: command not found
-----------------------------------------------------------------
      Perhaps "Requires: ipython" is needed.

* SourceURL
  - Please follow:
    https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net

* Linkage on installed system libraries
------------------------------------------------------------------
# rpmlint python-polybori
python-polybori.i586: W: undefined-non-weak-symbol
/usr/lib/libpolybori-0.5.0.so.0.0.0 Cudd_zddUnion
python-polybori.i586: W: undefined-non-weak-symbol
/usr/lib/libpolybori-0.5.0.so.0.0.0 _Z12defaultErrorSs
python-polybori.i586: W: undefined-non-weak-symbol
/usr/lib/libpolybori-0.5.0.so.0.0.0 Cudd_ReadZddSize
python-polybori.i586: W: undefined-non-weak-symbol
/usr/lib/libpolybori-0.5.0.so.0.0.0 cuddCacheInsert1
.....
.....
python-polybori.i586: W: undefined-non-weak-symbol
/usr/lib/libgroebner-0.5.0.so.0.0.0 Cudd_zddUnion
python-polybori.i586: W: undefined-non-weak-symbol
/usr/lib/libgroebner-0.5.0.so.0.0.0 _Z12defaultErrorSs
python-polybori.i586: W: undefined-non-weak-symbol
/usr/lib/libgroebner-0.5.0.so.0.0.0 Cudd_ReadZddSize
.....
.....
------------------------------------------------------------------
  - These two libraries undefined non-weak symbols. 
    This cannot be allowed when these libraries also have in
    -devel subpackage named "libfoo.so" used for linkage against 
    these libraries, because these symbols will cause linkage error.

* Documents
  - I prefer to include README as %doc of main package for this case.
  - ChangeLog seems useful for -devel subpackage.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.