Bug 201198 - Review Request: alleggl - OpenGL support library for Allegro
Review Request: alleggl - OpenGL support library for Allegro
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ian Chapman
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-08-03 11:10 EDT by Hans de Goede
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-08-10 16:59:55 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Build log in mock (failure result) (31.24 KB, text/plain)
2006-08-03 21:43 EDT, Mamoru TASAKA
no flags Details

  None (edit)
Description Hans de Goede 2006-08-03 11:10:30 EDT
Spec URL: http://people.atrpms.net/~hdegoede/alleggl.spec
SRPM URL: http://people.atrpms.net/~hdegoede/alleggl-0.4.0-0.1.rc4.src.rpm
Description:
AllegroGL is an Allegro add-on that allows you to use OpenGL alongside Allegro.
You use OpenGL for your rendering to the screen, and Allegro for miscellaneous
tasks like gathering input, doing timers, getting cross-platform portability,
loading data, and drawing your textures. So this library fills the same hole
that things like glut do.

AllegroGL also automatically exposes most, if not all, OpenGL extensions
available to user programs. This means you no longer have to manually load
them; extension management is already done for you.
Comment 1 Hans de Goede 2006-08-03 11:28:37 EDT
Hmm, I just noticed that this installs an autoheader generated header, which is
BAD! I'll whip up a fix for this.
Comment 2 Hans de Goede 2006-08-03 13:55:27 EDT
New version which has the autoheader stuff fixed here:
Spec URL: http://people.atrpms.net/~hdegoede/alleggl.spec
SRPM URL: http://people.atrpms.net/~hdegoede/alleggl-0.4.0-0.2.rc4.src.rpm
Comment 3 Mamoru TASAKA 2006-08-03 21:43:15 EDT
Created attachment 133604 [details]
Build log in mock (failure result)

========== I cannot formally review your package ==========

However rebuilding this package in mock fails on i386.
See the attachment.
Comment 4 Hans de Goede 2006-08-04 13:01:36 EDT
Next time please attach:
/var/lib/mock/fedora-development-x86_64-core/root/builddir/build/BUILD/xxx/config.log
too when a mock build fails because of a ./configure failure.

I've done a mockbuild myself on my painfull slow internet connection and now
have this fixed, it was missing BRs for libXext-devel and libXpm-devel.

New version here:
Spec URL: http://people.atrpms.net/~hdegoede/alleggl.spec
SRPM URL: http://people.atrpms.net/~hdegoede/alleggl-0.4.0-0.3.rc4.src.rpm

Comment 5 Hans de Goede 2006-08-04 13:03:41 EDT
p.s.

I forgot to mention that I'll be on a short vacation till wednesday, so don't
expect any response from me till then.
Comment 6 Ian Chapman 2006-08-10 13:39:40 EDT
* rpmlint: no problems
* Package named correctly: Yes
* Patches named correctly: N/A
* Spec file named correctly: Yes
* Licence field matches: Yes
* Licence file installed: No (OK but should query upstream)
* Spec file in American English: Yes
* Source matches upstream: Yes
* Locales use %find_lang: N/A
* Contains %clean: Yes
* Specfile legible: Yes
* Compiles and builds ok: Yes (mock: fc5/ppc)
* Calls ldconfig in %post/%postun for shlibs: Yes
* Owns directories it creates: Yes
* Duplicate files: No
* Permission set correctly: Yes
* Consistent macro use: Yes
* %doc affects runtime: No
* Headers and static libs in -devel: Yes
* .pc files in -devel: N/A
* .so in -devel: Yes
* -devel requires base: Yes
* Contains .la files: No
* Owns files it didn't create: No
* .desktop files installed correctly: N/A

1. As it needs allegro >= 4.2.0, i suggest putting a version dependency on 
allegro-devel for both the base and -devel packages

2. As faq.txt contains information exclusively of interest to developing/
building with alleggl I suggest moving it to -devel

3. readme.txt is primarily build instructions, I suggest excluding it.

4. In -devel, the *.hhk *.hhc and *.hhp files are Microsoft Windows HTML Help 
files if I remember right, probably of no interesting in Linux, I would exclude 
them.

Otherwise it's cool.
Comment 7 Hans de Goede 2006-08-10 15:35:46 EDT
(In reply to comment #6)
> 1. As it needs allegro >= 4.2.0, i suggest putting a version dependency on 
> allegro-devel for both the base and -devel packages
> 
I would rather not, tihs package is targeted soley at FC-5 and higher, so this
is not nescesarry. Versioned dependencies IMHO only give a false sense of
security, since often they are never updated after the initial specfile creation
even if upstreams requirements change.

> 2. As faq.txt contains information exclusively of interest to developing/
> building with alleggl I suggest moving it to -devel
> 
I was thinking along the same lines when creating the package, but I wasn't sure
since you agree I'll move it to devel.

> 3. readme.txt is primarily build instructions, I suggest excluding it.
> 
I was thinking along the same lines (again) when creating the package, but I
wasn't sure since you agree I'll remove it.

> 4. In -devel, the *.hhk *.hhc and *.hhp files are Microsoft Windows HTML Help 
> files if I remember right, probably of no interesting in Linux, I would
> exclude them.
Agreed, done.

Thanks for the review!, new version here:
Spec URL: http://people.atrpms.net/~hdegoede/alleggl.spec
SRPM URL: http://people.atrpms.net/~hdegoede/alleggl-0.4.0-0.4.rc4.src.rpm
Comment 8 Ian Chapman 2006-08-10 16:21:16 EDT
 > I would rather not, tihs package is targeted soley at FC-5 and higher, so 
this
> is not nescesarry. Versioned dependencies IMHO only give a false sense of
> security, since often they are never updated after the initial specfile > 
creation
> even if upstreams requirements change.

No problem. I prefer versioned dependencies if the source makes a specific 
request for a version but for different reasons :)

Looks good Hans, Approved!
Comment 9 Hans de Goede 2006-08-10 16:59:55 EDT
Thanks!

Imported & build, closing.

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