Bug 720998 (OpenNL)

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

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

(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

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.

(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.

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

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

(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:
> 
> http://hobbes1069.fedorapeople.org/OpenNL.spec



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

+: 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

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

Looks good to me!

*** APPROVED ***

Thank you for the review Richard! :D

New Package SCM Request
=======================
Package Name: OpenNL
Short Description: A library for solving sparse linear systems
Owners: ankursinha
Branches: f14 f15
InitialCC: susmit mrceresa

Git done (by process-git-requests).

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

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

OpenNL-3.2.1-5.fc15 has been pushed to the Fedora 15 testing repository.

OpenNL-3.2.1-5.fc14 has been pushed to the Fedora 14 stable repository.

OpenNL-3.2.1-5.fc15 has been pushed to the Fedora 15 stable repository.