Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: zn_poly - C library for polynomial arithmetic
https://bugzilla.redhat.com/show_bug.cgi?id=608206
Summary: Review Request: zn_poly - C library for polynomial arithmetic Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: tomspur@fedoraproject.org QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
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
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=608206
Thomas Spura tomspur@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |608199
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=608206
Chen Lei supercyper1@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |supercyper1@gmail.com
--- Comment #1 from Chen Lei supercyper1@gmail.com 2010-06-25 23:36:21 EDT --- 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
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=608206
Jussi Lehtola jussi.lehtola@iki.fi changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |jussi.lehtola@iki.fi AssignedTo|nobody@fedoraproject.org |jussi.lehtola@iki.fi Flag| |fedora-review?
--- Comment #2 from Jussi Lehtola jussi.lehtola@iki.fi 2010-06-26 15:35:26 EDT --- 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
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=608206
--- Comment #3 from Thomas Spura tomspur@fedoraproject.org 2010-06-27 06:18:38 EDT --- 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_Licens...
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.
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.)
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
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=608206
--- Comment #4 from Thomas Spura tomspur@fedoraproject.org 2010-06-27 07:00:35 EDT --- 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
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=608206
--- Comment #5 from Chen Lei supercyper1@gmail.com 2010-06-27 07:06:26 EDT --- (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.
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=608206
--- Comment #6 from Jussi Lehtola jussi.lehtola@iki.fi 2010-06-27 07:06:54 EDT --- 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.
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=608206
--- Comment #7 from Jussi Lehtola jussi.lehtola@iki.fi 2010-06-27 07:08:27 EDT --- (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".
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=608206
--- Comment #8 from Thomas Spura tomspur@fedoraproject.org 2010-06-27 07:45:46 EDT --- (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.
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=608206
Jussi Lehtola jussi.lehtola@iki.fi changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #9 from Jussi Lehtola jussi.lehtola@iki.fi 2010-06-27 13:23:04 EDT --- 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.
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=608206
--- Comment #10 from Thomas Spura tomspur@fedoraproject.org 2010-06-28 05:28:24 EDT --- 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:
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=608206
Thomas Spura tomspur@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
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=608206
--- Comment #11 from Jason Tibbitts tibbs@math.uh.edu 2010-06-28 12:44:35 EDT --- CVS done (by process-cvs-requests.py).
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=608206
--- Comment #12 from Fedora Update System updates@fedoraproject.org 2010-06-29 06:16:57 EDT --- 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
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=608206
--- Comment #13 from Fedora Update System updates@fedoraproject.org 2010-06-29 06:17:03 EDT --- 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
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=608206
--- Comment #14 from Fedora Update System updates@fedoraproject.org 2010-06-29 11:29:38 EDT --- 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.
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=608206
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Fixed In Version| |zn_poly-0.9-4.fc12 Resolution| |ERRATA
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=608206
--- Comment #15 from Fedora Update System updates@fedoraproject.org 2010-06-29 11:33:15 EDT --- 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.
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=608206
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|zn_poly-0.9-4.fc12 |zn_poly-0.9-4.fc13
package-review@lists.fedoraproject.org