Bug 427718

Summary: Review Request: Megazeux - Text-character driven game creation system
Product: [Fedora] Fedora Reporter: Steve Salevan <ssalevan>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, huzaifas, mtasaka, notting
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-06-20 00:01:44 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 201449    

Description Steve Salevan 2008-01-06 21:50:43 EST
Spec URL: http://www.thekobayashimaru.org/mzx/megazeux.spec
SRPM URL: http://www.thekobayashimaru.org/mzx/megazeux-2.8.1-h.src.rpm
Description:
Megazeux is a text-character driven game creation system with support
for sound and music, and an object-oriented programming language
known as Robotic.  Due to the unique flexibilities of this language, a vast
array of games utilizing the Megazeux engine have been created.

This is my first submission (and I'm still a little fresh at composing RPMs), so I'm seeking a sponsor to review my work.  Many thanks!
Comment 1 Huzaifa S. Sidhpurwala 2008-01-06 23:21:58 EST
Hi,
I cannot sponsor you, i am just doing a first review of your package.

[root@dhcp1-17 ~]# rpmlint megazeux-2.8.1-h.src.rpm 
megazeux.src:35: W: rpm-buildroot-usage %prep rm -rf $RPM_BUILD_ROOT
megazeux.src:37: W: setup-not-quiet
megazeux.src: E: no-cleaning-of-buildroot %install
megazeux.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 3)
megazeux.src: W: non-standard-group Applications/Games
megazeux.src: W: invalid-license GPL

Some hints for the non very obvious ones:
1. Replace setup with setup -q
2. For %install, it is advised that you start with a clean build root.
3. Licensing:  http://fedoraproject.org/wiki/Packaging/LicensingGuidelines
Comment 2 Steve Salevan 2008-01-07 00:03:17 EST
Thanks very much for the heads-up.  I fixed these issues, and now rpmlint runs
cleanly through the new src RPM, available from the original link.
Comment 3 Huzaifa S. Sidhpurwala 2008-01-08 06:27:26 EST
Just a couple of thoughts:

1. You need to follow Source URL (macro using) as
   http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
  check more on it http://fedoraproject.org/wiki/Packaging/SourceURL

2. Usage of make should be as per:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-8b14098227aebff1cf6188939e9d0877295ac448

3. 
Comment 4 Mamoru TASAKA 2008-01-21 13:15:03 EST
Hello. 
I just glanced at your spec file but it seems there are not
a few issues to be fixed.
For general packaging guidelines you can refer to:

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

* Please make summary more easy to grasp.
* Please consider to use disttag.
  http://fedoraproject.org/wiki/Packaging/DistTag
* Don't use Epoch unless necessary.
* Source must be given with full URL.
  http://fedoraproject.org/wiki/Packaging/SourceURL
* Please explain why you limit the BuildArch to only i386.
* Remove "Vendor" and "Distribution"
* BuildRoot tag does not meet Fedora packaging guidelines
* gcc is redundant for BuildRequires
* I guess all Requires you are writing now are not needed
  and should be removed.
  rpmbuild checks the dependency for libraries and adds the
  detected requires automatically to binary rpms.
* Please make sure that fedora specific compilaton flags
  are correctly honored.
  - Also, please make build log more verbose so that we can
    easily check if compilation flags are correctly used
  - Stripping binaries by yourself (or by "make" process") 
    is forbidden to create debuginfo rpm correctly.
* Please support parallel make if possible.
* To keep timestamps, please add "-p" option when using
  "cp" or "install" commands.
* Don't fix owners/permission on %post.
  This must be done by %install ends, or if it can't this
  must be treated by appropriate %attr in %files.
* Would you explain why you don't want %config(noreplace)?
* We now recommend %defattr(-,root,root,-)
Comment 5 Mamoru TASAKA 2008-01-21 13:16:48 EST
Also, please change the release number of spec file every time
you modify your spec file to avoid confusion.
Comment 6 Steve Salevan 2008-01-24 20:52:41 EST
Thanks very much for your comments and suggestions.  I've replaced the spec file
with one that should hopefully correspond much more closely to Fedora packaging
guidelines, per the points you recommended.  Additionally, the new source RPM
URL is:

http://www.thekobayashimaru.org/mzx/megazeux-281h-1.src.rpm

I've included a comment in the spec file as to why I don't use
%config(noreplace), but the long and short of it is that the config.sh file that
comes with Megazeux is homegrown, not produced with distutils, and requires its
own particular flags; hence, I have to invoke it in a kludgier fashion. 
Hopefully they'll fix that in the next release.
Comment 7 Mamoru TASAKA 2008-01-25 09:55:26 EST
* Please consider to use disttag.
  http://fedoraproject.org/wiki/Packaging/DistTag* Rebuild failed.

* Rebuild failed
  http://koji.fedoraproject.org/koji/taskinfo?taskID=372587
  - It seems that at least libpng-devel is missing from BuildRequires.
  - build.log says:
----------------------------------------------------------------
X11 could not be queried, disabling.
----------------------------------------------------------------
    Does this mean that some X releated development pacakges are
    required for BuildRequires?

* BuildRoot tag does not meet Fedora guidelines yet.
  Please check the section "BuildRoot tag" of
  http://fedoraproject.org/wiki/Packaging/Guidelines

* Please make build log more verbose. The output like
-----------------------------------------------------------------
+ make -j8
  CC       contrib/gdm2s3m/src/error.c
  CC       contrib/gdm2s3m/src/gdm.c
  CC       contrib/gdm2s3m/src/s3m.c
------------------------------------------------------------------
  is not useful and we cannot check if fedora specific compilation
  flags are correctly honored.

* ChangeLog entry should be like:
------------------------------------------------------------------
* Thu Jan 24 2008 Steve Salevan <ssalevan@redhat.com> 281h-2
------------------------------------------------------------------

! Please increase the release number of your spec file when
  you modify your spec file to avoid confusion.
Comment 8 Mamoru TASAKA 2008-02-04 02:30:37 EST
ping?
Comment 9 Mamoru TASAKA 2008-02-12 07:03:31 EST
ping again?
Comment 10 Steve Salevan 2008-02-12 13:31:47 EST
Pong...  I apologize for the delay; schoolwork's been dominating my life as of
late.  I should have a new spec up by this evening.
Comment 11 Alexander Kahl 2008-03-23 09:04:30 EDT
URL: contains a typo, "http://megazeux.sorceforge.net/" should (most probably)
be "http://megazeux.sourceforge.net/" (missing 'u')
Comment 12 Mamoru TASAKA 2008-03-26 08:58:18 EDT
Well, what is the status of this bug?
Comment 13 Mamoru TASAKA 2008-04-12 07:33:09 EDT
ping again?
Comment 14 Mamoru TASAKA 2008-06-09 13:46:37 EDT
ping again?
Comment 15 Jason Tibbitts 2008-06-20 00:01:44 EDT
Why on earth is this ticket private? 

In any case, it's been way too long without any response.  Closing.