Michael,

I'm hoping that you're willing to review this hence the CC, this is a dependency
for a game: http://home.exetel.com.au/tjaden/raidem/
Which I'm busy packaging. Its turning out to be quite a pain. It needs a lot of
libs. To be fair it does come with all these libs (including a few already in
FE) in the source package, but I want todo this right and thus package the libs
seperatly so that they can be used later for other games too.

Its late (bedtime) overhere now, so I'll take a look at your Doom work tommorow.

Thanks.

MUST
====
* Source tarball matches upstream
  aa71fbe7661e56421cab42e6bca70c7c  adime-2.2.1.tar.gz
* Package (and .spec) named properly
* License file (zlib) included
* Spec file readable, in Am. English
* Compiles and builds cleanly on FC-4 x86_64
* No locale files
* No excessive BR: or Requires:
* ldconfig used in %post/%postun as is necessary
* Not relocatable
* Does not create any directories that it needs to own
* buildroot cleaned up in %install and %clean
* No duplicate files
* permissions ok
* Macro usage consistent
* Contains code, not content
* headers, api docs, and .so library in -devel
* No .la archives
* No desktop file needed

SHOULD
======
* Packages builds in mock (FC4-i386, FC4-x86_64, FC5-i386)
* Did not test the package as I don't have anything useful to build against
  it.

NONBLOCKING
===========
* License is actually the zlib license, which is very similar to BSD.
  However, since the zlib package itself claims a BSD license (rpm -qi zlib),
  this is not a blocker.

MUSTFIX
=======
* "make docs" generates warnings and fails to build the html/rtf documentation
  because the $srcdir/docs/html and $srcdir/docs/rtf directories are missing.
  I suggest adding the following to %prep to suppress these warnings:
    mkdir docs/html
    mkdir docs/rtf
  ...and including docs/html and docs/rtf in %doc

* BuildRequires: texinfo is needed to pull in 'makeinfo' to build the
  info files.  mock builds fail without this.

* rpmlint complains on i386 packages:
  E: adime shlib-with-non-pic-code /usr/lib/libadime.so.0
  W: adime-debuginfo objdump-failed objdump:
/tmp/adime-debuginfo-2.2.1-1.fc4.i386.rpm.30986/usr/lib/debug/usr/lib/libadime.so.0.debug:
Invalid operation
  ...and on x86_64 packages:
  W: adime-debuginfo objdump-failed
  E: adime-devel only-non-binary-in-usr-lib

Many thanks, and sorry I only ran rpmlint on the mainpackage (x86_64).


NONBLOCKING
===========
* License is actually the zlib license, which is very similar to BSD.
  However, since the zlib package itself claims a BSD license (rpm -qi zlib),
  this is not a blocker.

I actually had "zlib License" as License tag first, which is for example what
mandriva uses and rpmlint upstream accepts. Our rpmlint package however
overrides the list of valid licenses used by upstream and does not include zlib
hence the change to BSD, which is indeed what zlib uses too. So I can either
file an RFE against rpmlint to add "zlib License" or leave it as is, waht do you
think?


MUSTFIX
=======
* "make docs" generates warnings and fails to build the html/rtf documentation
  because the $srcdir/docs/html and $srcdir/docs/rtf directories are missing.
  I suggest adding the following to %prep to suppress these warnings:
    mkdir docs/html
    mkdir docs/rtf
  ...and including docs/html and docs/rtf in %doc

Done.


* BuildRequires: texinfo is needed to pull in 'makeinfo' to build the
  info files.  mock builds fail without this.

Done.


* rpmlint complains on i386 packages:
  E: adime shlib-with-non-pic-code /usr/lib/libadime.so.0
Ah an actual problem found by rpmlint how nice (sorry bit sarcastic, see below),
I fixed this by not using allegro-config --libs as that drags in
-lalleg_unshareable, which is non-PIC and thus should only be in the application
and not in a .so file. This means all applications which use us must be linked
with -Wl,--export-dynamic -lalleg_unsharable (or `allegro-config --libs`)


  W: adime-debuginfo objdump-failed objdump:
/tmp/adime-debuginfo-2.2.1-1.fc4.i386.rpm.30986/usr/lib/debug/usr/lib/libadime.so.0.debug:
Invalid operation

rpmlint bug, see bug 185227



  ...and on x86_64 packages:
  W: adime-debuginfo objdump-failed
Same rpmlint bug 185227


  E: adime-devel only-non-binary-in-usr-lib
And another rpmlint bug, what fun see bug 185228


New attemp (get the src.rpm, the patches changed too) at:
Spec Name or Url: http://home.zonnet.nl/jwrdegoede/adime.spec
SRPM Name or Url: http://home.zonnet.nl/jwrdegoede/adime-2.2.1-2.src.rpm

I noticed that it was missing a "Requires: allegro-devel" for the -devel
package.  I've fixed this and oploaded updated versions to the 2 files linked to
in comment 3.

(In reply to comment #3)
> NONBLOCKING
> ===========
> * License is actually the zlib license, which is very similar to BSD.
>   However, since the zlib package itself claims a BSD license (rpm -qi zlib),
>   this is not a blocker.
>
> I actually had "zlib License" as License tag first, which is for example what
> mandriva uses and rpmlint upstream accepts. Our rpmlint package however
> overrides the list of valid licenses used by upstream and does not include zlib
> hence the change to BSD, which is indeed what zlib uses too. So I can either
> file an RFE against rpmlint to add "zlib License" or leave it as is, waht do you
> think?

I prefer to have the License: tag reflect the actual license as best as it can.
On my FC-4 system /usr/share/rpmlint/TagsCheck.py claims that "zlib License" is
a valid license, but /etc/rpmlint/config doesn't.  It's a valid GPL compatible
license, so rpmlint should be configured to accept it.  Change the License:
tag and file a RFE against rpmlint.

> MUSTFIX
> =======
[...]
> * rpmlint complains on i386 packages:
>   E: adime shlib-with-non-pic-code /usr/lib/libadime.so.0
> Ah an actual problem found by rpmlint how nice (sorry bit sarcastic, see below),
> I fixed this by not using allegro-config --libs as that drags in
> -lalleg_unshareable, which is non-PIC and thus should only be in the application
> and not in a .so file. This means all applications which use us must be
linked> with -Wl,--export-dynamic -lalleg_unsharable (or `allegro-config --libs`)

Now the Makefile has a hardcoded version for the allegro library:
LIB_FLAGS = -lalleg-4.2.0
...making it more sensitive to changes in the allegro version, and also
breaking the build on FC-4.

You could try stripping the offending -lalleg_unsharable flag in the
Makefile:

LIB_FLAGS= $(shell allegro-config --libs | sed 's/-lalleg_unsharable//')

Created attachment 126035 [details]
New so-fixes patch without hardcoded allero version

Good call,

Attached is a new version of so-fixes patch, sorry no new SRPM as I'm at work
and I don't have upload access to my homepage from here.

Created attachment 126040 [details]
spec with updated license, release and changelog

Here is an updated spec with the requested license change and a changelog entry
for the modified so-fixes patch.

The new spec/patch look good.  I'd prefer a final SRPM before giving approval,
but given your ISP issues that's not necessary this time.

APPROVED

Imported and Build, thanks!