Bug 216131 (glest-data)

Summary: Review Request: glest-data - Data files for the game Glest
Product: [Fedora] Fedora Reporter: Aurelien Bompard <gauret>
Component: Package ReviewAssignee: Christopher Stone <chris.stone>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: lemenkov
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: 2006-11-29 07:43:40 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 163779, 216130    

Description Aurelien Bompard 2006-11-17 13:25:15 UTC
Spec URL: http://gauret.free.fr/fichiers/rpms/fedora/glest-data.spec
SRPM URL: http://gauret.free.fr/fichiers/rpms/fedora/glest-data-2.0.0-1.src.rpm
Description:
Glest  is a free 3D real time strategy game, that can be modified
using XML and a set of tools
This package contains the data files for the game

See bug 216130 for the game engine.

Comment 1 Peter Lemenkov 2006-11-18 17:29:28 UTC
Maybe it would be better to use only one spec-file which produces two packages -
 glest and glest-data? )



Comment 2 Aurelien Bompard 2006-11-19 22:04:51 UTC
The objective of splitting into two spec file it to make glest-data noarch. This
way this large package only appears once for all arches on the repos. To my
knowledge, this is impossible with a single spec file.

Comment 3 Christopher Stone 2006-11-25 20:51:10 UTC
==== REVIEW CHECKLIST ====
- rpmlint output:
W: glest-data strange-permission glest-get-translations.sh 0775
W: glest-data no-%build-section

You should add a %build with a comment explaining it is not needed
Change permissions of SOURCE1 to 755
Explain why you do not install SOURCE1

- package named according to package naming guidelines
- spec filename matches %{name}
- package meets packaging guidelines
- package licensed with open source compatible license
X license does not match actual license
This is confusing, it looks like the data has a seperate license which just says
its distributable, can you clarify this with upstream?  I'm not sure this
qualifies as being open source compatible.
- license included in %doc
- spec written in American english
- spec legible
- source matches upstream
8b6902e82874011e768c64e20fbeead5  glest_data_2.0.0.zip
- package successfully compiles and builds on FC6 x86_64
- all build dependencies listed in BR
- no locales
- no shared libraries
- package is not relocatable
- package owns all directories it creates
- all other directories it uses are provided by Requries
- no duplicates in %files
- file permissions set properly
- package contains proper %clean section
- macro usage is consistent
- contains permissible content
- no large documentation
- files in %doc do not affect runtime
- no header files or static libraries
- no pkgconfig files
- no need for a devel subpackage
- no .la files
- not a GUI app needed a .desktop file
- package does not own files or directories owned by other packages


==== MUST FIX ====
- Why do you include SOURCE1 (glest-get-translations.sh) but do not install it?
- Change permissions on SOURCE1 to 755
- Add empty %build with a comment saying nothing to build
- data looks like it is not licensed as GPL.  Please clarify with upstream
- add punctuation to description

Comment 4 Aurelien Bompard 2006-11-25 23:44:44 UTC
> - Why do you include SOURCE1 (glest-get-translations.sh) but do not install it?

It is the script I used to make the translations tarball (SOURCE2). I included
it for transparency, in case someone wants to know where SOURCE2 comes from and
verify it.

> - Change permissions on SOURCE1 to 755

Done. This is ususally harmless since it's the src.rpm (and the script is not
installed in the build root)

> - Add empty %build with a comment saying nothing to build

I don't think this is a real problem. If rpmbuild accepts the spec file, then
the %build section is optional. If it is optional, it's probably for a reason,
and I think NoArch packages are why.

> - data looks like it is not licensed as GPL.  Please clarify with upstream

Exact, the licence tag is wrong. The data files are "distributable". However,
this complies with the Fedora Guidelines :
http://fedoraproject.org/wiki/Packaging/Guidelines#Shareware
Game content is allowed as long as it is distributable.
On top of that, one of glest's main features is that it can be easily modified
because the data files are XML. So I'm pretty sure you're allowed to modify it.

I've already contacted upstream to ask them to clarify if the content may be
modified. I'm still waiting for the reply, and I'll update the licence tag when
I get it.

> - add punctuation to description

Done.

New release : http://gauret.free.fr/fichiers/rpms/fedora/glest-data-2.0.0-2.src.rpm
(give it a few minutes to upload)

Thanks for the review

Comment 5 Christopher Stone 2006-11-27 00:26:54 UTC
Ya, the %build issue has been discussed in detail, it was finally decided that
it is safer to keep them in there even if they are not needed (see the php-pear
spec template in the rpmdevtools package as an example).

Everything else looks good.  The link to the Shareware is good enough for me, so
I'll go ahead and accept this now.


Comment 6 Aurelien Bompard 2006-11-29 07:43:40 UTC
Imported and built, thanks for the review !