Bug 979738 - Review Request: crawl-sdl - free roguelike game of exploration and treasure-hunting in dungeons
Review Request: crawl-sdl - free roguelike game of exploration and treasure-h...
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2013-06-30 02:24 EDT by Nazar Mishturak
Modified: 2015-07-21 08:29 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2014-02-24 09:36:33 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)
License check (239.89 KB, text/plain)
2013-07-23 21:04 EDT, Christopher Meng
no flags Details

  None (edit)
Description Nazar Mishturak 2013-06-30 02:24:05 EDT
Spec URL: http://goo.gl/trgiO
SRPM URL: http://goo.gl/trgiO
Description: Dungeon Crawl Stone Soup is a free roguelike game of exploration and treasure-hunting in dungeons filled with dangerous and unfriendly monsters in a quest for the mystifyingly fabulous Orb of Zot.
Fedora Account System Username: BlasterBlade
Comment 1 Nazar Mishturak 2013-06-30 03:19:58 EDT
Update (desktop file added and icon). Also changed binary name to avoid possible conflict(see the changelog).
SPEC https://docs.google.com/file/d/0B_CR4N27LxsuT05ROXBtcUtNSEk/edit?usp=sharing
SRPM https://docs.google.com/file/d/0B_CR4N27LxsuSDEwajJJMTZyT0k/edit?usp=sharing
Comment 2 Volker Fröhlich 2013-06-30 04:21:36 EDT
Why did you pick "crawl-sdl" as the name? http://fedoraproject.org/wiki/Packaging:NamingGuidelines#General_Naming

Please provide the files under some URL easy to use with wget or curl.

Remove the BRs listed under http://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2

Remove the Requires that are not fonts: http://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires

Use -p on cp to preserve the original timestamps.

You should use the version macro in your %setup invocation.

Most people leave a blank line between changelog entries. I think you should also wrap the lines at some point.
Comment 3 Nazar Mishturak 2013-06-30 04:55:42 EDT
Updated, removed BR and Requires that were unneeded.
SRPM https://docs.google.com/file/d/0B_CR4N27LxsueUxZMERnSkJTUGc/edit?usp=sharing
SPEC https://docs.google.com/file/d/0B_CR4N27LxsuT2ZuSFdyMUktSXM/edit?usp=sharing

Sorry I can't get my fedorapeople account, but I opened the Ininial Package Hosting ticket. When I get it I will make direct links.

I used crawl-sdl name because:
- game has 2 versions TILES and NCURSES, TILES use SDL and OpenGL for graphics
- you can specify which version to build when you do make( make TILES=y/n)
- executable file is still named crawl, so it doesn't get any prefix/suffix when you change versions, and TILES version requires extra png files in %{_datadir}/%{name} (i rename crawl to crawl-sdl in %{bindir} and provide a .desktop file, SDL version will have its own datadir - %{_datadir}/crawl-sdl)
- maybe i will package non tiles version with another name like crawl-ncurses or crawl-console(it will be a subpackage)
Comment 4 Nazar Mishturak 2013-07-02 14:26:53 EDT
Finally got my fedorapeople account. Now I have direct links:
SRPM http://blasterblade.fedorapeople.org/crawl-sdl-0.12.2-4.fc18.src.rpm
SPEC http://blasterblade.fedorapeople.org/crawl-sdl.spec
Still looking for a sponsor. Any comments?
Comment 5 Nazar Mishturak 2013-07-10 11:41:17 EDT
Anyone interested in this?
Comment 6 Christopher Meng 2013-07-23 06:22:31 EDT
I may take a review but I can't sponsor you.

You need to find a sponsor, otherwise there may have no people interested in this.
Comment 7 Christopher Meng 2013-07-23 21:03:45 EDT
I'm sorry I can't sponsor you, so here is an informal review of this package:

1. From spec file itself:

- Summary tag is too long;

- Remove rm -rf %{buildroot} in %install section;

- Remove INSTALL.txt in %doc of %files section;

- description field is not good. Please change it to less than 80 chars per line;

See: http://fedoraproject.org/wiki/Packaging:Guidelines#Summary_and_description

- Can you send the desktop file to upstream?

2. From the package itself:

- License is complex, but please 

  - Notify upstream to fix the incorrect license;
  - Please check the bugzilla attachment I just uploaded, make sure the license is correct.

- Large data in /usr/share should live in a noarch subpackage if package is
     Note: Arch-ed rpms have a total of 12922880 bytes in /usr/share.

So consider a -data subpackage to include all files under datadir;

I'll do another informal review once you correct these.
Comment 8 Christopher Meng 2013-07-23 21:04:51 EDT
Created attachment 777528 [details]
License check
Comment 9 Nazar Mishturak 2013-07-26 11:28:28 EDT
Does license tag apply only to binaries and files included in package?
Because this is what licence.txt says(#my comment):

Dungeon Crawl Stone Soup
Copyright 1997-2013 Linley Henzell, the dev team, and the contributors
(please refer to CREDITS.txt for details).

This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License, or
(at your option) any later version.

Certain pieces come with different licenses, all compatible with the GPL.
These include:
* 3-clause BSD: MSVC/stdint.h # Microsoft Visual Studio C99 hack, not compiled
* 2-clause BSD: all contributions by Steve Noonan and Jesse Luehrs # probably included in binary
* Public Domain|CC0: most of tiles, perlin.cc, perlin.h # tiles in datadir
* MIT: json.cc/json.h, some .js files in webserver/static/scripts/contrib/
       worley.{cc,h} # webserver part of crawl, allows to host a game online, not in this package
* zlib: webserver/static/scripts/contrib/inflate.js # same thing

Should I remove some of licenses from License tag using licence.txt file or follow licensecheck output?
Comment 10 Volker Fröhlich 2013-09-22 18:39:17 EDT
It applies for the files you ship. You're probably not shipping MS' stdint.h, but I wouldn't bother removing this information from license.txt. As all of the mentioned are compatible to GPLv2 and 3, stating GPLv2+ is technically correct.

We had another review where things were broken down for content/code. You could do the same and state "GPLv2+ and CC0".
Comment 11 Nazar Mishturak 2013-10-12 14:26:35 EDT
Updated to 0.13.0, fixed the licensecheck output:
SRPM http://blasterblade.fedorapeople.org/crawl-sdl-0.13.0-5.fc19.src.rpm
SPEC http://blasterblade.fedorapeople.org/crawl-sdl.spec
The fix was to use nodeps source package which doesn't include third party libraries.
Comment 12 Christopher Meng 2014-02-23 22:48:59 EST
New version 0.13.1 is available.

No need to ship INSTALL.txt as %doc since this is a package.

You need to ensure that ldflags="{%__global_ldflags}"

Remove rm -rf %{buildroot} in %install

Biggest question again:

Tarball name: stone_soup-0.13.1-nodeps.tar.xz

Why did you name it as crawl-sdl?

Can you acquire the best name from upstream again?
Comment 13 Nazar Mishturak 2014-02-24 09:36:33 EST
A lot of time passed. I contacted upstream and I published my package on openSUSE OBS repository. They kindly put my links on their download section: http://crawl.develz.org/wordpress/downloads. I won't be trying to add this package to Fedora anymore.

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