[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 #14 from Conrad Meyer <konrad at tylerc.org>  2009-03-24 16:07:29 EDT ---
(In reply to comment #13)
> * %define -> %global

Done.

> * %debug_package
>     Perhaps modifying "LDFLAGS_LINUX" in polybori/Makefile.in will
>     solve this issue.

This doesn't fix it --- I also removed -s as a link flag from SConstruct, and
now it builds without stripping at link time.

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

Ah. The only source I could find (sourceforge.net) just said GPL, so I assumed
GPLv3. I guess LICENSE says GPLv2+, good (and at least one source file says the
license information is in LICENSE).

> * Requires
>       This means -devel subpackage should have "Requires: boost-devel"
>       (here I am not saying about BuildRequires).

Good catch; fixed.

> -----------------------------------------------------------------
> # 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?

Well, it's an empty file and rpm complains about that. I will not remove it,
for now.

>   - Also please check the dependency for main package.
>       This may mean that python-polybori should have "Requires:
> python-imaging".

Done, thanks for spotting that.

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

Yep.

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

Thanks, I couldn't find that when I was originally packaging this. Fixed.

> * Linkage on installed system libraries
>   - 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.

What do I need to fix for this?

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

Ok, added.

Update URLs:
http://konradm.fedorapeople.org/fedora/SPECS/python-polybori.spec
http://konradm.fedorapeople.org/fedora/SRPMS/python-polybori-0.5-4.fc10.src.rpm

-- 
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.