Bug 245160
Summary: | Review Request: fann - A fast artificial neural network library | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Tomas Smetana <tsmetana> |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | 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
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 ---------------------------------------- Thanks a lot. I've fixed the issues. 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. 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 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) Fixed in: http://www.smetana.name/fann2/fann.spec http://www.smetana.name/fann2/fann-2.0.0-2.src.rpm 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. 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 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 New Package CVS Request ======================= Package Name: fann Short Description: A fast artificial neural network library Owners: tsmetana Branches: FC-6 F-7 cvs done. Thanks all for your help. 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. Git done (by process-git-requests). Package Change Request ====================== Package Name: fann New Branches: epel7 Owners: tsmetana Same as in the comment #13: this time for EPEL-7. Git done (by process-git-requests). |