Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 191582 - Review Request: xgalaxy - Galaga clone for X11
Summary: Review Request: xgalaxy - Galaga clone for X11
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christopher Stone
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-05-13 09:09 UTC by Hans de Goede
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-06-01 08:29:14 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Hans de Goede 2006-05-13 09:09:53 UTC
Spec URL: http://home.zonnet.nl/jwrdegoede/xgalaga.spec
SRPM URL: http://home.zonnet.nl/jwrdegoede/xgalaga-2.0.34-1.src.rpm
Description:
A clone of the classic game Galaga for the X Window System. Xgalaga is a space-
invader like game with additional features to produce a more interesting game.

Comment 1 Christopher Stone 2006-05-21 20:28:39 UTC
I'm getting a build error in mock during configure:

checking for gcc... gcc
checking whether the C compiler (gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2
-fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic
-fsigned-char -DXF86VIDMODE -lXxf86vm) works... no
configure: error: installation or configuration problem: C compiler cannot
create executables.
error: Bad exit status from /var/tmp/rpm-tmp.65804 (%build)


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


Comment 2 Hans de Goede 2006-05-21 21:01:07 UTC
Well it works fine for me can you lift the actual gcc error from config.log that
might help.


Comment 3 Christopher Stone 2006-05-21 21:55:41 UTC
$ cat
/var/lib/mock/fedora-5-x86_64-core/root/builddir/build/BUILD/xgalaga-2.0.34/config.log
This file contains any messages produced by compilers while
running configure, to aid debugging if configure makes a mistake.

configure:564: checking host system type
configure:588: checking for gcc
configure:701: checking whether the C compiler (gcc -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4
-m64 -mtune=generic -fsigned-char -DXF86VIDMODE -lXxf86vm) works
configure:717: gcc -o conftest -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2
-fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic
-fsigned-char -DXF86VIDMODE  -lXxf86vm conftest.c  1>&5
configure:714: warning: return type defaults to 'int'
/usr/bin/ld: cannot find -lXxf86vm
collect2: ld returned 1 exit status
configure: failed program was:

#line 712 "configure"
#include "confdefs.h"

main(){return(0);}


Comment 4 Christopher Stone 2006-05-21 23:32:33 UTC
* rpmlint output clean
* Package meets Package Naming Guidelines
* Spec filename matches base package %{name}
* Package meets Packaging Guidelines
* Package licensed with open source compatible license
* License in spec matches actual license
* License text included in %doc
* Spec file written in American English
* Spec file is legible
* Sources match upstream
9f7ee685e9c4741b5f0edc3f91df9510  xgalaga_2.0.34.orig.tar.gz
9f7ee685e9c4741b5f0edc3f91df9510  xgalaga_2.0.34.orig.tar.gz
* Package successfully compiles and builds on FC5 x86_64

O Package has all BR except libXxf86vm-devel which I needed to add for it to compile

* Package does not have any locales
* Package does not contain any shared library files
* Package is not relocatable
* Package owns all directories it creates
* Package does not contain any duplicate files in %files
* File permissions are set properly
* Package contains proper %clean section
* Macro usage consistant enough
- I notice you use %{__sed}, but don't bother using %{__make} or %{__rm} etc..
* Package contains permissble content
* Package does not contain large documentation to warrent a seperate package
* Package does not contain header files, libraries or .pc files
* Package does not contain any .so files
* Package does not require or use a -devel package
* Package does not contain any .la files
* Package adds an appropriate .desktop entry
* Package does not own any files or directories owned by other packages


*** MUST ***

- You MUST figure out why FC5 needs to add a BuildRequires of libXxf86vm-devel
and why this is not needed for your build (presumably FC6)


Non-blocking SHOULDs:

- Be more consistant with macro usage, for example %{__sed}, but no %{__rm} etc.
- I also prefer %{buildroot} instead of $RPM_BUILD_ROOT, but that is a matter of
preference.  I just think spec files look cleaner when everything consistantly
uses %{} format.  So basically I'm saying you should use a clean more legible
consistant style in your spec files, but I'm not going to say this is a blocker
or should be fixed, just a suggestion.
- Let me know that the name xgalaga isn't going to be a problem with Namco. 
I've heard the Lgames are not allowed because the names are too close to the
original, is this going to be a problem?
- Return the favor by reviewing some of my packages ;-)


Comment 5 Christopher Stone 2006-05-22 00:00:15 UTC
One other minor thing I noticed:

cat > README.fedora << EOF

The latest Fedora xgalaga package also includes fullscreen support, start
xgalaga with -window to get the old windowed behaviour. You can switch on the
fly between window and fullscreen mode with alt+enter .
EOF

The word "behaviour" is not American English.  It should be "behavior".  In
addition there should not be a space before the final period.

Comment 6 Hans de Goede 2006-05-23 07:59:00 UTC
Chris and I had a private discussion about this by email because BZ was down,
copy and pasting it here for future reference:

---

Hi Chris,

Bugzilla is down so I'm doing it this way. Thanks for the review.

About the missing BR I failed to add that its needed for the devel branch too,
things just worked on my system because I already had the needed devel-package
installed.

About the name, I wans't sure about this myself, so now I've changed the name to
xgalaxy (googled, not taken already).

New SRPM and spec are at:

http://people.atrpms.net/~hdegoede/

Regards,

Hans

---

Christopher Stone wrote:
> okay ill take a look at this tomorrow, been really busy today and
> didnt get the chance to look at it.
>
> Do you think the name is going to be a problem?  I'd prefer xgalaga,
> but then again, it's probably better to be safe than sorry.
>

The name is most likely not a problem, because the people with the rights to the
original name probably don't care. xgalaga has existed under this name for a
long time without trouble.

Then again the name had both me and you worried and those are valid worries the
name is a legal problem. Even if the other party _probably_ doesn't care it
still is a legal issue. It is the _probably_ that scares me and untill the "upto
now" part of upto now this hasn't been a problem. If the people with the rights
to the name one day all of a sudden do start caring, or get a grudge against OSS
we've a problem, which I would rather avoid. Since I've already done the hard
work of renaming (and recreating the "logo") I think its best / safest to stick
with the new name.

Regards,

Hans



Comment 7 Christopher Stone 2006-05-24 01:39:14 UTC
New rpm is STILL missing libXxf86vm-devel.

Comment 8 Hans de Goede 2006-05-24 05:02:17 UTC
Oops you're right, I did put adding it in the changelog, but I didn't actually
do this.

Fixed SRPM and spec are at:

http://people.atrpms.net/~hdegoede/



Comment 9 Hans de Goede 2006-06-01 08:29:14 UTC
Imported & Build,

Thanks!


Comment 10 Christopher Stone 2006-06-01 19:33:58 UTC
Fixing bug report summary.


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