Spec URL: http://tomspur.fedorapeople.org/review/linbox.spec SRPM URL: http://tomspur.fedorapeople.org/review/linbox-1.1.6-2.fc11.src.rpm Description: LinBox is a C++ template library for exact, high-performance linear algebra computation with dense, sparse, and structured matrices over the integers and over finite fields. __________________________ $ rpmlint linbox.spec ../SRPMS/linbox-1.1.6-2.fc11.src.rpm ../RPMS/x86_64/linbox-* ../RPMS/noarch/linbox-doc-1.1.6-2.fc11.noarch.rpm linbox.x86_64: W: shared-lib-calls-exit /usr/lib64/liblinboxsage.so.0.0.0 exit.5 linbox.x86_64: W: shared-lib-calls-exit /usr/lib64/liblbdriver.so.0.0.0 exit.5 5 packages and 1 specfiles checked; 0 errors, 2 warnings. Builds in koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1751108 __________________________ This is part of packaging sage into fedora [1]. [1] https://fedoraproject.org/wiki/SIGs/SciTech/SAGE#Missing_required_components __________________________ Patches are send upstream.
*** Bug 476299 has been marked as a duplicate of this bug. ***
- Change BuildRequires: /usr/bin/aclocal to BuildRequires: automake - Your patches are missing comments. - Drop INSTALL from %doc, at least http://www.linalg.org/linbox-html/INSTALL doesn't contain any relevant information.
Thanks for the advices. Done in: Spec URL: http://tomspur.fedorapeople.org/review/linbox.spec SRPM URL: http://tomspur.fedorapeople.org/review/linbox-1.1.6-3.fc11.src.rpm
Some quick comments. Either (1) Add the following to the spec -- Do we need to make a separate tarball? or (2) notify upstream to have the licencing headers attached. If you do notify upstream, I note that some headers are LGPLv2+ -- would upstream be able to specify LGPLv2+ to all code? #Examples and tests dir contains copyrighted code, not marked with any lic header. | #Upstream has been notified to assist in re-licencing. | rm -f ./examples/charpoly.C ./examples/checksolve.C ./examples/det.C ./examples/doubledet.C ./examples/echelon.C \ ./examples/fields/ex-fields-archetype.C ./examples/fields/ex-fields.C ./examples/fields/modular-int.C \ ./examples/graph-charpoly.C ./examples/minpoly.C ./examples/rank.C ./examples/smith.C ./examples/solve.C \ ./examples/sparseelimdet.C ./examples/sparseelimrank.C ./examples/valence.C rm -f ./tests/test-bitonic-sort.C ./tests/test-block-ring.C ./tests/test-companion.C ./tests/test-ffpack.C \ ./tests/test-hilbert.C ./tests/test-last-invariant-factor.C ./tests/test-matrix-stream.C \ ./tests/test-modular-balanced-int.C ./tests/test-ntl-hankel.C ./tests/test-ntl-RR.C \ ./tests/test-ntl-sylvester.C ./tests/test-ntl-toeplitz.C ./tests/test-quad-matrix.C \ ./tests/test-rational-solver-adaptive.C ./tests/test-rational-solver.C \ /tests/test-scalar-matrix.C ./tests/test-smith-form-adaptive.C ./tests/test-smith-form-binary.C \ /tests/test-smith-form.C ./tests/test-smith-form-iliopoulos.C ./tests/test-subiterator.C \ /tests/test-subvector.C ./tests/test-toeplitz-det.C ./tests/test-triplesbb.C ./tests/test-zero-one.C \ /tests/test-zo.C As with givaro (bug #475065), the -config script does not emit properly if --cflags --lflags are both used. The following patch can be applied --- linbox-config.in 2008-06-14 11:21:00.000000000 +1000 +++ linbox-config.in.new 2009-10-10 16:35:45.000000000 +1100 @@ -55,11 +55,11 @@ ;; --cflags) - echo -I${includedir} @GMP_CFLAGS@ @NTL_CFLAGS@ @GIVARO_CFLAGS@ @SACLIB_CFLAGS@ @LIDIA_CFLAGS@ + echo -n " -I${includedir} @GMP_CFLAGS@ @NTL_CFLAGS@ @GIVARO_CFLAGS@ @SACLIB_CFLAGS@ @LIDIA_CFLAGS@" ;; --libs) - echo -L${libdir} -llinbox @LIDIA_LIBS@ @NTL_LIBS@ @GIVARO_LIBS@ @SACLIB_LIBS@ @GMP_LIBS@ @BLAS_LIBS@ + echo -n " -L${libdir} -llinbox @LIDIA_LIBS@ @NTL_LIBS@ @GIVARO_LIBS@ @SACLIB_LIBS@ @GMP_LIBS@ @BLAS_LIBS@" ;; *) @@ -69,5 +69,6 @@ esac shift done +echo exit 0 On Linux spiderbox 2.6.27.37-170.2.104.fc10.i686, I am getting rpmlint errors. If you don't have access to this arch, you can use the test machines (https://fedoraproject.org/wiki/Test_Machine_Resources_For_Package_Maintainers) linbox.i386: E: shlib-with-non-pic-code /usr/lib/liblinboxsage.so.0.0.0 linbox.i386: W: shared-lib-calls-exit /usr/lib/liblinboxsage.so.0.0.0 exit linbox.i386: E: shlib-with-non-pic-code /usr/lib/liblbdriver.so.0.0.0 linbox.i386: W: shared-lib-calls-exit /usr/lib/liblbdriver.so.0.0.0 exit 4 packages and 0 specfiles checked; 2 errors, 2 warnings.
Hmm. Maybe you can't use the test machines -- no 686. At any rate, I'm still getting those errors.
(In reply to comment #4) > Some quick comments. > > Either (1) Add the following to the spec -- Do we need to make a separate > tarball? or (2) notify upstream to have the licencing headers attached. If you > do notify upstream, I note that some headers are LGPLv2+ -- would upstream be > able to specify LGPLv2+ to all code? This is really not necessary. Licensing and copyright are wholly different things. When some of the files that are included don't specify a license that they're under, it is assumed that they are under the license of COPYING. (IANAL, though.) See the order of operations at http://fedoraproject.org/wiki/Licensing/FAQ#How_do_I_figure_out_what_version_of_the_GPL.2FLGPL_my_package_is_under.3F In this case the attached COPYING is a generic LGPLv2 COPYING without an author's specification of which version is to be used, thus the license tag for the files without a license statement is LGPLv2+. However, you should still contact upstream and ask them to add the missing license headers, which makes things easier for everybody. > linbox.i386: E: shlib-with-non-pic-code /usr/lib/liblinboxsage.so.0.0.0 > linbox.i386: W: shared-lib-calls-exit /usr/lib/liblinboxsage.so.0.0.0 > exit > linbox.i386: E: shlib-with-non-pic-code /usr/lib/liblbdriver.so.0.0.0 > linbox.i386: W: shared-lib-calls-exit /usr/lib/liblbdriver.so.0.0.0 > exit > 4 packages and 0 specfiles checked; 2 errors, 2 warnings. Calling exit happens quite a lot with scientific libraries. To fix the non-pic code error just add -fPIC to %{optflags}.
Just to clarify what I am talking about: As an example looking at test-ring-block.C: 1 2 3 /* tests/test-block-ring.C 4 * Copyright (C) 2007 b d saunders 5 * 6 */ 7 That has a copyright header, and as the tests are not automated, and we are not using them, we don't need the file.
Yes, I understood what you were talking about. Still, the file doesn't specify a license different from that of COPYING, so it is assumed that the file is under the same license as the rest of linbox. Feel free to block FE-LEGAL if you disagree. I find that removing the files is unnecessary. ** Thomas: there are automated tests available, so add %check make -C tests check to the spec file.
I'm also assuming that the file is under the same license as the rest of linbox, if no license is specified, so I let the rm -f .... out for now. Hmm, they are not packaged anyway. Only the tests are used now in %check, if there are licensing problems, wouldn't just remove the %check again resolve this? If there are no licensing problems, maybe the examples directory would belong to the -doc package. What do you think? I just started a discussion upstream in the linbox-devel googlegroup and let you know, what (and if) happens. [1] Also about the exit call and the patches… %changelog - %%check - patch for --cflags and --lflags in config [1] http://groups.google.com/group/linbox-devel Spec URL: http://tomspur.fedorapeople.org/review/linbox.spec SRPM URL: http://tomspur.fedorapeople.org/review/linbox-1.1.6-4.fc11.src.rpm
>Feel free to block FE-LEGAL if you disagree. I'll just request that upstream be notified.
rpmlint output: linbox.x86_64: W: shared-lib-calls-exit /usr/lib64/liblinboxsage.so.0.0.0 exit.5 linbox.x86_64: W: shared-lib-calls-exit /usr/lib64/liblbdriver.so.0.0.0 exit.5 5 packages and 0 specfiles checked; 0 errors, 2 warnings. (Same warnings on i386, so everything is OK.) 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. NEEDSWORK - You are still missing comments from the patches. Please add comments in the spec file to indicate what the patch does. 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 - There are some files also in linbox/ that are missing proper license headers. Ask upstream to add them. MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK 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. OK 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 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. OK - Note that %doc htmldocs includes the htmldocs directory, so you currently have /usr/share/doc/linbox-doc-1.1.6/htmldocs/ in -doc. Instead you should use %doc htmldocs/* - Instead of # These docs are installed to the wrong place (/usr/doc), so we move # them to -doc. mv %{buildroot}%{_prefix}/doc ./htmldocs and in %files of -doc %doc htmldocs I suggest # Remove docs that are installed in the wrong place rm -rf %{buildroot}%{_prefix}/doc and in %files of -doc %doc doc/linbox.html doc/linbox-html as this is surer to preserve time stamps. - Also, note that BuildArch: noarch only works on Fedora >= 11. If you want to build for EPEL, use %if 0%{?fedora} > 10 || 0%{?rhel} > 5 BuildArch: noarch %endif MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. NEEDSWORK - Add NEWS, README and TODO. MUST: Header files must be in a -devel package. OK MUST: Static libraries must be in a -static package. N/A 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. OK 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
(In reply to comment #10) > >Feel free to block FE-LEGAL if you disagree. > I'll just request that upstream be notified. Answer at [1] Patches will be in next relase and headers maybe corrected. Not done so far, not by intention… [1] http://groups.google.com/group/linbox-devel/browse_thread/thread/6bd180a8ac29ca58 NEEDSWORK done: - Patches have comments - Add NEWS, README and TODO to %doc This won't go into EPEL for now, because givario is missing there (sure, it's an option, but sage depends on it, too). Different done: - %doc htmldocs/* instead %doc htmldocs: in configure is a docdir option, but now they are installed in /usr/share/doc/linbox-html and not in /usr/share/doc/linbox-doc-1.1.6 only one file /usr/share/doc/linbox.html needs to rm. Anything should be fine in: Spec URL: http://tomspur.fedorapeople.org/review/linbox.spec SRPM URL: http://tomspur.fedorapeople.org/review/linbox-1.1.6-5.fc11.src.rpm
(In reply to comment #12) > Different done: > - %doc htmldocs/* instead %doc htmldocs: > in configure is a docdir option, but now they are installed in > /usr/share/doc/linbox-html and not in /usr/share/doc/linbox-doc-1.1.6 > only one file /usr/share/doc/linbox.html needs to rm. That won't do. You're breaking the naming convention in /usr/share/doc. You have to place the files under /usr/share/doc/%{name}-%{version}/ (or /usr/share/doc/%{name}-doc-%{version}/ ). Note that linbox.html is the index page of the documentation and must not be removed.
My suggestion is still to use # Remove docs that are installed in the wrong place rm -rf %{buildroot}%{_prefix}/doc in %install and %doc doc/linbox.html doc/linbox-html in %files of -doc.
(In reply to comment #13) > (In reply to comment #12) > > Different done: > > - %doc htmldocs/* instead %doc htmldocs: > > in configure is a docdir option, but now they are installed in > > /usr/share/doc/linbox-html and not in /usr/share/doc/linbox-doc-1.1.6 > > only one file /usr/share/doc/linbox.html needs to rm. > > That won't do. You're breaking the naming convention in /usr/share/doc. You > have to place the files under /usr/share/doc/%{name}-%{version}/ (or > /usr/share/doc/%{name}-doc-%{version}/ ). Arrg, I suspected something like this :( > > Note that linbox.html is the index page of the documentation and must not be > removed. No, it's not, there is just a redirecting to linbox/index.html, so linbox.html isself is unecessary to me. The rest is installed with: %doc doc/linbox-html/* So the index file lives now at /usr/share/doc/linbox-doc-1.1.6/index.html Spec URL: http://tomspur.fedorapeople.org/review/linbox.spec SRPM URL: http://tomspur.fedorapeople.org/review/linbox-1.1.6-6.fc11.src.rpm
OK, everything seems fine now. APPROVED
Thanks for the review. New Package CVS Request ======================= Package Name: linbox Short Description: C++ Library for High-Performance Linear Algebra Owners: tomspur Branches: F-11 F-12 InitialCC:
cvs done.
linbox-1.1.6-6.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/linbox-1.1.6-6.fc11
linbox-1.1.6-6.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/linbox-1.1.6-6.fc12
linbox-1.1.6-6.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.
linbox is not shipped as an update anymore, it's pushed into f12-dist, but Fedora Update System does not close this bug. -> Doing so.