Bug 233782

This requires vegastrike-data, review bug 233783

Note to reviewers, this cannot be build out of the box on Fedora 7 test / devel
/  Rawhide due to boost not being rebuild against the new python 2.5 in devel,
see bug 233523. On i386 you can workaround this by downloading the latest srpm
for boost and rebuilding it, on x86_64 you will need to patch boost before
rebuilding with the patch attached to bug 233523 .

(In reply to comment #2)
> Note to reviewers, this cannot be build out of the box on Fedora 7 test / devel
> /  Rawhide due to boost not being rebuild against the new python 2.5 in devel,
> see bug 233523. On i386 you can workaround this by downloading the latest srpm
> for boost and rebuilding it, on x86_64 you will need to patch boost before
> rebuilding with the patch attached to bug 233523 .

As can be read in bug 233523 Boost-1.33.1-12 which has this fixed should show up
in rawhide soon.

I'd be happy to review this package and (and the data package) for you. I'll
post a full review later today.

Hans, this package fails to build in Mock (Devel/x86_64) during the :

-----
checking for glXGetProcAddressARB... yes
checking for GLU library... yes
checking for GL/gl.h... yes
checking for GL/gl.h... yes
checking for GL/glext.h... yes
checking whether glext.h is recent enough... yes
checking for glut32 library... no
checking for glut library... no
configure: error: GLUT library not found or too old version. 3.7 (beta) or later
required.
error: Bad exit status from /var/tmp/rpm-tmp.74454 (%build)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.74454 (%build)

Error building package from vegastrike-0.4.3-1.fc7.src.rpm, See build log
ending
DEBUG: Executing /usr/sbin/mock-helper umount
/var/lib/mock/fedora-development-x86_64-core/root/proc
DEBUG: Executing /usr/sbin/mock-helper umount
-----

This seems to be a missing BuildRequires on freeglut-devel. 

Yes, it seems so, could you be so kind to add that yourself (I'll fix ir
eventually) and continue with the review. I think there might be more issues
that need fixing and I don't want to go through a zillion iterations, I would
rather have a complete list of issues and fix them all at once.

Thanks!

Peter, any chance we could get some movement with this? If you don't have the
time thats ok, but then please say so, then I can find another reviewer.

I'm incredibly sorry; I completely forgot about these review. (I am SUCH A FART!
>.<)

Thanks for the reminder; I'll get to doing these tomorrow morning. 

Hmm. Hans,

I added "BuildRequires: freeglut-devel" to the spec file and rebuilt the SRPM,
then ran that through mock; and it still fails with that above error. Is there
another package that needs to be set as a dependency?

Alternatively, freeglut is only at version 2.4 in Fedora Development, which
makes this check currently fail. Can it be disabled? 

Thanks.

(In reply to comment #9)
> Hmm. Hans,
> 
> I added "BuildRequires: freeglut-devel" to the spec file and rebuilt the SRPM,
> then ran that through mock; and it still fails with that above error. Is there
> another package that needs to be set as a dependency?
> 

Yes, a couple actually (my bad) here is a fixed version:
Spec URL: http://people.atrpms.net/~hdegoede/vegastrike.spec
SRPM URL: http://people.atrpms.net/~hdegoede/vegastrike-0.4.3-2.fc7.src.rpm

This new version also includes a patch to make it keep the python system dirs in
its import path, so that the builtin dir can be removed from the data package.

Hi Hans. Just so you don't think I've up and vanished or anything (^_^), I've
been very busy with schoolwork and whatnot recently; but I'm working my way
through the review and should finish it within the next couple of days.

Thanks.

Hi, Hans.

Schoolwork has caught up with me quicker than I expected it; and I simply don't
have the time to do the review any longer. Sorry about this. :(

I'd greatly appreciate someone picking up the review from 0.4.3-2 that Hans
posted in comment #10. Moving myself to the CC.

Thanks for your understanding.

I can take over.  Downloading SRPM, SPEC.  Downloading SPEC for vegastrike-data.
 Downloading SROM for vegastrike-daBUFFERING[---40%.....]

Builds.  rpmlint clean with one exception:

W: vegastrike patch-not-applied Patch4: vegastrike-0.4.2-opengl-fix.patch
A patch is included in your package but was not applied. Refer to the patches
documentation to see what's wrong.

Looks like the line including the patch needs to be commented out.

Meets naming guidelines.
Spec name is good.
Meets packaging guidelines. (albeit with some interesting scriptlets)

License is good, but COPYING needs to be included in %doc, along with AUTHORS,
DOCUMENTATION, etc.

Spec is legible American English.
Source does not match upstream tarball, but there is no such thing, and spec
includes tar.bz2 generation steps.  Hence, no URL.
Builds on fc6/i386.
More coming. . .

I think the BRs are OK, but I'm building in mock to test.  Taking awhile. .. 

No locales, shared libraries.
Not relocatable.
No ownership issues evident.
Permissions, defattr OK.
%clean present and correct.
Macros look OK.
No overly large docs.
No header files or static libraries.
No pkgconfig
No versioned .so
No devel package.
No .la.
Desktop file present and handled properly.
No ownership conflicts.
install begins with buildroot wipe.
All UTF-8 filenames.

Mock builds fine.  BRs are OK.  All other MUSTS are met.

Looks like it's just the %doc issue and the patch question.

(In reply to comment #14)
> Looks like the line including the patch needs to be commented out.
> 
It has this comment above it:
# not applied as no longer needed, kept for reference

Commenting it out will exclude it from the srpm and from cvs-import, which is
not what I want.

> License is good, but COPYING needs to be included in %doc, along with AUTHORS,
> DOCUMENTATION, etc.
> 

Good catch will fix.

(In reply to comment #16)
> Mock builds fine.  BRs are OK.  All other MUSTS are met.
> 
> Looks like it's just the %doc issue and the patch question.

See above, here is a new version with the %doc fixed:
http://people.atrpms.net/~hdegoede/vegastrike.spec
http://people.atrpms.net/~hdegoede/vegastrike-0.4.3-3.fc7.src.rpm

Ok.  Sounds reasonable.
APPROVED.

Thanks for the review!!

New Package CVS Request
=======================
Package Name:      vegastrike
Short Description: 3D OpenGL spaceflight simulator
Owners:            j.w.r.degoede
Branches:          FC-6 devel
InitialCC:         <empty>

Imported and build, closing.