Spec URL: http://tomspur.fedorapeople.org/review/zn_poly.spec SRPM URL: http://tomspur.fedorapeople.org/review/zn_poly-0.9-1.fc13.src.rpm Description: zn_poly is a C library for polynomial arithmetic in Z/nZ[x], where n is any modulus that fits into an unsigned long. ############################################################################# koji build succesfull: http://koji.fedoraproject.org/koji/taskinfo?taskID=2273644 rpmlint ignorable except this one: zn_poly-devel.x86_64: E: no-ldconfig-symlink /usr/lib64/libzn_poly.so Don't know what to do with this one, is it ignoreable too? "rpmlint -I no-ldconfig-symlink" lists it as 'should'... ############################################################################# This package is a missing dependency for flint, which in turn is needed for SAGE: http://fedoraproject.org/wiki/SIGs/SciTech/SAGE
Some suggests: 1.License: (GPLv2 or GPLv3) and GPLv2+ and LGPLv2+ -> GPLv3 Because license field only applies to binary rpm 2. It'll be better to delete .a files instead of shipping it in -static subpackage. From packaging guideline: Static libraries should only be included in exceptional circumstances 3. You can add a make test to %check
rpmlint output: zn_poly.src: W: spelling-error %description -l en_US zn -> Zn, z, n zn_poly.src: W: spelling-error %description -l en_US nZ -> NZ, Zn, n zn_poly.src:48: W: configure-without-libdir-spec zn_poly.x86_64: W: spelling-error %description -l en_US zn -> Zn, z, n zn_poly.x86_64: W: spelling-error %description -l en_US nZ -> NZ, Zn, n zn_poly-debuginfo.x86_64: W: spelling-error Summary(en_US) zn -> Zn, z, n zn_poly-debuginfo.x86_64: W: spelling-error %description -l en_US zn -> Zn, z, n zn_poly-devel.x86_64: W: spelling-error Summary(en_US) zn -> Zn, z, n zn_poly-devel.x86_64: W: spelling-error %description -l en_US zn -> Zn, z, n zn_poly-devel.x86_64: W: spelling-error %description -l en_US nZ -> NZ, Zn, n zn_poly-devel.x86_64: E: no-ldconfig-symlink /usr/lib64/libzn_poly.so zn_poly-devel.x86_64: W: no-documentation zn_poly-static.x86_64: W: spelling-error Summary(en_US) zn -> Zn, z, n zn_poly-static.x86_64: W: spelling-error %description -l en_US zn -> Zn, z, n zn_poly-static.x86_64: W: spelling-error %description -l en_US nZ -> NZ, Zn, n zn_poly-static.x86_64: W: no-documentation 5 packages and 0 specfiles checked; 1 errors, 15 warnings. - You can fix the configure-without-libdir warning by replacing ./configure --cflags="%{optflags} -fPIC" --prefix=%{_prefix} \ --gmp-prefix=%{_prefix} \ --ntl-prefix=%{_prefix} \ --flint-prefix=%{_prefix} with python makemakefile.py \ --cflags="%{optflags} -fPIC" --prefix=%{_prefix} \ --gmp-prefix=%{_prefix} --ntl-prefix=%{_prefix} \ --flint-prefix=%{_prefix} > makefile - Fix the no-ldconfig-symlink either by patching makemakefile.py so that libzn_poly.so becomes a symlink, or by replacing the installed duplicate with a symlink at the end of %install. MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK MUST: The spec file for the package is legible and macros are used consistently. OK MUST: The package must be named according to the Package Naming Guidelines. OK MUST: The spec file name must match the base package %{name}. OK MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. OK MUST: The License field in the package spec file must match the actual license. OK - Specifying the multiple license scenario is OK, although you could shorten it to "GPLv2 or GPLv3", since that's what comes out at the end. MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK 0eeaae2524addf558de94bfbc914d22e zn_poly-0.9.tar.gz 0eeaae2524addf558de94bfbc914d22e ../SOURCES/zn_poly-0.9.tar.gz MUST: The package MUST successfully compile and build into binary rpms. OK MUST: The spec file MUST handle locales properly. N/A MUST: Optflags are used and time stamps preserved. NEEDSWORK - Preserve time stamps in %install by adding -p (or -a) to the cp argument. MUST: Packages containing shared library files must call ldconfig. OK MUST: A package must own all directories that it creates or require the package that owns the directory. OK - I recommend using a trailing / for directories in %files to make it clearer, i.e. %{_includedir}/zn_poly/ MUST: Files only listed once in %files listings. OK MUST: Debuginfo package is complete. OK MUST: Permissions on files must be set properly. OK MUST: Clean section exists. OK MUST: Large documentation files must go in a -doc subpackage. N/A MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. NEEDSWORK - Don't include README, it only contains instructions for compilation, which aren't relevant to the binary rpm. - Maybe include doc/REFERENCES ? MUST: Header files must be in a -devel package. OK MUST: Static libraries must be in a -static package. OK - There's nothing wrong with shipping static libraries (in -static), but normally it isn't done in Fedora. - If you want, you can just not ship the static library by either not installing it in %install, or by deleting it at the end of %install. MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. OK MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. OK MUST: Packages does not contain any .la libtool archives. N/A MUST: Desktop files are installed properly. N/A MUST: No file conflicts with other packages and no general names. OK MUST: Buildroot cleaned before install. OK SHOULD: %{?dist} tag is used in release. OK SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK SHOULD: The package builds in mock. OK
Jussi, thanks for the review. (In reply to comment #1) > Some suggests: > 1.License: (GPLv2 or GPLv3) and GPLv2+ and LGPLv2+ > -> GPLv3 > Because license field only applies to binary rpm It's in summary GPLv3, yes, but I refer to: https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios If there are some files under a different license, it must stated in the spec, and refering to the COPYING file is one possibiliy. e.g. wide_arith.h is licensed under LGPLv2+, so this needs to be reflected somewhere. > 2. > It'll be better to delete .a files instead of shipping it in -static > subpackage. > > From packaging guideline: > Static libraries should only be included in exceptional circumstances Yes, I usually delete them, but because this is a dependency of flint, which shipps static libs, I choosed to ship them here too. (Furthermore, I don't know yet, if sagemath needs static libs, if it doesn't maybe we can delete them later on.) > 3. > > You can add a make test to %check I think, I already did... :) ########################################################################### (In reply to comment #2) > rpmlint output: [snip] > > - You can fix the configure-without-libdir warning by replacing > ./configure --cflags="%{optflags} -fPIC" --prefix=%{_prefix} \ > --gmp-prefix=%{_prefix} \ > --ntl-prefix=%{_prefix} \ > --flint-prefix=%{_prefix} > with > python makemakefile.py \ > --cflags="%{optflags} -fPIC" --prefix=%{_prefix} \ > --gmp-prefix=%{_prefix} --ntl-prefix=%{_prefix} \ > --flint-prefix=%{_prefix} > makefile Done. > - Fix the no-ldconfig-symlink either by patching makemakefile.py so that > libzn_poly.so becomes a symlink, or by replacing the installed duplicate with a > symlink at the end of %install. Done in %install. [snip] > MUST: Optflags are used and time stamps preserved. NEEDSWORK > - Preserve time stamps in %install by adding -p (or -a) to the cp argument. Done. > MUST: A package must own all directories that it creates or require the package > that owns the directory. OK > - I recommend using a trailing / for directories in %files to make it clearer, > i.e. > %{_includedir}/zn_poly/ Changed. > MUST: All relevant items are included in %doc. Items in %doc do not affect > runtime of application. NEEDSWORK > - Don't include README, it only contains instructions for compilation, which > aren't relevant to the binary rpm. > - Maybe include doc/REFERENCES ? ok, done. > MUST: Static libraries must be in a -static package. OK > - There's nothing wrong with shipping static libraries (in -static), but > normally it isn't done in Fedora. > - If you want, you can just not ship the static library by either not > installing it in %install, or by deleting it at the end of %install. flint has static libs, so I have to ship them too, or the libs from flint would be nonsense. (Maybe if sage doesn't require the static libs, we'll delete it.) ########################################################################### Spec URL: http://tomspur.fedorapeople.org/review/zn_poly.spec SRPM URL: http://tomspur.fedorapeople.org/review/zn_poly-0.9-2.fc13.src.rpm
flint wants to have the headers in zn_poly/src Spec URL: http://tomspur.fedorapeople.org/review/zn_poly.spec SRPM URL: http://tomspur.fedorapeople.org/review/zn_poly-0.9-3.fc13.src.rpm
(In reply to comment #3) > Jussi, thanks for the review. > (In reply to comment #1) > > Some suggests: > > 1.License: (GPLv2 or GPLv3) and GPLv2+ and LGPLv2+ > > -> GPLv3 > > Because license field only applies to binary rpm > It's in summary GPLv3, yes, but I refer to: > See http://lists.fedoraproject.org/pipermail/packaging/2010-May/007075.html for more explanation. I think the mainpackage is a pure GPLv3 license, the license for -devel may be a bit complicated.
I'm not sure I agree with cp -pv include/*.h %{buildroot}%{_includedir}/zn_poly/src/ If flint assumes a nonstandard location for the headers, then it should be fixed.
(In reply to comment #1) > Some suggests: > 1.License: (GPLv2 or GPLv3) and GPLv2+ and LGPLv2+ > -> GPLv3 > Because license field only applies to binary rpm Note that this is not true, since "(GPLv2 or GPLv3) and GPLv2+ and LGPLv2+" resolves to "GPLv2 or GPLv3".
(In reply to comment #6) > I'm not sure I agree with > cp -pv include/*.h %{buildroot}%{_includedir}/zn_poly/src/ > > If flint assumes a nonstandard location for the headers, then it should be > fixed. Ok, reverted in: Spec URL: http://tomspur.fedorapeople.org/review/zn_poly.spec SRPM URL: http://tomspur.fedorapeople.org/review/zn_poly-0.9-4.fc13.src.rpm Will do the flint fix with the update in bug #608199.
Very well, APPROVED PS. The soname = %{version} looks a bit funny. Maybe the library still is under heavy development; the soname changing with updates might cause troubles with other packages later on.
Thanks again. New Package CVS Request ======================= Package Name: zn_poly Short Description: C library for polynomial arithmetic Owners: tomspur konradm Branches: F-12 F-13 InitialCC:
CVS done (by process-cvs-requests.py).
zn_poly-0.9-4.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/zn_poly-0.9-4.fc13
zn_poly-0.9-4.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/zn_poly-0.9-4.fc12
zn_poly-0.9-4.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.
zn_poly-0.9-4.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.