[Bug 193312] Review Request: <DevIL: A cross-platform image library>
bugzilla at redhat.com
bugzilla at redhat.com
Sun May 28 13:40:36 UTC 2006
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Review Request: <DevIL: A cross-platform image library>
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=193312
------- Additional Comments From j.w.r.degoede at hhs.nl 2006-05-28 09:33 EST -------
MUST:
=====
* rpmlint output clean
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License (LGPL) ok, license file included
* spec file is legible and in Am. English.
* Source matches upstream
* Compiles and builds on devel-x86_64
* BR: redundant (see below)
* No locales
* ldconfig properly called for shared libraries
* Not relocatable
* Package owns / or requires all dirs
* No duplicate files (except for %doc) & Permissions ok
* %clean & macro usage OK
* Contains code only
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* -devel package as needed
* .a and .la files removed
* no gui -> no .desktop file required
MUST fix:
=========
* (Build)Requires: xorg-x11-devel . This is for non modular X, your package will
however be initially imported and build for the -devel branch which uses
modular X. Luckily this can be easily fixed:
-The BR can be dropped completly, see should fix below.
-The Requires for the -devel package should be:
Requires: allegro-devel libGL-devel libGLU-devel
Because GL/gl.h and GL/glu.h are the "X11" headers used by DevIL's headers.
libGL-devel and libGlu-devel are virtual provides and should work on FC4-devel
* Don't include the same docs in the -devel package again -devel Requires the
main package and as such the docs will already be available. I know this makes
rpmlint issue a warning, which should be ignored.
* DevIL-devel contains an autoconf generated config.h, this is a problem,
because if other packages contain one too then there will be #define colissions
and other unpleasant-ness. I've checked things and this file is used by, ilut.h
which checks the following defines:
#define ILUT_USE_ALLEGRO
#define ILUT_USE_OPENGL
#define ILUT_USE_SDL
The easiest and best way to fix this is to replace config.h with a file
containing just these 3 lines and nothing else at the end of %install. An
alternative would be to completly remove config.h and patch ilut.h to include
these 3 lines instead of the #include IL/config.h line. But that could break
compilation of software which wants to include IL/config.h directly.
Other software building against DevIL, might check for some the other defines
in config.h, but then that software is broken and we will need to fix the
other software as we go. I don't think that is a very likely scenario though.
* In %install you manually install ilu_internal.h because that is needed by
ilu_region.h, this is however not the correct fix, the correct fix is to
patch or sed ilu_region.h to include IL/il.h instead of ilu_internal.h as il.h
contains all the nescesarry things ilu_region.h needs.
Should fix:
===========
* In %changelog you write:
- Made zlib-devel and xorg-x11-devel explicit buildrequires
Please don't unless you've got a very good reason for this, this goed directly
against the Packaging Guidelines!
* Add an all lowercase same name Provides to both the base and sub-package as
discussed on f-e-l (this is a should fix because it isn't in the guidelines
yet, but it will most likely be in the guidelines soon. Example for the base:
Provides: devil = %{version}-%{release}
* Change the Source0 URL to:
http://download.sourceforge.net/openil/%{name}-%{version}-RC1-src.tar.gz
This is the generic sf donwload site which is prefered to using a specific
mirror as you have, mirrors sometimes come and go.
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.
More information about the Fedora-package-review
mailing list