Bug 309061

Well, I must say that there are some issues to fix
on 1.2.1-1 spec/srpm. Please check the following URLs 
for general packaging procedure.

http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/DistTag

A. Description stage
* Please explain why you want to introduce seemingly-redundant
  macros like %name, %version, ..... etc
* Please consider to use %?dist tag.
* Please follow the BuildRoot format requested for Fedora.
* Remove redundant BuildRequires
  - For example, libGLU-devel requires libGL-devel, so
    "BuildRequires: libGL-devel" can be removed
  - Remove BuildRequires listed on "Exceptions" list
    on "Guidelines" wiki.
  - Also I suggest to write one BuildRequires per one line
    because
    * it makes easier to read
    * it makes easier to find out the difference when BuildRequiers
      changes.
* Check the missing BuildRequires
  - This srpm cannot be rebuilt.
    http://koji.fedoraproject.org/koji/taskinfo?taskID=191544
    At least libogg-devel is missing from BuildRequires.

B. %prep/%build/%install stage
* Support parallel make if possible, otherwise write as
  a comment which tells that this package cannot support
  parallel make.
* Don't use %makeinstall unless it cannot be avoided.
* Desktop files must be installed by "desktop-file-install"
  (BuildRequires: desktop-file-utils is needed).
  - I don't think the Category 
    "X-MandrivaLinux-MoreApplications-Games-Arcade" is needed.
  - The category "MoreApplications" is not valid.
-----------------------------------------------------------
error: value
"Game;ActionGame;MoreApplications;X-MandrivaLinux-MoreApplications-Games-Arcade;"
for key "Categories" in group "Desktop Entry" contains an unregistered value
"MoreApplications"
-----------------------------------------------------------
* When using "cp" or "install" command, add "-p" option
  to keep timestamps.
* Consider to install pixmaps image data under
  %_datadir/icons/hicolor/??x??/apps and call gtk-update-icon-cache
  (please check the section "GTK+ icon cache" of
   http://fedoraproject.org/wiki/Packaging/ScriptletSnippets )

C. %files section
* Make it sure that all directories created by this package
  are owned by this package.
  For example, %_datadir/adanaxisgpl/ itself is not owned by
  any package.

Thanks for the review.  I'll get on to those fixes as soon as I can.

Here are the updated .spec and SRPM:

Spec URL: http://www.mushware.com/files/adanaxisgpl-1.2.3-1.fc7.spec
SRPM URL: http://www.mushware.com/files/adanaxisgpl-1.2.3-1.fc7.src.rpm

Thanks again.

For 1.2.3-1:

* Source tarball
  - Tarball in your srpm does not match the tarball downloaded
    from upstream:
-------------------------------------------------------
20015891 2007-10-19 08:24 adanaxisgpl-1.2.3-1.fc7/adanaxisgpl-1.2.3.tar.gz
20015932 2007-10-19 09:32 adanaxisgpl-1.2.3.tar.gz

4a199ac0da9468d6b622af53d2189015  adanaxisgpl-1.2.3-1.fc7/adanaxisgpl-1.2.3.tar.gz
2a0b49cb7c2b13b6272420983279288f  adanaxisgpl-1.2.3.tar.gz
-------------------------------------------------------

* Redundant BuildRequires
  - Still some BuildRequires are redundant.
    autoconf, automake - Please explain why these are needed.
                         This spec file does not call autoconf or automake.
    binutils          <- gcc requires this
    glibc-devel       <- gcc requires this
    libGLU-devel      <- freeglut-devel requires this
    SDL-devel         <- SDL_mixer-devel requires this

* Timestamp
  - To keep timestamps, please use the following
--------------------------------------------------------
make DESTDIR=%{buildroot} INSTALL="install -p" CPPROG="cp -p" install
--------------------------------------------------------
    For recent Makefiles, adding 'INSTALL="install -p"' will usually
    save timestamps on installed files. However as this Makefile
    uses install-sh, 'CPPROG="cp -p"' is also needed.

Then:
-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on:
http://fedoraproject.org/PackageReviewStatus/NEW.html
(NOTE: please don't choose "Merge Review")


Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------

ping?

I've been a bit busy recently, but I should be able to find some time to
repackage this weekend.  I will also do some pre-reviews as you suggest when
time permits, but it might take a week or two.  Will get back to you soon...

New .spec and SRPM:

Spec URL: http://www.mushware.com/files/adanaxisgpl-1.2.4-1.fc7.spec
SRPM URL: http://www.mushware.com/files/adanaxisgpl-1.2.4-1.fc7.src.rpm

For 1.2.4-1:

* ICE dependency
  - Please check if the following result in build.log is
    correct.
----------------------------------------------------------------
   193  checking for shmat... yes
   194  checking for IceConnectionNumber in -lICE... no
   195  checking for main in -lm... yes
----------------------------------------------------------------

* Ruby dependency
  - By the way, is ruby not needed for this package?

! Still I am waiting for your pre-review or another review request
  submission.

I've had a look a the ICE dependency and I don't think it's a problem.  It looks
like one of the macros - maybe AC_PATH_XTRA - is looking for X libraries, but
the app doesn't use anything from libICE directly.  I've tried the binary from
the mock build (i.e. without libICE) and it runs fine.

There's no ruby dependency because the game source contains a modified version
of the ruby source code, and doesn't refer to installed extensions.  The game
runs happily without it.  I'll get the pre-review done as soon as I can.

I have done a pre-review:

https://bugzilla.redhat.com/show_bug.cgi?id=366841

(In reply to comment #10)
> I have done a pre-review:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=366841
> 

Looks good, and thanks I had that on my todo list to review. Note that I'm a
potential candidate to sponsor you to, just like Mamoru, but before doing so I
would like to see one more pre-review, or another package submitted by you and
reviewed.

About your last version, you say:
"There's no ruby dependency because the game source contains a modified version
of the ruby source code, and doesn't refer to installed extensions.  The game
runs happily without it.  I'll get the pre-review done as soon as I can."

Note that its much prefered to always use the system version of any libs / tools
instead of your own private copy, that way there is only one place to take into
account when adding bugfixes / security fixes.

What exactly has been modified, and is it possible (with a reasonable effort) to
use the system version of ruby instead?

Hi,

I agree that it's best to use the installed ruby .so if you can, but in this
case normal ruby isn't ideal.  The philosophical problem is that the security
model is wrong for level scripting (where the scripts might be hostile) as it's
aimed at web sites (where everything except the scripts might be hostile).  The
practical problem is that game relies on the way this version of ruby does
things a bit too much.  From a developer point of view it's useful to know
exactly what code you're dealing with on all platforms (Windows, Mac OS X,
Linux, etc.) to reduce the number of test configurations.  Security problems are
less of an issue because the game never opens a socket.  In practice what I'll
do is pull later versions of ruby into the upstream as necessary, test it, and
then re-release.

Ok, thats a reasonable explanation, in that case using an included copy of ruby
is fine. So that only leaves one more package reviewed, or one other good pre
review to get you sponsored.

I've done another pre-review:

https://bugzilla.redhat.com/show_bug.cgi?id=361701

Well,
- When I launch adanaxisgpl from xterm, I get the following warning.
  Is this okay?
--------------------------------------------------------
mkdir /usr/share/adanaxisgpl/mushware-cache failed: Permission denied
--------------------------------------------------------

- Umm.. Currently rawhide mesa seems a heavy mess and when I launch
  adanaxisgpl no characters are displayed correctly...

Yep, the mkdir message is normal, and if fully-featured transparency isn't
working in your mesa you will get blocks instead of letters.

Okay.

------------------------------------------------------------
    This package (adanaxisgpl) is APPROVED by me
------------------------------------------------------------

Please follow the procedure according to:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account".
At a point a mail should be sent to sponsor members which notifies
that you need a sponsor (at the stage, please also write on
this bug for confirmation that you requested for sponsorship)
Then I will sponsor you.

If you want to import this package into Fedora 7 or Fedora 8, 
you also have to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on Fedora rebuilding system).

If you have questions, please ask me.

If you have already requested cvsextras group, please write your
FAS (Fedora Account System) name in this bug.

Thanks!

I've created a fedora account with the username southa, and I confirm that I 
am requesting sponsorship.

Now I should be sponsoring you.

New Package CVS Request
=======================
Package Name: adanaxisgpl
Short Description: FPS in 4 spatial dimensions
Owners: southa
Branches: F-7 F-8
InitialCC: 
Cvsextras Commits: yes

cvs done.

Seems to build without problems:

http://koji.fedoraproject.org/koji/buildinfo?buildID=25640
http://koji.fedoraproject.org/koji/buildinfo?buildID=25641
http://koji.fedoraproject.org/koji/buildinfo?buildID=25633

I've closed this bug as NEXTRELEASE.  Thanks all!

adanaxisgpl-1.2.4-1.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update adanaxisgpl'

adanaxisgpl-1.2.4-1.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update adanaxisgpl'

adanaxisgpl-1.2.4-1.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

adanaxisgpl-1.2.4-1.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.