Bug 443238 - Review Request: cave9 - 3d clone of SFCave.
Summary: Review Request: cave9 - 3d clone of SFCave.
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
low
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-04-19 16:38 UTC by Victor Bogado
Modified: 2008-12-15 23:40 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-12-15 23:40:00 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Victor Bogado 2008-04-19 16:38:17 UTC
Spec URL: http://bogado.net/rpm/cave9.spec
SRPM URL: http://bogado.net/rpm/cave9-0.2.1-1.bog9.src.rpm
Description: Cave9 is a gravity cave-exploration game.

Comment 1 Victor Bogado 2008-04-19 16:44:25 UTC
That may be a problem with the font file, the font used by the game is free for
personal and non-commercial uses. Since this is incompatible with fedora
guidelines, the font is not bundled in the package. 

The problem is that since the font file is in the pristine source of the game it
gets into the src.rpm file. Not sure if this is a problem or not. 

Also to change the font I used an already installed font, linked to the position
that the game search for the font file and putted the font file as a dependency.
Not sure if this is the best way (maybe a patch in the game should be better?). 

Comment 2 Victor Bogado 2008-04-19 16:52:14 UTC
My packages are revisioned using git at gitorious: 

http://git.gitorious.org/specs-for-fedora/mainline.git

Comment 3 Victor Bogado 2008-04-19 20:57:30 UTC
I submitted for revision. Now this package is honouring fedora compiler flags
and has a desktop file. 

The new spec and srpm files are : 

http://bogado.net/rpm/cave9.spec
http://bogado.net/rpm/cave9-0.2.1-2.bog9.src.rpm


Comment 4 Mamoru TASAKA 2008-04-27 16:07:20 UTC
(Removing needsponsor: I am sponsoring the submitter)

Comment 5 Mamoru TASAKA 2008-07-24 17:16:02 UTC
Victor, are you still interested in this package? If so I will try
to review this package.

Comment 6 Victor Bogado 2008-07-24 17:31:52 UTC
Yes! :-) 

But let me do a clean-up first, I think the upstream have a data package that
fits better on free distribution. The font on this package is not free, and even
though I have removed it from the rpm file, the source package still has it.

Comment 7 Mamoru TASAKA 2008-09-10 07:26:28 UTC
Any updates here?

Comment 8 Victor Bogado 2008-09-10 11:10:46 UTC
Not yet, I am in contact with the upstream developers, they seem to have a version of the data that is completely free, so I am waiting to package that data instead of the one packaged here that has a font that is non-free.(*)

The non-free font now does not get into the final package, but it does shows in the src package, since it is in the pristine data-package. Since I am not lawyer I can't know if this is okay or not, but my common sense says that it is not and a free font is better anyway. 

I will talk to Rodolfo, the mainstream developer, today if this don't walk I will package a set of free data, that I was making.

Comment 9 Victor Bogado 2008-09-15 21:39:08 UTC
Finally!! Development, there are a new version of the srpm and spec file with the verision 0.3 that cointains free data. :)

the location of the new packages are the following: 

http://bogado.net/rpm/cave9-0.3-1.bog9.src.rpm
http://bogado.net/rpm/cave9.spec

Comment 10 Mamoru TASAKA 2008-09-18 18:23:13 UTC
For 0.3-1:

* Summary
  - I think the current Summary is not good sentence to
    explain this package shortly...

* License
  - The codes used in cave9_src-0.3.tgz are actually under
     GPLv3+ (although COPYING.txt shows LGPLv3...)
  - The data files in cave9_data-4.tgz are licensed under
    CC-BY, CC-BY-SA.

    So the License tag must be "GPLv3+ and CC-BY and CC-BY-SA".

* BuildRequires
   - This srpm uses desktop-file-install so "BuildRequires:
     desktop-file-utils" is missing.

* sourceURL
  - Source0,1 must be written with full URL:
    https://fedoraproject.org/wiki/Packaging/SourceURL

* Macros
  - Use macros for standard directories. For example
    /usr/bin must be %{_bindir}
    https://fedoraproject.org/wiki/Packaging/RPMMacros

* Timestamps
  - When using "install" or "cp" commands, add "-p" option
    to keep timestamps on installed files:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps

* Documents
  - data/README.txt in cave9_data-4.tgz contains the needed
    license information of data tarball so this file
    must be added to %doc.

! Question
  - Is it possible to create a seperate srpm for hud.ttf
    font? (do you know where hud.ttf is originally distributed?)
    Currently Fedora strongly suggests not to package
    bundled fonts:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Avoid_bundling_of_fonts_in_other_packages

Comment 11 Alexey Torkhov 2008-10-12 16:07:05 UTC
Victor, any updates?

Comment 12 Jason Tibbitts 2008-11-05 16:21:22 UTC
Setting NEEDINFO; I will close this soon if there is no further response.

Comment 13 Victor Bogado 2008-11-07 13:39:03 UTC
Sorry for the delay, I didn't have noticed your comments until that last one. :-P 

I made all the changes you have stated. For the "font" I don't know how I would do it, the website of the font author is a flash-only-mess, I really hate those sites, and don't have a download link for the font it self. But even if there were, the font was altered by the author of the game to include numbers. So as far as I know there is no upstream release of this font. 

But I do understand the need for a separated package, so I will email the author that knows the designer that made the font so we can figure something out. I would suggest to bundle at first with the font, and later when we complete the font package change this one to use the former.

the new packages are at the same place as usual : 

http://bogado.net/rpm/cave9.spec
http://bogado.net/rpm/cave9-0.3-2.bog9.src.rpm

Comment 14 Jason Tibbitts 2008-11-07 13:49:15 UTC
Mamoru, did you want to take this one?

Comment 15 Mamoru TASAKA 2008-11-07 14:09:37 UTC
(In reply to comment #14)
> Mamoru, did you want to take this one?

If you want to review this, I would appreciate it.

Comment 16 Jason Tibbitts 2008-11-07 17:29:41 UTC
Unfortunately I have no way to test this; it doesn't over a remote X connection.  I got someone on IRC to test it and it works fine.

I get the following from rpmlint:
 cave9.x86_64: E: non-standard-executable-perm /usr/bin/cave9 0775
This doesn't happen in the buildsys so it must be some weirdness with my local setup, but you really should force the permissions just in case, either by using install -p -m 755 or by running chmod manually.

You need to document which parts of the package are under which licenses.  See 
https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios
for some suggestions on how to go about doing this.  It should be sufficient to indicate that the code is GPLv3+ and the licenses for the data are indicated in data_README.txt.  Also, you are missing an "and Public Domain" for the NASA-produced sounds.

%description is a complete sentence and should end with a period.

* source files match upstream:
  89a1ef99f2399bf7638b25ce4b51c5c088e01c29bc407eab689ccbb39c5b8d39  
   cave9_data-4.tgz
  569d311b4f02d3b25ed98051b752bb3606fc243f0f1a7a0c8901c4569eceb11f  
   cave9_src-0.3.tgz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
X description needs a trailing period.
* dist tag is present.
* build root is OK.
X license field missing "and Public Domain".
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
X rpmlint has a valid complaint.
* final provides and requires are sane:
   cave9 = 0.3-2.fc10
   cave9(x86-64) = 0.3-2.fc10
  =
   dejavu-fonts
   libGL.so.1()(64bit)
   libGLU.so.1()(64bit)
   libSDL-1.2.so.0()(64bit)
   libSDL_image-1.2.so.0()(64bit)
   libSDL_ttf-2.0.so.0()(64bit)

* %check is not present; no way to test this automatically.  Testing indicates 
   that it works fine.
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
X error in file permissions.
* no generically named files
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.
* desktop files valid and installed properly.

Comment 17 Victor Bogado 2008-11-09 10:56:55 UTC
I made the required changes, I wasn't experiencing the mode problem for here my binaries were correctly setted to 755, but as you said, better safe then sorry. :-)

specs and source rpm at the same bat-place : 

http://bogado.net/rpm/cave9-0.3-3.bog9.src.rpm
http://bogado.net/rpm/cave9.spec

Comment 18 Jason Tibbitts 2008-11-11 14:58:14 UTC
Yes, this one looks fine.  I still haven't been able to determine why I and a few others see these issues in their personal mock builds but most people don't.

Anyway, this looks fine now; the permissions are good and rpmlint is silent.

APPROVED

Comment 19 Victor Bogado 2008-11-13 00:45:56 UTC
New Package CVS Request
=======================
Package Name: Cave9
Short Description: 3d game of cave exploration
Owners: bogado
Branches: F-9 F-10
InitialCC:

Comment 20 Kevin Fenzi 2008-11-14 05:47:16 UTC
The package name is 'cave9' right, not 'Cave9' ?

cvs done with that change.

Comment 21 Jason Tibbitts 2008-12-02 23:24:27 UTC
Is there any reason this hasn't been pushed out yet?  I only see it in rawhide, with an update for f9 in testing (and nothing for f10).

Comment 22 Victor Bogado 2008-12-03 12:16:07 UTC
The reason is probably my "newbieness", I thought I already made a build for testing on the F10 branch. I will check again, now. :-)

Comment 23 Victor Bogado 2008-12-03 12:17:11 UTC
I don't have access to the build system right now, I will do it as soon as I can build it.

Comment 24 Jason Tibbitts 2008-12-03 17:50:16 UTC
I'm happy to help you through the process if you need it; just contact me via email or find me on IRC in #fedora-devel.

Comment 25 Victor Bogado 2008-12-04 12:09:30 UTC
I had already built it, but I think the process of publishing the update had failed then, I thought then that this was because F10 was in a freeze then and that it would "automagicaly" be published afterward. I was, obviously mistaken, but now I already pushed the update into testing for F10. I guess that from now on all is automatic, isn't?


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