Bug 470727

a few comments:

- there is a newer upstream 2.6.1b, and it seems upstream switched to a more sane naming. 

- "a" and "b" tags from version seems to me like post release package, so please see:
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#NonNumericRelease

- there is a doc target for building development documentation (requires doxgey and pdfjam)

- rpmlint is not silent:

slimdata.x86_64: W: unstripped-binary-or-object /usr/lib64/libslim.so
you need to set the exec bit on the so file

slimdata.x86_64: W: no-soname /usr/lib64/libslim.so
library does not have soname set, as this is a system library this is a blocker. you will have to recompile the file with -Wl,-soname -Wl,libslim.so. You should also report this upstream.

slimdata.x86_64: W: shared-lib-calls-exit /usr/lib64/libslim.so exit.5
slimdata.x86_64: W: shared-lib-calls-exit /usr/lib64/libslim.so exit@@GLIBC_2.2.5
This library package calls exit() or _exit(), probably in a non-fork()
context. Doing so from a library is strongly discouraged - when a library
function calls exit(), it prevents the calling program from handling the
error, reporting it to the user, closing files properly, and cleaning up any
state that the program has. It is preferred for the library to return an
actual error code and let the calling program decide how to handle the
situation.
these are not blockers but they should be reported upstream

slimdata-devel.x86_64: W: no-documentation
see my previous comment about documentation.

> - there is a newer upstream 2.6.1b, and it seems upstream switched to a more
> sane naming. 

Thanks for pointing it out, and yes, significantly saner in the naming department.

> - "a" and "b" tags from version seems to me like post release package, so
> please see:
> https://fedoraproject.org/wiki/Packaging/NamingGuidelines#NonNumericRelease

Of course, and since they are "Properly ordered simple versions" it's ok to have the 'b' in the Version (and the 'a' previously).

> - there is a doc target for building development documentation (requires doxgey
> and pdfjam)

Thanks for pointing it out.  It now builds the docs when I build locally, but there are errors when I try a koji scratch build.

> - rpmlint is not silent:
> 
> slimdata.x86_64: W: unstripped-binary-or-object /usr/lib64/libslim.so
> you need to set the exec bit on the so file

OK.

> slimdata.x86_64: W: no-soname /usr/lib64/libslim.so
> library does not have soname set, as this is a system library this is a
> blocker. you will have to recompile the file with -Wl,-soname -Wl,libslim.so.
> You should also report this upstream.

Then also the symlinks need to be generated properly.  I'll report (and discuss) this with upstream before I fix fully.  

> slimdata.x86_64: W: shared-lib-calls-exit /usr/lib64/libslim.so
> exit.5
> slimdata.x86_64: W: shared-lib-calls-exit /usr/lib64/libslim.so
> exit@@GLIBC_2.2.5
> This library package calls exit() or _exit(), probably in a non-fork()
> context. Doing so from a library is strongly discouraged - when a library
> function calls exit(), it prevents the calling program from handling the
> error, reporting it to the user, closing files properly, and cleaning up any
> state that the program has. It is preferred for the library to return an
> actual error code and let the calling program decide how to handle the
> situation.
> these are not blockers but they should be reported upstream

I'll let upstream know.

> slimdata-devel.x86_64: W: no-documentation
> see my previous comment about documentation.

OK.

New spec and srpm available: 
http://matt.truch.net/fedora/slimdata.spec
http://matt.truch.net/fedora/slimdata-2.6.1b-1.fc11.src.rpm

(In reply to comment #2)
> Thanks for pointing it out.  It now builds the docs when I build locally, but
> there are errors when I try a koji scratch build.
It won't fail if you correctly pick doxygen and pdfjam as BR.
Also please add doc section as requested to -devel subpackage



> Then also the symlinks need to be generated properly.  I'll report (and
> discuss) this with upstream before I fix fully.  
My suggestion is to set soname to to libslim.so. And no further modification would be necessary. Now the test target fails.

> It won't fail if you correctly pick doxygen and pdfjam as BR.
> Also please add doc section as requested to -devel subpackage

Missed that first time around; docs now build, and since there are docs, I now have them included in -devel.

> > Then also the symlinks need to be generated properly.  I'll report (and
> > discuss) this with upstream before I fix fully.  
> My suggestion is to set soname to to libslim.so. And no further modification
> would be necessary. Now the test target fails.

Soname set as requested.  

The test target has never failed on my machine, nor in koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1064087
What error are you seeing?

New spec and srpm:
http://matt.truch.net/fedora/slimdata.spec
http://matt.truch.net/fedora/slimdata-2.6.1b-2.fc11.src.rpm

I forgot to add, rpmlint still complains about the soname, and about the unstripped binaries/executables.  I'm not fully sure what is wrong now.

(In reply to comment #5)
> I forgot to add, rpmlint still complains about the soname, and about the
> unstripped binaries/executables.  I'm not fully sure what is wrong now.

You need to setup soname to libslim.so.<major>. I'm sorry for misleading you.
For now you can setup soname to libslim.so.2, but probably the best way to handle this is to contact upstream. btw any news on that ?
Also you need to make sure test target correctly picks libslim.so.2, so you need to symlink libslim.so to libslim.so.2 in builddir, or tests would fail:

../bin/slim --preserve -k -C -m2 -i -c1 -r16384 /tmp/data_partial.bin
../bin/slim: error while loading shared libraries: libslim.so.2: cannot open shared object file: No such file or directory

Another suggestion is using INSTALL="install -p" to make install from %files section to preserve timestamps of installed files.

Still having trouble with the soname stuff.  It seems that regardless of what I do, rpmlint complains about the soname.  Also, regardless of what the executable bit is set to, rpmlint also complains about the unstripped binaries.  

As for upstream, they do not want to change how they do the sonames as they claim that Debian requires it the way they do it.  In regards to the exit() calls, they have it on the todo list, but won't get to it in the short-term.

(In reply to comment #7)
> Still having trouble with the soname stuff.  It seems that regardless of what I
> do, rpmlint complains about the soname.  Also, regardless of what the
> executable bit is set to, rpmlint also complains about the unstripped binaries. 

I forgot to add that things work much better on F-10 (which is all I have to test on).  See the scratch build I did: 
http://koji.fedoraproject.org/koji/taskinfo?taskID=1131415

Please bump & post your spec and srpm for each modification you make. It is easier to track.

(In reply to comment #7)
> As for upstream, they do not want to change how they do the sonames as they
> claim that Debian requires it the way they do it.
Shared libraries should have a proper versioned soname. I really doubt Debian forbids that.
http://www.tldp.org/HOWTO/Program-Library-HOWTO/shared-libraries.html

You can make one but problems will appear in case upstream decides on a different scheme later on.
https://fedoraproject.org/wiki/PackageMaintainers/Common_Rpmlint_Issues#no-soname
Starting with .0 for major is the best way. I still think you should insist on proper soname upstream.

(In reply to comment #9)
> Please bump & post your spec and srpm for each modification you make. It is
> easier to track.

Sorry, forgot to do that.  They're at:
http://matt.truch.net/fedora/slimdata.spec
http://matt.truch.net/fedora/slimdata-2.6.1b-3.fc11.src.rpm

> (In reply to comment #7)
> > As for upstream, they do not want to change how they do the sonames as they
> > claim that Debian requires it the way they do it.
> Shared libraries should have a proper versioned soname. I really doubt Debian
> forbids that.
> http://www.tldp.org/HOWTO/Program-Library-HOWTO/shared-libraries.html
> 
> You can make one but problems will appear in case upstream decides on a
> different scheme later on.
> https://fedoraproject.org/wiki/PackageMaintainers/Common_Rpmlint_Issues#no-soname
> Starting with .0 for major is the best way. I still think you should insist on
> proper soname upstream.

I have told them upstream, and I think they will with the next release.  For now I have .0 as you suggest.  I think it's now kosher wrt sonames, please let me know otherwise.

Thanks for the update.

Package fails to build under mock for rawhide (gcc 4.4).
Mainly because of gcc headers cleanup some headers now are not included by default or indirectly through some other headers.
please consider the following patch:

--- slim-2.6.1b/include/slim.h  2008-11-07 00:46:47.000000000 +0200
+++ slim-2.6.1b-mod/include/slim.h      2009-02-24 15:22:57.000000000 +0200
@@ -8,9 +8,11 @@
 #define SLIM_H

 #include <iostream>
+#include <cstdio>
 #include <cstdlib>
 #include <cassert>
 #include <cstring>   // for strlen, strcpy
+#include <stdint.h>

 #define SLIM_VERSION "v2_6_0"

--- slim-2.6.1b/src/slim_control.cpp    2008-11-07 00:46:48.000000000 +0200
+++ slim-2.6.1b-mod/src/slim_control.cpp        2009-02-24 15:51:01.000000000 +0200
@@ -503,7 +503,7 @@
   // Normally, this means stripping any trailing dot-suffix.
   // But if we have --preserve, then append a ".raw" suffix instead.
   char *rawname;
-  char *last_suffix = strrchr(compname,'.');
+  const char *last_suffix = strrchr(compname,'.');
   size_t baselen;
   if (last_suffix)
     baselen = (last_suffix-compname);

it would be nice to send it upstream too.

(In reply to comment #11)
> Thanks for the update.
> 
> Package fails to build under mock for rawhide (gcc 4.4).
> Mainly because of gcc headers cleanup some headers now are not included by
> default or indirectly through some other headers.
> please consider the following patch:

Excellent.  Thanks for the patch.  I've sent it upstream.  

New spec, srpm, and koji scratch build:
http://matt.truch.net/fedora/slimdata.spec
http://matt.truch.net/fedora/slimdata-2.6.1b-4.fc11.src.rpm
http://koji.fedoraproject.org/koji/taskinfo?taskID=1159533

Review: 

OK source files match upstream:
        c69db6265da59079263043dc5f5540e67f6d35cabed54016a17a5e82f31326d2  slim-2.6.1b.tgz
OK package meets naming and versioning guidelines.
OK specfile is properly named, is cleanly written and uses macros consistently.
OK summary a short and concise description.
OK description is OK.
OK dist tag is present.
OK build root is sane.
NOT OK license field matches the actual license.
OK license is open source-compatible.
OK license text included in package.
OK latest version is being packaged.
OK BuildRequires are proper.
OK compiler flags are appropriate.
OK %clean is present.
OK package builds in mock (rawhide, x86_64).
OK package installs properly.
OK debuginfo package looks complete.
OK rpmlint has acceptable warnings.
OK final provides and requires are sane:
slimdata-2.6.1b-4.fc11.x86_64.rpm
        libslim.so.0()(64bit)
        slimdata = 2.6.1b-4.fc11
        slimdata(x86-64) = 2.6.1b-4.fc11
=
        /sbin/ldconfig
        libc.so.6()(64bit)
        libgcc_s.so.1()(64bit)
        libm.so.6()(64bit)
        libslim.so.0()(64bit)
        libstdc++.so.6()(64bit)
slimdata-devel-2.6.1b-4.fc11.x86_64.rpm
        slimdata-devel = 2.6.1b-4.fc11
        slimdata-devel(x86-64) = 2.6.1b-4.fc11
=
        libslim.so.0()(64bit)
        pkgconfig
        slimdata = 2.6.1b-4.fc11
OK %check is present and all tests pass:
Running compression data-type tests on /tmp/data_partial.bin (size 1000000)...
...Passed all 8 data-type tests
Running compression tests on /tmp/data_partial.bin (size 1000000)...
...Passed all 16 compression tests
Running expansion tests on /tmp/data_partial.bin (size 1000000)...
...Passed all 17 expansion tests
Running compression data-type tests on /tmp/data_partial.bin (size 1000003)...
...Passed all 8 data-type tests
Running compression data-type tests on /tmp/data_partial.bin (size 524289)...
...Passed all 8 data-type tests
Running compression data-type tests on /tmp/data_partial.bin (size 524287)...
...Passed all 8 data-type tests
Running compression data-type tests on /tmp/fake_test_data.bin (size 20000000)...
...Passed all 8 data-type tests
Running compression tests on /tmp/fake_test_data.bin (size 20000000)...
...Passed all 16 compression tests
Running expansion tests on /tmp/fake_test_data.bin (size 20000000)...
...Passed all 2 expansion tests
OK shared libraries are present, ldconfig called properly
OK owns the directories it creates.
OK doesn't own any directories it shouldn't.
OK no duplicates in %files.
OK file permissions are appropriate.
OK no scriptlets present.
OK code, not content.
OK documentation is small, so no -docs subpackage is necessary.
OK %docs are not necessary for the proper functioning of the package.
OK headers are in a separate -devel package.
OK no pkgconfig files.
OK no libtool .la droppings.

Blocker: License field should be GPL+

Suggestions: please consider preserving timestamps of installed files (adding INSTALL="install -p" to make install target in %install section)

(In reply to comment #13)
> 
> Blocker: License field should be GPL+

The included license is GPLv3, and most of the source files don't have a copyright/license notice.  Is this from the src/crc.h which includes code from gzip 1.2.4 and has a license notice?  Downloading gzip 1.2.4 indicates that it is covered by GPLv2.  I guess this is a little more ambiguous than I thought.  I have contacted upstream about this so they can properly clarify what they want.  

> Suggestions: please consider preserving timestamps of installed files (adding
> INSTALL="install -p" to make install target in %install section)

Timestamps are preserved already with install -p; for example, see the build log from my most recent koji scratch build: 
http://koji.fedoraproject.org/koji/getfile?taskID=1159537&name=build.log
Are you seeing a situation where install is called without the -p flag?

(In reply to comment #14)
> The included license is GPLv3, and most of the source files don't have a
> copyright/license notice.  Is this from the src/crc.h which includes code from
> gzip 1.2.4 and has a license notice?
No.
Paragraph 9 of COPYING:

"If the Program does not specify a version number of
this License, you may choose any version ever published by the Free Software
Foundation."

http://fedoraproject.org/wiki/Licensing says:

"A GPL or LGPL licensed package that lacks any statement of what version that
it's licensed under in the source code/program output/accompanying docs is
technically licensed under *any* version of the GPL or LGPL, not just the
version in whatever COPYING file they include."


> I have contacted upstream about this so they can properly clarify what they
> want.  
The best way is to contact upstream to ask the version of the license and add headers to source files or perhaps state this in README file.


> Timestamps are preserved already with install -p; for example, see the build
> log from my most recent koji scratch build: 
> http://koji.fedoraproject.org/koji/getfile?taskID=1159537&name=build.log
> Are you seeing a situation where install is called without the -p flag?
you're right disregard this.

Sorry it's been a while; I've been swamped at work.

Upstream has dealt with the licensing by explicitly making everything GPLv3+.  

New files:
http://matt.truch.net/fedora/slimdata.spec
http://matt.truch.net/fedora/slimdata-2.6.3-2.fc11.src.rpm
http://koji.fedoraproject.org/koji/taskinfo?taskID=1277605

Thanks for the update.
License is GPLv3+

because:

//  Slim is free software: you can redistribute it and/or modify
//  it under the terms of the GNU General Public License as published by
//  the Free Software Foundation, either version 3 of the License, or
//  (at your option) any later version.

rpmlint complains:
slimdata.x86_64: W: unstripped-binary-or-object /usr/lib64/libslim.so.0.0
please set the exec bit on this, this was suggested in comment 1 perhaps you removed it accidentally, please set it back for /usr/lib64/libslim.so.0.0

With a little effort test suite can be made functional. It seems a mostly permissions issue.


Unfortunately name of binary file in this package /usr/bin/slim cannot be used since it conflicts with binary from another package slim (Simple Login Manager).

Also I notice that slimcat and unslim are actually symlinks to slim binary. make install target copies them over to /usr/bin, I do not think there is much sense to copy this file over and over, but symlink them instead.

I've changed the name of the binary to slimdata.  I've discussed this with upstream, and although longer and more boring sounding, they agree that's correct.  Symlinks also made.  

New files:

http://matt.truch.net/fedora/slimdata.spec
http://matt.truch.net/fedora/slimdata-2.6.3-4.fc10.src.rpm
http://koji.fedoraproject.org/koji/taskinfo?taskID=1351775

thank you for the update.

test suite has been fixed, binary name has been renamed and symlinks are created properly.

this package is:

APPROVED.

New Package CVS Request
=======================
Package Name: slimdata
Short Description: Tools and library for reading and writing slim compressed data
Owners: truch
Branches: F-11 F-10 F-9
InitialCC:

There seems to be no FAS account "truch".  Please correct the CVS request and re-set the fedora-cvs flag to '?' when you're ready.

Whoops, it's 'mtruch'.  I should know my own username.  Sorry.

New Package CVS Request
=======================
Package Name: slimdata
Short Description: Tools and library for reading and writing slim compressed
data
Owners: mtruch
Branches: F-11 F-10 F-9
InitialCC:

CVS done.

Building now (or already done) with pushes soon thereafter.