Bug 312701 - Review Request: PySolFC - A collection of solitare card games
Summary: Review Request: PySolFC - A collection of solitare card games
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
Depends On: 281751
TreeView+ depends on / blocked
Reported: 2007-09-29 21:24 UTC by Stewart Adam
Modified: 2007-11-30 22:12 UTC (History)
3 users (show)

Clone Of:
Last Closed: 2007-10-24 07:08:58 UTC
tibbs: fedora-review+
tibbs: fedora-cvs+

Attachments (Terms of Use)

Description Stewart Adam 2007-09-29 21:24:51 UTC
Spec URL: http://www.diffingo.com/downloads/diffingo-repo/PySolFC.spec
SRPM URL: http://www.diffingo.com/downloads/diffingo-repo/PySolFC-1.1-1.fc8.src.rpm
PySolFC is a collection of more than 1000 solitaire card games. It is a fork
of PySol solitaire. Its features include modern look and feel (uses Tile widget
set), multiple cardsets and tableau backgrounds, sound, unlimited undo, player
statistics, a hint system, demo games, a solitaire wizard, support for user
written plug-ins, an integrated HTML help browser, and lots of documentation.

Comment 1 Christopher Stone 2007-09-30 03:13:21 UTC
I get an error when I run it here:
$ pysol
python: ./Modules/_tkinter.c:941: AsObj: Assertion `size < size *
sizeof(Tcl_UniChar)' failed.
/usr/bin/pysol: line 2:  3659 Aborted                 /usr/share/PySolFC/pysol.py $*

Comment 2 Stewart Adam 2007-09-30 15:07:55 UTC
I get this too, I thought it was rawhide-specific... On the support forums other
users have this problem, however the maintainer uses the same version of python
and tkinter as we do and yet his works... Maybe one of our patches is causing
the error?

Comment 3 Stewart Adam 2007-10-07 22:03:31 UTC
It seems it's a know bug (see bz#281751). Hopefully it will be solved soon.

Comment 4 Jason Tibbitts 2007-10-19 19:22:21 UTC
Given that the blocker is fixed, installed this on a clean system but now it
won't run:

  File "/usr/share/PySolFC/pysol.py", line 29, in <module>
    from pysollib.main import main
  File "/usr/lib/python2.5/site-packages/pysollib/main.py", line 43, in <module>
    from util import DataLoader
  File "/usr/lib/python2.5/site-packages/pysollib/util.py", line 65, in <module>
    from mfxutil import Image
  File "/usr/lib/python2.5/site-packages/pysollib/mfxutil.py", line 60, in <module>
    import ImageTk
ImportError: No module named ImageTk

I think a dependency is missing but it's not immediately clear to me what it is.

Comment 5 Jason Tibbitts 2007-10-19 20:19:12 UTC
OK, the missing dependency is python-imaging-tk.  I'm about to head home from
work; I'll finish up this review as soon as I make it home.

Comment 6 Stewart Adam 2007-10-19 20:27:12 UTC
Fixed - New SRPM:

Comment 7 Jason Tibbitts 2007-10-22 05:39:18 UTC
Too bad when I got home my Internet connection had crapped out.  It's not much
better now but I got through long enough to have a longer look at this.

Looks like you need to s/imageing/imaging/ in your spec.  You could probably
drop some of the manual dependencies (at least python-imaging and tkinter) if
you wanted, but it's not a big deal.

Unfortunately the bug that was blocking this has been closed but the fix hasn't
been pushed yet, and it hasn't been fixed for F8 either.  I expect that the F7
fix will be pushed before this package, but I don't know about F8.

Not a blocker, but perhaps you could consider having this package provide
"pysol" since that's the name of the executable it provides and it would make it
a bit easier on people looking for a pysol package.  I can see a few reasons why
you might not want to do this, though, so I'll leave it up to you.

rpmlint says:
   PySolFC.noarch: E: non-executable-script 
   /usr/lib/python2.5/site-packages/pysollib/games/siebenbisas.py 0644

which I don't see as a problem; it's just the usual "anything python starts with
a shebang even if it's not an executable" disease which python programmers seem
to acquire.

I never ran through my checklist, so:
* source files match upstream:
  (downloaded manually because sourceforge is sucking as usual)
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* rpmlint has acceptable complaints.
* final provides and requires are sane:
   PySolFC = 1.1-2.fc8
   python(abi) = 2.5

* %check is not present; no test suite upstream.  Manually tested with fixed 
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* desktop file present and installed properly.
* locale files handled properly.

APPROVED; just fix the "imageing" bit before you check in.

Comment 8 Jason Tibbitts 2007-10-22 06:10:38 UTC
BTW, one thing I notice after looking closely at the file list is the presence
of "COPYRIGHT" files in each of the cardset directories.  There are many ways to
deal with these:

Leave them where they are and mark them as %doc (thus making the %files list
Rename them all to files named after the cardset and put them into docdir.
Leave them alone.

I prefer the third option, simply because everything else is useless complexity
for no gain.

Comment 9 Stewart Adam 2007-10-22 22:03:40 UTC
I agree, I'll leave them for now and if someones requests/complains about it
then I'll mark them %doc in the spec (BTW, is there a way to mark files as %doc
without moving them to the root of the source directory first?).

New Package CVS Request
Package Name: PySolFC
Short Description: A collection of solitaire card games
Owners: firewing
Branches: FC-6 F-7 F-8
InitialCC: firewing
Cvsextras Commits: yes

Comment 10 Jason Tibbitts 2007-10-22 22:27:55 UTC
If you do
  %doc path/file
RPM will copy it from the source directory.  But
  %doc /path/file
will just set the documentation flag on an installed file.

Comment 11 Jason Tibbitts 2007-10-23 00:27:38 UTC
CVS done.

Comment 12 Fedora Update System 2007-10-24 07:08:56 UTC
PySolFC-1.1-3.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 13 Fedora Update System 2007-11-06 16:02:18 UTC
PySolFC-1.1-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 PySolFC'

Comment 14 Stewart Adam 2007-11-06 22:12:22 UTC
Sorry for the noise, I forgot to uncheck the close this bug when update is
pushed option in Koji.

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