This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 185215 - Review Request: adime - Allegro Dialogs Made Easy
Review Request: adime - Allegro Dialogs Made Easy
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Wart
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT 185257
  Show dependency treegraph
 
Reported: 2006-03-11 17:50 EST by Hans de Goede
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-03-13 16:51:04 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
New so-fixes patch without hardcoded allero version (1.37 KB, patch)
2006-03-13 08:22 EST, Hans de Goede
no flags Details | Diff
spec with updated license, release and changelog (2.76 KB, text/plain)
2006-03-13 09:57 EST, Hans de Goede
no flags Details

  None (edit)
Description Hans de Goede 2006-03-11 17:50:35 EST
Spec Name or Url: http://home.zonnet.nl/jwrdegoede/adime.spec
SRPM Name or Url: http://home.zonnet.nl/jwrdegoede/adime-2.2.1-1.src.rpm
Description:
Adime is a portable add-on library for Allegro with functions for generating
Allegro dialogs in a very simple way. Its main purpose is to give as easy an
API as possible to people who want dialogs for editing many kinds of input
data.
Comment 1 Hans de Goede 2006-03-11 17:57:04 EST
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.
Comment 2 Wart 2006-03-11 19:52:01 EST
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
Comment 3 Hans de Goede 2006-03-12 03:50:01 EST
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
Comment 4 Hans de Goede 2006-03-12 16:17:28 EST
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.
Comment 5 Wart 2006-03-12 17:39:10 EST
(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//')
Comment 6 Hans de Goede 2006-03-13 08:22:58 EST
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.
Comment 7 Hans de Goede 2006-03-13 09:57:09 EST
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.
Comment 8 Wart 2006-03-13 14:02:31 EST
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
Comment 9 Hans de Goede 2006-03-13 16:50:23 EST
Imported and Build, thanks!

Note You need to log in before you can comment on or make changes to this bug.