Bug 979738 - Review Request: crawl-sdl - free roguelike game of exploration and treasure-hunting in dungeons
Summary: Review Request: crawl-sdl - free roguelike game of exploration and treasure-h...
Keywords:
Status: CLOSED CANTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-06-30 06:24 UTC by Nazar Mishturak
Modified: 2015-07-21 12:29 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-02-24 14:36:33 UTC
Type: ---
Embargoed:


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

Description Nazar Mishturak 2013-06-30 06:24:05 UTC
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 07:19:58 UTC
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 08:21:36 UTC
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 08:55:42 UTC
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 18:26:53 UTC
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 15:41:17 UTC
Anyone interested in this?

Comment 6 Christopher Meng 2013-07-23 10:22:31 UTC
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-24 01:03:45 UTC
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
     arched.
     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-24 01:04:51 UTC
Created attachment 777528 [details]
License check

Comment 9 Nazar Mishturak 2013-07-26 15:28:28 UTC
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 22:39:17 UTC
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 18:26:35 UTC
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-24 03:48:59 UTC
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 14:36:33 UTC
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.