Bug 400911 - Review Request: pioneers - Turnbased board strategy game (colonize an island)
Review Request: pioneers - Turnbased board strategy game (colonize an island)
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michael Schwendt
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-11-27 08:07 EST by Hans de Goede
Modified: 2008-01-11 17:17 EST (History)
2 users (show)

See Also:
Fixed In Version: 1.0beta2-3.fc8
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-01-11 17:17:05 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
bugs.michael: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Hans de Goede 2007-11-27 08:07:45 EST
Spec URL: http://people.atrpms.net/~hdegoede/pioneers.spec
SRPM URL: http://people.atrpms.net/~hdegoede/pioneers-0.11.3-1.fc9.src.rpm
Description:
Pioneers is a computerized version of a well known strategy board game. The
goal of the game is to colonize an island. The players play the first
colonists hence the name pioneers.

Pioneers is a networkbased multiplayer game, this package contains the GUI
client as well as both a GUI and CLI version of the server for local games.
Comment 1 Michael Schwendt 2007-11-27 09:18:59 EST
The only thing I don't like in the spec file is that you override
pkgdatadir during make install, but not during configure. Obviously,
this can break in not so funny ways.

Btw, note that the default pkgdatadir is the same than yours, but
no file is installed to that location.

[...]

$ find /usr/share/applications/*pion* | xargs -n 1 desktop-file-validate
/usr/share/applications/fedora-pioneers.desktop: warning: key "Encoding" in
group "Desktop Entry" is deprecated
/usr/share/applications/fedora-pioneers-editor.desktop: warning: key "Encoding"
in group "Desktop Entry" is deprecated
/usr/share/applications/fedora-pioneers-server.desktop: warning: key "Encoding"
in group "Desktop Entry" is deprecated

[...]

$ pioneers-editor 

** (pioneers-editor:5509): WARNING **: Pixmap not found:
/usr/share/pixmaps/pioneers-editor.png

$ pioneers-server-gtk 

** (pioneers-server-gtk:5512): WARNING **: Pixmap not found:
/usr/share/pixmaps/pioneers-server.png

$ pioneers

** (pioneers:13601): WARNING **: Pixmap not found: /usr/share/pixmaps/pioneers.png


It uses these pixmaps for the window title bar.

[...]

Help menu gives this error:

15:17:16 Show the manual: There was an error launching the default action
command associated with this location.
Comment 2 Hans de Goede 2007-11-27 10:51:04 EST
(In reply to comment #1)
> The only thing I don't like in the spec file is that you override
> pkgdatadir during make install, but not during configure. Obviously,
> this can break in not so funny ways.
> 

Oops, that shouldn't be there copy and paste error from another spec, will
remove with the next release.

> $ find /usr/share/applications/*pion* | xargs -n 1 desktop-file-validate
> /usr/share/applications/fedora-pioneers.desktop: warning: key "Encoding" in
> group "Desktop Entry" is deprecated
> /usr/share/applications/fedora-pioneers-editor.desktop: warning: key "Encoding"
> in group "Desktop Entry" is deprecated
> /usr/share/applications/fedora-pioneers-server.desktop: warning: key "Encoding"
> in group "Desktop Entry" is deprecated
> 

These are upstream provided, I can patch them if you want, but I would rather
wait for non-critical changes like this to trickle downstream through upstream.

> $ pioneers-editor 
> 
> ** (pioneers-editor:5509): WARNING **: Pixmap not found:
> /usr/share/pixmaps/pioneers-editor.png
> 
> $ pioneers-server-gtk 
> 
> ** (pioneers-server-gtk:5512): WARNING **: Pixmap not found:
> /usr/share/pixmaps/pioneers-server.png
> 
> $ pioneers
> 
> ** (pioneers:13601): WARNING **: Pixmap not found: /usr/share/pixmaps/pioneers.png
> 
> 
> It uses these pixmaps for the window title bar.
>

Oops,

I'll guess I'll stop moving them them, my bad.

> Help menu gives this error:
> 
> 15:17:16 Show the manual: There was an error launching the default action
> command associated with this location.
> 

Hmm, I'll investigae this.

Can you do a full review, or do you want a new fixed release first?
Comment 3 Michael Schwendt 2007-11-28 10:01:26 EST
There's nothing else to review at the packaging-level.

It needs a lawyer to judge whether this thing is "clean".
For instance, "Catan" is a world-wide registered trademark,
and according to legal advise on the following page, the
original game concept is protected, too:
http://www.das-leinhaus.de/siedler/sets/frames.html
Comment 4 Hans de Goede 2007-11-28 11:01:54 EST
(In reply to comment #3)
> There's nothing else to review at the packaging-level.
> 
> It needs a lawyer to judge whether this thing is "clean".
> For instance, "Catan" is a world-wide registered trademark,

There should be no user visible use of the word "Catan" in either the package
summary / description, nor in the game itself and accompanying documentation,
thats what the sanitize patch is for

Although I just realize I might have missed the .desktop files, I'll check those
when I find some time.

> and according to legal advise on the following page, the
> original game concept is protected, too:
> http://www.das-leinhaus.de/siedler/sets/frames.html

Good luck to them with that, there have been several lawsuits based on game
mechanics / concepts in the US and all were lost, see:
http://www.copyright.gov/fls/fl108.html

But I can fully understand if you do not want to make this assessment, let me
know, then I'll ask Spot to ok this (or not).
Comment 5 Michael Schwendt 2007-11-28 12:11:49 EST
I'm aware of that patch, which also removes the splash/About.
There are GNOCATAN env variables and the default server name
left, though, and overall I'm not sure whether maintaining the
patch will be _safe_ in the future as long as upstream mentions
the trademarks in several places, e.g. in translations and help.

$ rpmdev-extract pioneers-0.11.3-1.fc8.i386.rpm 
$ find pioneers-0.11.3-1.fc8/usr/share/locale | xargs strings | grep -i -e
'Siedler\|Settlers'
Settlers of Catan board game.
Settlers of Catan board game.
Settlers of Catan board game.
dspelet Settlers of Catan.
Settlers of Catan board game.
Settlers of Catan board game.
Settlers of Catan board game.
Brettspiel 'Die Siedler von Catan'.
Settlers of Catan board game.
bordspel "Settlers of Catan".
Settlers of Catan board game.
Settlers of Catan board game.

You may argue that these are "not user-visible" when the patch
removes the trademarks from all English strings. But you would
need to control all future translations, too, or just hope that
none of them will ever be user-visible.

U.S. jurisdiction may be different. The legal background is
from the rights owner (Cartan GmbH):
http://www.das-leinhaus.de/siedler/sets/recht.html
Comment 6 Hans de Goede 2007-11-29 15:18:01 EST
(In reply to comment #1)
> Help menu gives this error:
> 
> 15:17:16 Show the manual: There was an error launching the default action
> command associated with this location.
> 

Thats because you don't have yelp installed (you use KDE I guess), in the past
there has been some discussion on the list about adding requires to yelp for all
packages needing it for help and it was decided not to do this. I will happily
reopen this discussion on the mailinglist, but not here please.

All other comments fixed, including all the things discussed in the email
conversation with Spot:
Spec URL: http://people.atrpms.net/~hdegoede/pioneers.spec
SRPM URL: http://people.atrpms.net/~hdegoede/pioneers-0.11.3-2.fc9.src.rpm
Comment 7 Patrice Dumas 2007-11-29 16:09:50 EST
(In reply to comment #6)

> 
> Thats because you don't have yelp installed (you use KDE I guess), in the past
> there has been some discussion on the list about adding requires to yelp for all
> packages needing it for help and it was decided not to do this.

That's not what I recall. In my opinion this should be left to
the packager.
Comment 8 Fedora Update System 2007-12-03 00:35:57 EST
libtheora-1.0beta2-3.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 libtheora'
Comment 9 Hans de Goede 2007-12-03 03:34:57 EST
(In reply to comment #8)
> libtheora-1.0beta2-3.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 libtheora'

Oopsee, I accidentally entered the wrong bug id when pushing an update, ignore
please.
Comment 10 Hans de Goede 2007-12-03 03:38:36 EST
p.s. The correct bug for the libtheora problem is bug 401681.
Comment 11 Michael Schwendt 2007-12-06 05:52:53 EST
> Thats because you don't have yelp installed 

Indeed. It's a temporary victim of Firefox broken deps.

> (you use KDE I guess),

Nah.

> Requires:       hicolor-icon-theme

No longer true.


APPROVED
Comment 12 Mamoru TASAKA 2007-12-06 06:40:25 EST
(In reply to comment #6)
> Thats because you don't have yelp installed (you use KDE I guess), in the past
> there has been some discussion on the list about adding requires to yelp for all
> packages needing it for help and it was decided not to do this. I will happily
> reopen this discussion on the mailinglist, but not here please.

Just a note:
It was once decided that libgnomeui should require yelp (see bug 243478),
however after one month it is again removed because of dependency
chain problem (bug 249000)
Comment 13 Hans de Goede 2007-12-06 15:12:48 EST
Thanks for the review!

Unfortunately this has turned up, so I first have to fix that, and I'm currently
a bit short on spare time, so this may take a while:
http://www.cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2007-6010
http://lwn.net/Alerts/260519/

Still lets gets CVS ready for when I do find the time:

New Package CVS Request
=======================
Package Name:      pioneers
Short Description: Turnbased board strategy game (colonize an island)
Owners:            jwrdegoede
Branches:          F-7 F-8 devel
InitialCC:         <empty>
Cvsextras Commits: Yes
Comment 14 Kevin Fenzi 2007-12-06 16:00:20 EST
cvs done.
Comment 15 Hans de Goede 2007-12-11 10:27:48 EST
CVE fixed, imported and build, closing.
Comment 16 Fedora Update System 2008-01-11 17:17:04 EST
libtheora-1.0beta2-3.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

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