Bug 243109

I took a quick look at this.  It builds ok in mock on rawhide; here's what
rpmlint says:

W: loki-lib unused-direct-shlib-dependency /usr/lib64/libloki.so.0.1.6
/lib64/libpthread.so.0
W: loki-lib unused-direct-shlib-dependency /usr/lib64/libloki.so.0.1.6
/lib64/libm.so.6

Now, the makefile explicitly adds -lpthread on linux; I can only assume they do
this because it's required in some way.  This library does seem to wrap pthread
functions but perhaps they aren't directly called.  In any case I don't really
understand enough to decide whether this is a problem.

I don't know where the link against libm comes from; I don't see it specified
anywhere.

There's a test suite in the package, but it requires that you do a static
compile and even then it doesn't actually seem to work.  You might want to make
a comment about that in the spec (or perhaps investigate and see if you can make
it work).

I have managed to remove the dependency on pthread.
The dependency on libm appears because the library is linked with g++ and the
compiler introduces automatically a dependency on it that I don't know how to
remove.

New spec and srpm

Spec URL: http://astro-sfg.fis.ucm.es/~spr/loki-lib.spec
SRPM URL: http://astro-sfg.fis.ucm.es/~spr/loki-lib-0.1.6-0.2.src.rpm

I think I have fixed the problem with libm:

Spec URL: http://astro-sfg.fis.ucm.es/~spr/loki-lib.spec
SRPM URL: http://astro-sfg.fis.ucm.es/~spr/loki-lib-0.1.6-0.3.src.rpm

rpmlint is clean now.  Some concerns I have:

The summary isn't very descriptive.

Your release should start with one and should be an integer unless the naming
guidelines specify otherwise.

All of the .cpp source files are in the debuginfo package, but only a few of the
.h files are.  I have no idea why.  Perhaps they're not used in compilation but
are installed as additional usable templates?

A readme.txt file is installed into /usr/include.

So, just comment on those and fix them if you think they're fixable and we're
good to go.

Review:
* source files match upstream:
   b2dbf0c89098a4c51822bae767bf0c33650f206caad848eaea37b61e8f482395  
   loki-0.1.6.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
 summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text not included upstream (It's in each source file and scattered 
  throughout the documentation but not actually included in a separate file.)
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64[).
* package installs properly
? debuginfo package might be complete; I can't tell.
* rpmlint is silent.
* final provides and requires are sane:
  loki-lib-0.1.6-0.3.fc8.x86_64.rpm
   libloki.so.0.1.6()(64bit)
   loki-lib = 0.1.6-0.3.fc8
  =
   /sbin/ldconfig
   libstdc++.so.6()(64bit)
   libstdc++.so.6(CXXABI_1.3)(64bit)
   libstdc++.so.6(GLIBCXX_3.4)(64bit)

  loki-lib-devel-0.1.6-0.3.fc8.x86_64.rpm
   loki-lib-devel = 0.1.6-0.3.fc8
  =
   libloki.so.0.1.6()(64bit)
   loki-lib = 0.1.6-0.3.fc8

  loki-lib-doc-0.1.6-0.3.fc8.x86_64.rpm
   loki-lib-doc = 0.1.6-0.3.fc8
  =

* %check is not present; the included test suite doesn't seem to work.
* shared libraries present; ldconfig is called properly.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets are OK (ldconfig)
* code, not content.
* documentation is in a -doc subpackage.
* %docs are not necessary for the proper functioning of the package.
* headers are in -devel subpackage.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.

I have fixed some of the issues:

* the summary is longer and more descriptive
* I have patched the Makefile of the test suite and added a check rule to the
specfile
* fixed the release number
* the readme file in /usr/include is removed


Respecting the debug rpm, the library is heavely based on templates, so it makes
sense that only a few ha files are actually compiled. Most of them are directly
installed.

Spec URL: http://astro-sfg.fis.ucm.es/~spr/loki-lib.spec
SRPM URL: http://astro-sfg.fis.ucm.es/~spr/loki-lib-0.1.6-1.src.rpm

Hmm, this one didn't build for me; the test suite fails:

g++ --shared -fPIC -L../../lib -L. -o libsingletondll.so singletondll.lo -lfoo
../../lib/libloki.so
/usr/bin/ld: cannot find -lfoo
collect2: ld returned 1 exit status
make[1]: *** [libsingletondll.so] Error 1
make[1]: Leaving directory `/builddir/build/BUILD/loki-0.1.6/test/SingletonDll'

Since I build on an 8-way, I disabled parallel make and got a different set of
errors:
g++ -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic -I../../include -DNDEBUG  -c -o
LockTest.o LockTest.cpp
LockTest.cpp: In function 'void* RunLocked(void*)':
LockTest.cpp:374: error: cast from 'void*' to 'int' loses precision
LockTest.cpp: In function 'void* RunLockedStorage(void*)':
LockTest.cpp:390: error: cast from 'void*' to 'int' loses precision
LockTest.cpp: In function 'void* Run(void*)':
LockTest.cpp:411: error: cast from 'void*' to 'int' loses precision
LockTest.cpp: In function 'void DoLockedPtrTest()':
LockTest.cpp:426: warning: ignoring return value of 'int system(const char*)',
declared with attribute warn_unused_result
LockTest.cpp:434: warning: ignoring return value of 'int system(const char*)',
declared with attribute warn_unused_result
LockTest.cpp: In function 'void DoLockedStorageTest()':
LockTest.cpp:451: warning: ignoring return value of 'int system(const char*)',
declared with attribute warn_unused_result
LockTest.cpp:458: warning: ignoring return value of 'int system(const char*)',
declared with attribute warn_unused_result
make[1]: *** [LockTest.o] Error 1
make[1]: Leaving directory `/builddir/build/BUILD/loki-0.1.6/test/SmartPtr'
make: *** [SmartPtr] Error 2

I have sent a bug and a patch upstream to solve the parallel make problem. the
falliure of the test suite on x86_64 systems is also an open bug upstream. So I
have disabled the entire %check rule. I have also added Requires: glibc-headers
rule in the -devel subpackage to get /usr/include/pthread.h

Spec URL: http://astro-sfg.fis.ucm.es/~spr/loki-lib.spec
SRPM URL: http://astro-sfg.fis.ucm.es/~spr/loki-lib-0.1.6-2.src.rpm

OK, this builds fine for me.

According to your explanation above, the debuginfo package is OK.
Summary looks good.

However, there's one rather significant blocker which I should have noticed
before: I simply cannot approve a package which includes files with such common
names as /usr/include/Key.h or /usr/include/Tuple.h.  These really need to be
installed into a subdirectory of /usr/include; the potential for conflicts is
just too great otherwise.

Fixed, the files are now under directory /usr/include/loki

Spec URL: http://astro-sfg.fis.ucm.es/~spr/loki-lib.spec
SRPM URL: http://astro-sfg.fis.ucm.es/~spr/loki-lib-0.1.6-3.src.rpm

This one fails to build for me:

error: Installed (but unpackaged) file(s) found:
   /usr/lib/libloki.a
   /usr/lib/libloki.so
   /usr/lib/libloki.so.0.1.6


RPM build errors:
    File not found by glob:
/var/tmp/loki-lib-0.1.6-3.fc8-root-mockbuild/usr/lib64/*.so.0.1.6
    File not found by glob:
/var/tmp/loki-lib-0.1.6-3.fc8-root-mockbuild/usr/lib64/*.so
    File not found: /var/tmp/loki-lib-0.1.6-3.fc8-root-mockbuild/usr/lib64/libloki.a
    Installed (but unpackaged) file(s) found:
   /usr/lib/libloki.a
   /usr/lib/libloki.so
   /usr/lib/libloki.so.0.1.6
Error building package from loki-lib-0.1.6-3.fc8.src.rpm, See build log

I think I have fixed this. Makefile installs directly in $prefix/lib instead of
$libdir

Spec URL: http://astro-sfg.fis.ucm.es/~spr/loki-lib.spec
SRPM URL: http://astro-sfg.fis.ucm.es/~spr/loki-lib-0.1.6-0.4.src.rpm

I get a 404 on that SRPM link, but I grabbed the updated spec and used that.

This one builds and installs fine.  rpmlint is still quiet, and the include
files are now in a non-conflicting location.  Everything looks good to me now.

APPROVED

Thanks for all the work.

Erm, I set the wrong flag somehow.

Thanks for the review work also

New Package CVS Request
=======================
Package Name: loki-lib
Short Description: A C++ library of design patterns and idioms
Owners: spr.ucm.es
Branches: FC-6 F-7
InitialCC:

cvs done. 
Note that you should be setting the fedora-cvs flag to ? to request cvs, not +.