Bug 235588 - Review Request: escape - an extensible puzzle game
Summary: Review Request: escape - an extensible puzzle game
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Gwyn Ciesla
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: 235589
TreeView+ depends on / blocked
 
Reported: 2007-04-07 16:35 UTC by Adam Goode
Modified: 2021-07-30 20:04 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2007-04-22 21:34:11 UTC
Type: ---
Embargoed:
gwync: fedora-review+
jwboyer: fedora-cvs+


Attachments (Terms of Use)

Description Adam Goode 2007-04-07 16:35:33 UTC
Spec URL: http://www.spicenitz.org/fedora/escape.spec
SRPM URL: http://www.spicenitz.org/fedora/escape-200704070-1.fc7.src.rpm
Description: 
Escape is a tile-based puzzle game in the style of "Adventures of
Lolo" or "Chip's Challenge." Unlike either of those games, Escape
doesn't rely at all on reflexes--it's all about your brain.

Although Escape comes with hundreds of levels, the game places an
emphasis on the composition of new puzzles. Thus Escape has a
built-in level editor and facilities for automatically sharing
puzzles with other players.

Comment 1 Adam Goode 2007-04-07 22:57:30 UTC
This is mutually dependent on escape-data, also pending review.

Comment 2 Gwyn Ciesla 2007-04-12 17:27:31 UTC
Wiki is down, but this is an initial list of issues:
escape owns all of %{bindir}.  Should only own what it puts there.
Remove X-Fedora category
Icon-cache is not updated on install and remove, see the wiki when it's back up.
Source0 should include the full download URL.

Builds in mock, though.  I'll peek at -data.

Comment 3 Andrea Musuruane 2007-04-13 10:06:26 UTC
You should also add hicolor-icon-theme to Requires.
This is an empty package owning the /usr/share/icons/hicolor dir hierarchy,
sortoff like the filesystem package but then for icons. Note that all packages
which have an icon should Require this to ensure proper icon dir ownership.

Version is wrong. Please read:
http://fedoraproject.org/wiki/Packaging/NamingGuidelines?action=show#head-cfd71146dbb6f00cec9fe3623ea619f843394837

BTW, why do you use a snapshot version (CVS checkout) instead of a stable
release? There may be good reasons, but I'd like to know :)


Comment 4 Gwyn Ciesla 2007-04-13 11:23:47 UTC
(In reply to comment #3)

> BTW, why do you use a snapshot version (CVS checkout) instead of a stable
> release? There may be good reasons, but I'd like to know :)
> 

+1.  Would probably also make the URL issue go away more easily.

Comment 5 Adam Goode 2007-04-13 23:23:00 UTC
New release:

http://www.spicenitz.org/fedora/escape-200704130-1.fc7.src.rpm
http://www.spicenitz.org/fedora/escape.spec

 - Merge -data package into this
 - Install icon into correct place and update the icon cache
 - Use upstream tarball, created by upstream at my request
 - Remove X-Fedora category


Comment 6 Adam Goode 2007-04-14 04:51:26 UTC
Backup location if spicenitz.org is down:

http://www.andrew.cmu.edu/user/agoode/fedora/

Comment 7 Gwyn Ciesla 2007-04-16 15:42:13 UTC
Ok, let's see.

rpmlint -i clean, except for debuginfo, which complains that several source
files are marked executable.  Might want to submit to upstream, but not a
showstopper IMHO:
W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/dirt.h
W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/sound.h
E: escape-debuginfo script-without-shebang /usr/src/debug/escape-src/dirt.cpp
E: escape-debuginfo script-without-shebang /usr/src/debug/escape-src/sound.cpp
W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/progress.h
W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/sound_enum.h
E: escape-debuginfo script-without-shebang /usr/src/debug/escape-src/progress.cpp

Good package/spec names.
Meets packaging guidelines.
License OK.
Spec is legible American English.

[limb@fawkes fedora]$ md5sum escape-src-200704130.tar.bz2 
07263976a54607792dbbe4bc442f0735  escape-src-200704130.tar.bz2
[limb@fawkes fedora]$ md5sum /usr/src/redhat/SOURCES/escape-src-200704130.tar.bz2 
1764ef8aad634e6f71c5533fc7eafe21 
/usr/src/redhat/SOURCES/escape-src-200704130.tar.bz2

ISSUE: MD5 mismatch.  Put the tarball from upstream straight in the SRPM, no
modification.

Comment 8 Andrea Musuruane 2007-04-16 16:05:33 UTC
If I may add, even if I'm not the reviewer, the version is wrong. 

Using the release or snapshot date as the version is going to cause you trouble
when version x.y will be released. E.g. 200704130 > 1.0. 

If a stable release is not yet planned upstream, and therefore it is not
possibile to identify this as a prerelease of a planned release, I would suggest
to use 0.0 as version and 0.release.200704130pre as the release.

For a complete overview, please read from the following link onwards.

http://fedoraproject.org/wiki/Packaging/NamingGuidelines?action=show&redirect=PackageNamingGuidelines#head-18aa467fc6925455e44be682fd336667a17e8933


Comment 9 Gwyn Ciesla 2007-04-16 16:09:39 UTC
Good point.  What are upstream's plans?  If they never plan on a sane versioning
system 2007x would work, but even so I agree with Andrea.

Comment 10 Adam Goode 2007-04-16 16:55:27 UTC
I'll have to look into the MD5 mismatch thing later tonight. Something weird has
happened.

As for the version, upstream has no plans to move from date-based version
number. So for now the version number is correct. If upstream changes its
scheme, that's what epoch is for.

Comment 11 Adam Goode 2007-04-17 02:40:09 UTC
http://www.spicenitz.org/fedora/escape-200704130-2.fc7.src.rpm
http://www.spicenitz.org/fedora/escape.spec

 - Fix permissions in debuginfo package
 - Generate SRPM with source matching upstream MD5


Comment 12 Gwyn Ciesla 2007-04-17 11:41:52 UTC
MD5 matches.

Still some -debug perm issues.

W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/dirt.h
W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/sound.h
W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/progress.h
W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/sound_enum.h


Comment 13 Hans de Goede 2007-04-17 18:05:47 UTC
Jon, you should set the fedora review flag to ? when doing a review.


Comment 14 Gwyn Ciesla 2007-04-17 18:18:57 UTC
Whoops.  Thanks. :)

Comment 15 Adam Goode 2007-04-17 23:08:17 UTC
New version:

http://www.spicenitz.org/fedora/escape.spec
http://www.spicenitz.org/fedora/escape-200704130-3.fc7.src.rpm

Changes:

 - REALLY fix spurious-executable-perm


Comment 16 Gwyn Ciesla 2007-04-18 12:56:08 UTC
(In reply to comment #15)
>
>  - REALLY fix spurious-executable-perm
> 
Confirmed. :)

Builds on FC6 1386.
BRs are good.
No shared libs.
Not relocatable.
ISSUE: Ownership is OK, except for the .desktop line.  It should own it's own
desktop file, not all of them (*.desktop).

Comment 17 Adam Goode 2007-04-19 01:35:21 UTC
What's wrong with having something like this?

%files
%defattr(-,root,root,-)
%doc COPYING design.txt escape.txt README
%{_bindir}/*
%{_datadir}/%{name}
%{_datadir}/icons/hicolor/32x32/apps/*.png
%{_datadir}/applications/*.desktop


The * doesn't mean I own everything, just the files that I installed to
$RPM_BUILD_ROOT. This can be verified using rpm -ql escape.

Comment 18 Gwyn Ciesla 2007-04-19 11:58:33 UTC
Oh, so it is.  I didn't know that.  Nevermind. :)
No duplicate files.
Permissions are ok.
%clean present and correct.
Macros OK.
Code, not content.
No huge docs.
No runtime doc issues.
No shared or static libs, or headers.
No pkgconfig files.
No devel package.
No .la.
.desktop file handled properly.
No cross-ownership.
%install starts right.
Good filenames.

All MUSTS met.

Builds in mock of FC-6 i386.
Runs as advertised.
APPROVED.

Comment 19 Adam Goode 2007-04-20 00:56:52 UTC
New Package CVS Request
=======================
Package Name: escape
Short Description: an extensible puzzle game
Owners: adam
Branches: FC-5 FC-6
InitialCC: 

Comment 20 Matt McCutchen 2021-07-30 16:29:48 UTC
In case anyone still watching this thread (or who finds this thread later) is interested:

Escape was apparently dropped between Fedora 30 and 31 due to a FTBFS (#1674862) and lack of maintenance in Fedora.  I'm a fan of Escape and may be willing to do the necessary packaging work to get it back into Fedora at some point (I'm pretty busy with other things right now).  However, I almost certainly wouldn't be willing to do a bunch of grunt work reviewing unrelated Fedora packages in order to gain sponsorship as a packager, so if that's still a requirement, someone else would have to be the official maintainer and I could still do most of the work.

Note that I currently have my own fork of Escape (https://mattmccutchen.net/escape/#app) with numerous small but valuable fixes and enhancements.  The upstream maintainer (Tom 7) has expressed interest in merging at least some of them but is very busy with projects other than Escape and hasn't gotten around to spending any time on it yet.  I would be much more motivated to help maintain Escape in Fedora if it were a version with my enhancements.  I could try to get upstream to merge them, but if it seems like that won't happen anytime soon, we could discuss whether Fedora might be willing to recognize me as its "upstream".

From my point of view, the main benefit of having Escape in Fedora is making it discoverable to users looking for games of that kind: I was lucky enough to find it via something like "dnf search puzzle game" (I don't remember my exact search terms) shortly before I upgraded from Fedora 30 to 32.  It's very hard to find in a web search because many other, very different games are commonly described as "escape" games.  For someone who already knows about Escape, the benefit compared to installing from a binary tarball and using the built-in updater is not great.  A copr may or may not be a useful compromise with respect to discoverability, ease of installation, and maintenance work.

(Was there a better place for me to post the above comment than on this bug thread?)

Comment 21 Gwyn Ciesla 2021-07-30 20:04:41 UTC
Oh my, this is a blast from the past. :)  If you'd like to attach a spec and srpm to this BZ, I can submit them for review and maintainer.

The sponsorship model has evolved somewhat in the intervening years, you could submit it yourself and I could potentially sponsor you. 

Your call. :)


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