Bug 216131 - (glest-data) Review Request: glest-data - Data files for the game Glest
Review Request: glest-data - Data files for the game Glest
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christopher Stone
Fedora Package Reviews List
Depends On:
Blocks: FE-ACCEPT glest
  Show dependency treegraph
Reported: 2006-11-17 08:25 EST by Aurelien Bompard
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2006-11-29 02:43:40 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description Aurelien Bompard 2006-11-17 08:25:15 EST
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
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 12:29:28 EST
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 17:04:51 EST
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 15:51:10 EST
- 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 18:44:44 EST
> - 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 :
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


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-26 19:26:54 EST
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 02:43:40 EST
Imported and built, thanks for the review !

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