Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: OpenNL - A library for solving sparse linear systems
https://bugzilla.redhat.com/show_bug.cgi?id=720998
Summary: Review Request: OpenNL - A library for solving sparse linear systems Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: sanjay.ankur@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: ---
Spec URL: http://ankursinha.fedorapeople.org/opennl/OpenNL.spec SRPM URL: http://ankursinha.fedorapeople.org/opennl/OpenNL-3.2.1-1.fc15.src.rpm Description: OpenNL (Open Numerical Library) is a library for solving sparse linear systems, especially designed for the Computer Graphics community. The goal for OpenNL is to be as small as possible, while offering the subset of functionalities required by this application field. The Makefiles of OpenNL can generate a single .c + .h file, very easy to integrate in other projects. The distribution includes an implementation of our Least Squares Conformal Maps parameterization method.
New version: OpenNL 3.2.1, includes support for CUDA and Fermi architecture (Concurrent Number Cruncher and Nathan Bell's ELL formats)
OpenNL offers the following set of functionalities:
Efficient sparse matrix data structure (for non-symmetric and symmetric matrices) Iterative builder for sparse linear system Iterative builder for sparse least-squares problems Iterative solvers: conjugate gradient, BICGStab, GMRES Preconditionners: Jacobi, SSOR Iterative solver on the GPU (Concurrent Number Cruncher and Nathan Bell's ELL) Sparse direct solvers: OpenNL can be linked with SuperLU Simple demo program with LSCM (Least Squares Conformal Maps)
=========================================================================
[ankur@ankur SRPMS]$ rpmlint -i /var/lib/mock/fedora-rawhide-i386/result/*.rpm OpenNL.i686: W: spelling-error %description -l en_US functionalities -> functionalists, functionality, functionalist The value of this tag appears to be misspelled. Please double-check.
OpenNL.i686: W: spelling-error %description -l en_US parameterization -> characterization, computerization, polymerization The value of this tag appears to be misspelled. Please double-check.
OpenNL.src: W: spelling-error %description -l en_US functionalities -> functionalists, functionality, functionalist The value of this tag appears to be misspelled. Please double-check.
OpenNL.src: W: spelling-error %description -l en_US parameterization -> characterization, computerization, polymerization The value of this tag appears to be misspelled. Please double-check.
OpenNL-devel.i686: E: zero-length /usr/share/doc/OpenNL-devel-3.2.1/examples/DATA/example_2.mtx OpenNL-devel.i686: E: zero-length /usr/share/doc/OpenNL-devel-3.2.1/examples/DATA/example_1.mtx 4 packages and 0 specfiles checked; 2 errors, 4 warnings.
Empty files are for the examples.
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=720998
Ankur Sinha sanjay.ankur@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Alias| |OpenNL
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=720998
Ankur Sinha sanjay.ankur@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |721112(vmtk)
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=720998
Richard Shaw hobbes1069@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |hobbes1069@gmail.com Flag| |fedora-review?
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=720998
Richard Shaw hobbes1069@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |hobbes1069@gmail.com
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=720998
Richard Shaw hobbes1069@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
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=720998
--- Comment #1 from Ankur Sinha sanjay.ankur@gmail.com 2011-07-14 10:41:18 EDT --- Updated spec/srpm:
http://ankursinha.fedorapeople.org/opennl/OpenNL-3.2.1-2.fc15.src.rpm
http://ankursinha.fedorapeople.org/opennl/OpenNL.spec
* Thu Jul 14 2011 Ankur Sinha <ankursinha AT fedoraproject DOT org> - 3.2.1-2 - add patch to correct libname and versioning (courtesy of Richard Shaw)
Thanks! Ankur
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=720998
--- Comment #2 from Richard Shaw hobbes1069@gmail.com 2011-07-14 11:53:22 EDT --- Hey,
Just some quick questions about the spec file.
I'm pretty sure my patch update had cmake creating the correct library symlinks. Is there a particular reason you want to create them manually? Also, since you've hardcoded the names in %install and %files, you'll have to fix this on every version change.
Other than that, everything looks good. I'm pulling the srpm now for the usual checks.
Richard
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=720998
--- Comment #3 from Ankur Sinha sanjay.ankur@gmail.com 2011-07-14 12:55:42 EDT --- (In reply to comment #2)
Hey,
Just some quick questions about the spec file.
I'm pretty sure my patch update had cmake creating the correct library symlinks. Is there a particular reason you want to create them manually? Also, since you've hardcoded the names in %install and %files, you'll have to fix this on every version change.
Really?
With this in the spec:
--------------------------------------------------------------
# Install includes install -d $RPM_BUILD_ROOT/%{_includedir}/NL/ cp -av src/NL/nl.h $RPM_BUILD_ROOT/%{_includedir}/ find src/NL/ -name "*.h" ! -name "nl.h" -execdir cp -av '{}' $RPM_BUILD_ROOT/%{_includedir}/NL/ ;
# Create the .so symlinks #pushd $RPM_BUILD_ROOT/%{_libdir} # ln -sfv libopennl.so.3.2.1 libopennl.so # ln -sfv libopennl.so.3.2.1 libopennl.so.3 #popd
%post -p /sbin/ldconfig %postun -p /sbin/ldconfig
%files %doc doc #%{_libdir}/libopennl.so.3.2.1 #%{_libdir}/libopennl.so.3
%files devel %doc examples #%{_libdir}/libopennl.so %{_includedir}/*
-----------------------------------------------------------
mock fails saying this:
DEBUG: Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 DEBUG: Processing files: OpenNL-debuginfo-3.2.1-2.fc16.i686 DEBUG: Checking for unpackaged file(s): /usr/lib/rpm/check-files /builddir/build/BUILDROOT/OpenNL-3.2.1-2.fc16.i386 DEBUG: error: Installed (but unpackaged) file(s) found: DEBUG: /usr/lib/libopennl.so.3.2.1 DEBUG: Installed (but unpackaged) file(s) found: DEBUG: /usr/lib/libopennl.so.3.2.1 DEBUG: RPM build errors:
--------------------------------------------------------------
which is why I created the symlinks manually. rpmlint also spewed the "no shared library symlink" error. Can you please recheck once? I don't think it generated the symlinks.
I can replace use macros in the spec. I'll go do that :)
Other than that, everything looks good. I'm pulling the srpm now for the usual checks.
Richard
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=720998
--- Comment #4 from Ankur Sinha sanjay.ankur@gmail.com 2011-07-14 13:04:18 EDT --- http://ankursinha.fedorapeople.org/opennl/OpenNL-3.2.1-3.fc15.src.rpm
http://ankursinha.fedorapeople.org/opennl/OpenNL.spec
* Thu Jul 14 2011 Ankur Sinha <ankursinha AT fedoraproject DOT org> - 3.2.1-3 - add version macros to soname etc.
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=720998
--- Comment #5 from Richard Shaw hobbes1069@gmail.com 2011-07-14 13:51:40 EDT --- (In reply to comment #3)
(In reply to comment #2)
Hey,
Just some quick questions about the spec file.
I'm pretty sure my patch update had cmake creating the correct library symlinks. Is there a particular reason you want to create them manually? Also, since you've hardcoded the names in %install and %files, you'll have to fix this on every version change.
Really?
Yup! But I was concentrating so much on the build/cmake side I forgot to update the install side. My Fault!
With this in the spec:
# Install includes install -d $RPM_BUILD_ROOT/%{_includedir}/NL/ cp -av src/NL/nl.h $RPM_BUILD_ROOT/%{_includedir}/ find src/NL/ -name "*.h" ! -name "nl.h" -execdir cp -av '{}' $RPM_BUILD_ROOT/%{_includedir}/NL/ ;
# Create the .so symlinks #pushd $RPM_BUILD_ROOT/%{_libdir} # ln -sfv libopennl.so.3.2.1 libopennl.so # ln -sfv libopennl.so.3.2.1 libopennl.so.3 #popd
%post -p /sbin/ldconfig %postun -p /sbin/ldconfig
%files %doc doc #%{_libdir}/libopennl.so.3.2.1 #%{_libdir}/libopennl.so.3
They're not commented out in mine but I instead used one line: %{_libdir}/libopennl.so.*
That way it picks up all the versioned libraries and leaves the .so for the -devel package.
To fix "%install" I first need to understand why you're using the "install" command. Generally mkdir and cp (with appropriate option, -p, -a, -r, etc.) is sufficient. In my spec files (and many others I've learned from) install is generally used to correct things like permissions. Cmake *SHOULD* create libraries with correct permissions. If not, rpmlint will complain about it. That being said I think the library portion of %install could look like this:
""" %install # Install library files mkdir -p $RPM_BUILD_ROOT/%{_libdir} cp -p build/linux-Release/binaries/lib/libopennl.so* $RPM_BUILD_ROOT/%{_libdir}/ """
Which will copy the library and symlinks (instead of just the library, which is what I forgot to fix). Also all the above changes will make all the global variables at the top of the spec unnecessary.
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=720998
--- Comment #6 from Ankur Sinha sanjay.ankur@gmail.com 2011-07-15 09:37:56 EDT --- Hi Richard,
Not sure what's going on here:
# Mock says this during the build: # DEBUG: *** WARNING: identical binaries are copied, not linked: # DEBUG: /usr/lib/libopennl.so.3.2.1 # DEBUG: and /usr/lib/libopennl.so.3
# DEBUG: *** WARNING: identical binaries are copied, not linked: # DEBUG: /usr/lib/libopennl.so # DEBUG: and /usr/lib/libopennl.so.3.2.1 # Manually creating a symlink
So, instead of symlinking, it's creating new files?
Thanks, Ankur
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=720998
--- Comment #7 from Richard Shaw hobbes1069@gmail.com 2011-07-15 10:29:49 EDT --- Ack! I've been spoiled by "make install" :)
I've fixed it and I'm doing a mock build now to make sure. Due to the symbolic links the "cp" command needed "-a" not "-p". It was copying the target of the symlink (the real library) instead of copying the symlink itself.
Ok, done with the mock build and unfortunately cmake does not create the library with the correct permissions (0775 instead of 0755). There may be a way to fix that during the build but the only reference I could find to PERMISSIONS was during "install" and of course we are manually installing.
Also, since we can't copy them over all at once I had to use some 'find' mojo.
I know in some ways this seems like more work than what you were doing (and it may well be) but it's generally better not to rely on macros and hard coded paths whenever practical. Although in this particular case you were pretty sure this was the last version it's best to try and keep things automatic as possible. I'm pretty sure if there was a new patch release you could just upload the new source, change the version in the spec file, and rebuild.
Double Ack! They don't include the patch level in the version for the source archive file name... Oh well. I think we're close enough :)
Here's the spec file with the fixes for the library install:
http://hobbes1069.fedorapeople.org/OpenNL.spec
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=720998
--- Comment #8 from Ankur Sinha sanjay.ankur@gmail.com 2011-07-15 11:12:51 EDT --- (In reply to comment #7)
Ack! I've been spoiled by "make install" :)
I've fixed it and I'm doing a mock build now to make sure. Due to the symbolic links the "cp" command needed "-a" not "-p". It was copying the target of the symlink (the real library) instead of copying the symlink itself.
Ok, done with the mock build and unfortunately cmake does not create the library with the correct permissions (0775 instead of 0755). There may be a way to fix that during the build but the only reference I could find to PERMISSIONS was during "install" and of course we are manually installing.
Also, since we can't copy them over all at once I had to use some 'find' mojo.
I know in some ways this seems like more work than what you were doing (and it may well be) but it's generally better not to rely on macros and hard coded paths whenever practical. Although in this particular case you were pretty sure this was the last version it's best to try and keep things automatic as possible. I'm pretty sure if there was a new patch release you could just upload the new source, change the version in the spec file, and rebuild.
Double Ack! They don't include the patch level in the version for the source archive file name... Oh well. I think we're close enough :)
Here's the spec file with the fixes for the library install:
Corrected/modified
http://ankursinha.fedorapeople.org/opennl/OpenNL-3.2.1-4.fc15.src.rpm
http://ankursinha.fedorapeople.org/opennl/OpenNL.spec
Now, *everything* finally looks okay :P
Thanks Richard, Ankur
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=720998
--- Comment #9 from Richard Shaw hobbes1069@gmail.com 2011-07-15 14:38:16 EDT --- +: OK -: must be fixed =: should be fixed (at your discretion) ?: Question or clairification needed N: not applicable
MUST: [+] rpmlint output: shown in comment: No major issues. [+] follows package naming guidelines [+] spec file base name matches package name [+] package meets the packaging guidelines [+] package uses a Fedora approved license: BSD [+] license field matches the actual license. [+] license file is included in %doc [+] spec file is in American English [+] spec file is legible [+] sources match upstream: md5sum matches (6e182f15bf9bc8ffe95547c1cdd7e7b4) [+] package builds on at least one primary arch: Tested F14 x86_64 [N] appropriate use of ExcludeArch [+] all build requirements in BuildRequires [N] spec file handles locales properly [+] ldconfig in %post and %postun [+] no bundled copies of system libraries [N] no relocatable packages [+] package owns all directories that it creates [+] no files listed twice in %files [+] proper permissions on files [+] consistent use of macros [+] code or permissible content [N] large documentation in -doc [+] no runtime dependencies in %doc [+] header files in -devel [N] static libraries in -static [+] .so in -devel [+] -devel requires main package [+] package contains no libtool archives [N] package contains a desktop file, uses desktop-file-install/validate [+] package does not own files/dirs owned by other packages [+] all filenames in UTF-8
SHOULD: [+] query upstream for license text [N] description and summary contains available translations [+] package builds in mock [+] package builds on all supported arches [?] package functions as described [+] sane scriptlets [+] subpackages require the main package [N] placement of pkgconfig files [N] file dependencies versus package dependencies [N] package contains man pages for binaries/scripts
Ok, it's not a big deal but the only thing I would change is:
%doc doc
to
%doc doc/*
Right now documentation is going into:
/usr/share/doc/OpenNL-3.2.1/doc
which is redundant...
Let me know what you think!
Richard
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=720998
--- Comment #10 from Ankur Sinha sanjay.ankur@gmail.com 2011-07-17 10:00:29 EDT --- Hi Richard,
I've fixed the doc macro usage.
Freshly baked packages:
http://ankursinha.fedorapeople.org/opennl/OpenNL.spec
http://ankursinha.fedorapeople.org/opennl/OpenNL-3.2.1-5.fc15.src.rpm
Thanks! Ankur
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=720998
Richard Shaw hobbes1069@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #11 from Richard Shaw hobbes1069@gmail.com 2011-07-17 10:28:57 EDT --- Looks good to me!
*** APPROVED ***
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=720998
--- Comment #12 from Ankur Sinha sanjay.ankur@gmail.com 2011-07-17 12:53:13 EDT --- Thank you for the review Richard! :D
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=720998
Ankur Sinha sanjay.ankur@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #13 from Ankur Sinha sanjay.ankur@gmail.com 2011-07-17 12:54:18 EDT --- New Package SCM Request ======================= Package Name: OpenNL Short Description: A library for solving sparse linear systems Owners: ankursinha Branches: f14 f15 InitialCC: susmit mrceresa
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=720998
--- Comment #14 from Jon Ciesla limb@jcomserv.net 2011-07-17 13:17:05 EDT --- Git done (by process-git-requests).
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=720998
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
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=720998
--- Comment #15 from Fedora Update System updates@fedoraproject.org 2011-07-17 13:56:09 EDT --- OpenNL-3.2.1-5.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/OpenNL-3.2.1-5.fc14
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=720998
--- Comment #16 from Fedora Update System updates@fedoraproject.org 2011-07-17 13:56:18 EDT --- OpenNL-3.2.1-5.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/OpenNL-3.2.1-5.fc15
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=720998
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #17 from Fedora Update System updates@fedoraproject.org 2011-07-18 18:35:26 EDT --- OpenNL-3.2.1-5.fc15 has been pushed to the Fedora 15 testing repository.
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=720998
--- Comment #18 from Fedora Update System updates@fedoraproject.org 2011-08-03 18:54:45 EDT --- OpenNL-3.2.1-5.fc14 has been pushed to the Fedora 14 stable repository.
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=720998
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Fixed In Version| |OpenNL-3.2.1-5.fc14 Resolution| |ERRATA Last Closed| |2011-08-03 18:54:51
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=720998
--- Comment #19 from Fedora Update System updates@fedoraproject.org 2011-08-03 18:55:58 EDT --- OpenNL-3.2.1-5.fc15 has been pushed to the Fedora 15 stable repository.
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=720998
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|OpenNL-3.2.1-5.fc14 |OpenNL-3.2.1-5.fc15
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla