Bug 245160

Summary: Review Request: fann - A fast artificial neural network library
Product: [Fedora] Fedora Reporter: Tomas Smetana <tsmetana>
Component: Package ReviewAssignee: Jason Tibbitts <j>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora, fedora-package-review, notting, tyler.l.owen
Target Milestone: ---Flags: j: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-08-03 07:14:16 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Tomas Smetana 2007-06-21 12:39:52 UTC
Spec URL: http://www.smetana.name/fann2/fann.spec
SRPM URL: http://www.smetana.name/fann2/fann-2.0.0-0.src.rpm
Description:
FANN is a neural network library which implements multilayer artificial neural networks in C with support for both fully connected and sparsely connected networks. Detailed info can be found at http://leenissen.dk/fann/.

Comment 1 Tyler Owen 2007-06-22 01:20:06 UTC
This is not an official review as I am not sponsored yet.


---------
Summary:
---------
* Main RPM includes .so
(http://fedoraproject.org/wiki/Packaging/Guidelines#head-2302ec1e1f44202c9cc4bcce24cb711266557ad7)
* fan-devel should not include .la files
(http://fedoraproject.org/wiki/Packaging/Guidelines#head-2302ec1e1f44202c9cc4bcce24cb711266557ad7)
* fann-devel ships a pkgconfig file (.pc), it should have "Requires: pkgconfig"
* Source0 is not set to the standard for Sourceforge
(http://fedoraproject.org/wiki/Packaging/SourceURL#head-e27982f18a3bfd26b5b6ecbee113d2d8f3f006f2)

---------
Details:
---------

 OK - Mock : Built on F-7 (x86)
 OK - Package meets naming and packaging guidelines
 OK - Spec file matches base package name.
 OK - Spec has consistant macro usage.
 OK - License field in spec matches
 OK - License is LGPL
 OK - License match packaging policy licenses allowed
 OK - License file is included in package
 OK - Spec in American English
 OK - Spec is legible.
 OK - Sources SHOULD match upstream md5sum:
4224efa533265dcf39237667973d0e20  fann-2.0.0.tar.bz2
 OK - Package has correct buildroot.
 OK - BuildRequires are not redundant.
 OK - %build and %install stages are correct and work.
 OK - Package has %defattr and permissions on files is good.
 OK - Package has a correct %clean section.
 OK - Package is code or permissible content.
 OK - Packages %doc files don't affect runtime.
 OK - No large doc files not in a -doc package
 OK - Package has no duplicate files in %files.
 OK - Package doesn't own any directories that other packages own.
 OK - Changelog section is correct. 
 FIX - Does not contain any .la libtool archives
 NA - .desktop file installed correctly

 ? - Should function as described. 
        No way to test
 OK - Should package latest version

---------------
Rpmlint output:
---------------
* rpmlint fann-2.0.0-0.fc7.src.rpm
        silent
* rpmlint fann-2.0.0-0.fc7.i386.rpm 
        W: fann devel-file-in-non-devel-package /usr/lib/libfloatfann.so
        W: fann devel-file-in-non-devel-package /usr/lib/libfann.so
        W: fann devel-file-in-non-devel-package /usr/lib/libdoublefann.so
        W: fann devel-file-in-non-devel-package /usr/lib/libfixedfann.so

* rpmlint fann-devel-2.0.0-0.fc7.i386.rpm

----------------------------------------


Comment 2 Tomas Smetana 2007-06-22 06:56:03 UTC
Thanks a lot. I've fixed the issues.

Comment 3 Jason Tibbitts 2007-07-04 02:21:41 UTC
Where did you fix the issues?  I don't see an updated package, just the 2.0.0-0
package that was posted initially.  Please bump the release and post a fresh
package.

I went ahead and built the srpm linked above; here's what I noticed:

If you must ship static libraries, you must add a comment indicating why they're
needed to the spec and they must be in a separate -static subpackage.

Don't ship .la files; if they are required for some reason, you must document
that fact in the spec.

The rpmlint issues noted above:
W: fann devel-file-in-non-devel-package /usr/lib64/libfloatfann.so
W: fann devel-file-in-non-devel-package /usr/lib64/libfixedfann.so
W: fann devel-file-in-non-devel-package /usr/lib64/libfann.so
W: fann devel-file-in-non-devel-package /usr/lib64/libdoublefann.so

And some additional ones:
W: fann undefined-non-weak-symbol /usr/lib64/libfloatfann.so.2.0.0 floor
W: fann undefined-non-weak-symbol /usr/lib64/libfloatfann.so.2.0.0 expf
W: fann undefined-non-weak-symbol /usr/lib64/libfloatfann.so.2.0.0 log
W: fann undefined-non-weak-symbol /usr/lib64/libfloatfann.so.2.0.0 exp
W: fann undefined-non-weak-symbol /usr/lib64/libfloatfann.so.2.0.0 pow
W: fann undefined-non-weak-symbol /usr/lib64/libfann.so.2.0.0 floor
W: fann undefined-non-weak-symbol /usr/lib64/libfann.so.2.0.0 expf
W: fann undefined-non-weak-symbol /usr/lib64/libfann.so.2.0.0 log
W: fann undefined-non-weak-symbol /usr/lib64/libfann.so.2.0.0 exp
W: fann undefined-non-weak-symbol /usr/lib64/libfann.so.2.0.0 pow
W: fann undefined-non-weak-symbol /usr/lib64/libfixedfann.so.2.0.0 log
W: fann undefined-non-weak-symbol /usr/lib64/libfixedfann.so.2.0.0 pow
W: fann undefined-non-weak-symbol /usr/lib64/libdoublefann.so.2.0.0 floor
W: fann undefined-non-weak-symbol /usr/lib64/libdoublefann.so.2.0.0 log
W: fann undefined-non-weak-symbol /usr/lib64/libdoublefann.so.2.0.0 exp
W: fann undefined-non-weak-symbol /usr/lib64/libdoublefann.so.2.0.0 pow
It looks like the library needs to be linked against libm.

Comment 4 Tomas Smetana 2007-07-09 08:34:20 UTC
Eh... sorry. The new files are here:
http://www.smetana.name/fann2/fann.spec
http://www.smetana.name/fann2/fann-2.0.0-1.fc8.src.rpm

Comment 5 Ralf Corsepius 2007-07-09 09:02:33 UTC
Mustfix:

- Please either disable building the static libs (Append --disable-static tp
%configure) or package the *.a's into a *-static sub-packages.

- Your buildroot doesn't comply to the Fedora Packaging Guidelines

Recommendation:
- I recommend to install this package's headers into a subdirectory of
%{_includedir} instead of directly installing them into %{_includedir}.

(Append --includedir=%{_includedir}/fann (or similar) to %configure)


Comment 7 Jason Tibbitts 2007-07-13 18:10:30 UTC
This is looking much better, but I'm still seeing all of the
undefined-non-weak-symbol complaints listed in comment #3.

I was able to force a link against libm by placing
  LIBS=-lm
  export LIBS
between the %setup and %configure calls in the spec.  With that, I see no
rpmlint warnings.  This seems to me to be a relatively sane way of getting this
to link properly without actually patching and regenerating the autoconf
scaffolding, since all four of the generated libraries need to link with libm.

Comment 8 Tomas Smetana 2007-08-01 12:08:41 UTC
Sorry for that but I did never get those warnings. I've updated the packages, so
it should be all OK now:
http://www.smetana.name/fann2/fann.spec
http://www.smetana.name/fann2/fann-2.0.0-3.src.rpm

Comment 9 Jason Tibbitts 2007-08-01 17:56:11 UTC
You won't see those warnings unless you install the package and run rpmlint
against it, which is something I do for all packages I review..

I've built the -3 package; rpmlint is completely quiet now and as far as I can
tell all of the complaints raised in this review have been dealt with.

Tyler's review checklist above seems complete enough to me; in addition I
checked the compiler flags and the debuginfo package and they're fine as well.

APPROVED

Comment 10 Tomas Smetana 2007-08-02 06:28:29 UTC
New Package CVS Request
=======================
Package Name: fann
Short Description: A fast artificial neural network library
Owners: tsmetana
Branches: FC-6 F-7

Comment 11 Kevin Fenzi 2007-08-02 16:42:52 UTC
cvs done.

Comment 12 Tomas Smetana 2007-08-03 07:14:16 UTC
Thanks all for your help.

Comment 13 Tomas Smetana 2013-10-08 11:25:05 UTC
Package Change Request
======================
Package Name: fann
New Branches: el6
Owners: tsmetana

Hello, I have been asked to build fann in the EPEL-6 so we can have the fann extension for PHP: bug #1015945, therefore requesting the el6 branch.

Comment 14 Gwyn Ciesla 2013-10-08 12:22:30 UTC
Git done (by process-git-requests).

Comment 15 Tomas Smetana 2014-07-31 13:56:33 UTC
Package Change Request
======================
Package Name: fann
New Branches: epel7
Owners: tsmetana

Same as in the comment #13: this time for EPEL-7.

Comment 16 Gwyn Ciesla 2014-07-31 15:48:11 UTC
Git done (by process-git-requests).