Bug 533558 - Review Request: gtkwhiteboard - GTK Wiimote Whiteboard
Summary: Review Request: gtkwhiteboard - GTK Wiimote Whiteboard
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-11-07 10:23 UTC by Paulo Roma Cavalcanti
Modified: 2012-06-09 13:20 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-06-09 13:20:02 UTC
Type: ---
Embargoed:
j: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Paulo Roma Cavalcanti 2009-11-07 10:23:08 UTC
Spec URL: http://orion.lcg.ufrj.br/RPMS/SPECS/gtkwhiteboard.spec

SRPM URL: http://orion.lcg.ufrj.br/RPMS/src/gtkwhiteboard-1.3-2.fc10.src.rpm

Description:

Wii whiteboard python implementation.


This is a linux implementation for Johnny Lee's whiteboard:

http://johnnylee.net/projects/wii/

Comment 1 Rich Mattes 2009-11-14 17:26:39 UTC
informal review:

* You're supposed to put up the rpmlint output:
$ rpmlint gtkwhiteboard.spec ../RPMS/noarch/gtkwhiteboard-1.3-2.fc11.noarch.rpm 
gtkwhiteboard.noarch: E: explicit-lib-dependency python-xlib
gtkwhiteboard.noarch: E: script-without-shebang /usr/lib/python2.6/site-packages/gtkwhiteboard-1.3/linuxWiimoteLib.py
1 packages and 1 specfiles checked; 2 errors, 0 warnings.

There's a couple errors there that you can fix.  To find out more, do
rpmlint -I error (i.e. rpmlint -I explicit-lib-dependency)

* There's no LICENSE file provided in the %doc section

Comment 2 Paulo Roma Cavalcanti 2009-11-15 11:49:15 UTC
(In reply to comment #1)
> informal review:
> 
> * You're supposed to put up the rpmlint output:
> $ rpmlint gtkwhiteboard.spec ../RPMS/noarch/gtkwhiteboard-1.3-2.fc11.noarch.rpm 
> gtkwhiteboard.noarch: E: explicit-lib-dependency python-xlib

This is a noarch package. It contains only python scripts.
It will not force the installation of anything.   


> gtkwhiteboard.noarch: E: script-without-shebang
> /usr/lib/python2.6/site-packages/gtkwhiteboard-1.3/linuxWiimoteLib.py

This script is not supposed to be called directly.

> 1 packages and 1 specfiles checked; 2 errors, 0 warnings.
> 
> There's a couple errors there that you can fix.  To find out more, do
> rpmlint -I error (i.e. rpmlint -I explicit-lib-dependency)
> 
> * There's no LICENSE file provided in the %doc section  

The license agreement is in the README file.

Comment 3 Jason Tibbitts 2010-11-13 20:46:09 UTC
Something's rather broken with this package; it seems to make a directory directly in the filesystem root:

/gtkwhiteboard-1.3
/gtkwhiteboard-1.3/gtkwhiteboard.ico
/gtkwhiteboard-1.3/gtkwhiteboard.py
/gtkwhiteboard-1.3/linuxWiimoteLib.py
/gtkwhiteboard-1.3/mousecontrol.py
/gtkwhiteboard-1.3/perspective.py
/gtkwhiteboard-1.3/whiteboard.py

This seems to be because python_sitelib is undefined, which seems to derive from the fact that the package has no build dependency on python.

Comment 4 Paulo Roma Cavalcanti 2010-11-15 10:24:00 UTC
You are right. It needs a BR asking for python.

Spec URL: http://orion.lcg.ufrj.br/RPMS/SPECS/gtkwhiteboard.spec

SRPM URL: http://roma.fedorapeople.org/srpms/gtkwhiteboard-1.3-3.fc12.src.rpm

Scratch build:

http://koji.fedoraproject.org/koji/taskinfo?taskID=2601540


Thanks.

Comment 5 Jason Tibbitts 2010-11-16 19:39:41 UTC
Thanks.  This looks pretty good.  rpmlint says:

  gtkwhiteboard.noarch: E: explicit-lib-dependency python-xlib
Which is bogus.

  gtkwhiteboard.noarch: E: script-without-shebang
   /usr/lib/python2.7/site-packages/gtkwhiteboard-1.3/linuxWiimoteLib.py
Why is this file executable?  Actually, why are any of the .py files executable?  They're not intended to be run directly.  This should be fixed.

  gtkwhiteboard.noarch: W: no-manual-page-for-binary gtkwhiteboard
It would be nice to have a manual page but it's not mandatory.

Where did whii.png come from?  I am concerned that a picture of a Wii controller with the Wii logo visible as an icon may pose a legal issue.  Perhaps I'm just being paranoid, but I know Nintendo is fond of lawsuits so it would be good to have the legal folks give their OK.  I suspect that if this is an issue, fixing it should be easy since the icon does not come from upstream at all.  If it came from someplace on the net, that may also pose a copyright issue.

The desktop file has a problem:

/builddir/build/BUILDROOT/gtkwhiteboard-1.3-3.fc14.x86_64/usr/share/applications/gtkwhiteboard.desktop: error: (will be fatal in the future): value "GNOME" in key "Categories" in group "Desktop Entry" requires another category to be present among the following categories: GTK

* source files match upstream.  sha256sum:
  9610f498bb1711aff2898fdaa9533319fbb2fb74c7b2588c8387b969ed510e2c
   gtkwhiteboard-1.3.zip
* 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.
* license field matches the actual license (for the code, at least).
* license is open source-compatible.
* license text included in package (in README file).
* latest version is being packaged.
* BuildRequires are proper.
* package builds in mock (14, x86_64).
* package installs properly.
X rpmlint has valid complaints.
* final provides and requires are sane:
   gtkwhiteboard = 1.3-3.fc14
  =
   /bin/sh  
   /usr/bin/env  
   /usr/bin/python  
   pybluez  
   python(abi) = 2.7
   python-xlib  
   wxPython  

* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
X file permissions are odd.
* no generically named files
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
X desktop file has an error.

Comment 6 Paulo Roma Cavalcanti 2010-11-16 20:38:45 UTC
(In reply to comment #5)
> Thanks.  This looks pretty good.  rpmlint says:
> 
>   gtkwhiteboard.noarch: E: explicit-lib-dependency python-xlib
> Which is bogus.
> 
>   gtkwhiteboard.noarch: E: script-without-shebang
>    /usr/lib/python2.7/site-packages/gtkwhiteboard-1.3/linuxWiimoteLib.py
> Why is this file executable?  Actually, why are any of the .py files
> executable?  They're not intended to be run directly.  This should be fixed.

Just to make rpmlint happy. Otherwise, we get:


[cascavel:/home/mock/fedora-14-x86_64/result] rpmlint gtkwhiteboard-1.3-4.fc14.noarch.rpm
gtkwhiteboard.noarch: E: explicit-lib-dependency python-xlib
gtkwhiteboard.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/gtkwhiteboard-1.3/gtkwhiteboard.py 0644 /usr/bin/python
gtkwhiteboard.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/gtkwhiteboard-1.3/mousecontrol.py 0644 /usr/bin/python
gtkwhiteboard.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/gtkwhiteboard-1.3/perspective.py 0644 /usr/bin/env
gtkwhiteboard.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/gtkwhiteboard-1.3/whiteboard.py 0644 /usr/bin/python
gtkwhiteboard.noarch: W: no-manual-page-for-binary gtkwhiteboard
1 packages and 0 specfiles checked; 5 errors, 1 warnings.

What should I do? Keep the files 755 or use 644 and get those warnings?

> 
>   gtkwhiteboard.noarch: W: no-manual-page-for-binary gtkwhiteboard
> It would be nice to have a manual page but it's not mandatory.
> 
> Where did whii.png come from?  I am concerned that a picture of a Wii
> controller with the Wii logo visible as an icon may pose a legal issue. 
> Perhaps I'm just being paranoid, but I know Nintendo is fond of lawsuits so it
> would be good to have the legal folks give their OK.  I suspect that if this is
> an issue, fixing it should be easy since the icon does not come from upstream
> at all.  If it came from someplace on the net, that may also pose a copyright
> issue.

I think I just cut and paste from here:

http://computer.yourdictionary.com/images/computer/_WII.GIF

The picture is really nice for an icon ...

> 
> The desktop file has a problem:
> 
> /builddir/build/BUILDROOT/gtkwhiteboard-1.3-3.fc14.x86_64/usr/share/applications/gtkwhiteboard.desktop:
> error: (will be fatal in the future): value "GNOME" in key "Categories" in
> group "Desktop Entry" requires another category to be present among the
> following categories: GTK

This is easy to fix. 

> 
> * source files match upstream.  sha256sum:
>   9610f498bb1711aff2898fdaa9533319fbb2fb74c7b2588c8387b969ed510e2c
>    gtkwhiteboard-1.3.zip
> * 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.
> * license field matches the actual license (for the code, at least).
> * license is open source-compatible.
> * license text included in package (in README file).
> * latest version is being packaged.
> * BuildRequires are proper.
> * package builds in mock (14, x86_64).
> * package installs properly.
> X rpmlint has valid complaints.
> * final provides and requires are sane:
>    gtkwhiteboard = 1.3-3.fc14
>   =
>    /bin/sh  
>    /usr/bin/env  
>    /usr/bin/python  
>    pybluez  
>    python(abi) = 2.7
>    python-xlib  
>    wxPython  
> 
> * owns the directories it creates.
> * doesn't own any directories it shouldn't.
> * no duplicates in %files.
> X file permissions are odd.
> * no generically named files
> * code, not content.
> * documentation is small, so no -doc subpackage is necessary.
> * %docs are not necessary for the proper functioning of the package.
> X desktop file has an error.

I am looking forward to hearing from you, so I can produce the final .src.rpm version.

Thanks.

Comment 7 Jason Tibbitts 2010-11-16 21:39:18 UTC
For some reason python programmers like to put the '#!' line on files that are not supposed to be executed directly.  I've no idea at all why they do this, but it causes the rpmlint complaints you tried to fix.  You can either accept the complaint (they're not sufficiently problematic to block acceptance of the package) or you can fix them by removing the first line of the python source files (with sed -i -e '1d'.  Making the files executable when they aren't executables isn't the right solution.

Comment 8 Paulo Roma Cavalcanti 2010-11-16 22:06:40 UTC
Lets keep the warnings, then:

Spec URL: http://orion.lcg.ufrj.br/RPMS/SPECS/gtkwhiteboard.spec

SRPM URL: http://roma.fedorapeople.org/srpms/gtkwhiteboard-1.3-4.fc14.src.rpm

Comment 9 Jason Tibbitts 2010-11-17 22:09:15 UTC
I'd approve this package, if not for the legal issue.  So let's wait for that.

Comment 10 Paulo Roma Cavalcanti 2010-12-30 16:03:30 UTC
If I use this picture from wikipedia would that be acceptable?  

http://en.wikipedia.org/wiki/File:Wiimote.png


I can also take my own picture of a real wii remote ...

Comment 11 Tom "spot" Callaway 2011-06-30 16:53:23 UTC
I would say that the picture in Comment 10, with a slight edit to remove the "Wii" trademark from the controller would be an acceptable solution.

Comment 12 Paulo Roma Cavalcanti 2011-08-14 12:08:21 UTC
Thanks Tom. I did exactly what you suggested:

Spec URL: http://roma.fedorapeople.org/specs/gtkwhiteboard.spec

SRPM URL: http://roma.fedorapeople.org/srpms/gtkwhiteboard-1.3-5.fc14.src.rpm

Rpmlint output:

[cascavel:~/redhat/RPMS/noarch] rpmlint gtkwhiteboard-1.3-5.fc14.noarch.rpm 
gtkwhiteboard.noarch: E: explicit-lib-dependency python-xlib
gtkwhiteboard.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/gtkwhiteboard-1.3/perspective.py 0644L /usr/bin/env
gtkwhiteboard.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/gtkwhiteboard-1.3/gtkwhiteboard.py 0644L /usr/bin/python2.7
gtkwhiteboard.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/gtkwhiteboard-1.3/mousecontrol.py 0644L /usr/bin/python2.7
gtkwhiteboard.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/gtkwhiteboard-1.3/whiteboard.py 0644L /usr/bin/python2.7
gtkwhiteboard.noarch: W: no-manual-page-for-binary gtkwhiteboard
1 packages and 0 specfiles checked; 5 errors, 1 warnings.

Comment 13 Tom "spot" Callaway 2011-08-16 14:47:26 UTC
Lifting FE-Legal.

Comment 14 Jason Tibbitts 2012-05-09 19:19:07 UTC
Somehow I never noticed that the legal blocker had been lifted.  I sure wish you had pinged me.  Since I already did the review, everything still builds fine and there's been no new release upstream which would render any of the earlier work invalid, I'm just going to go ahead and approve this.  If you're still interested, feel free to move forward if the process.  If you're not, please just go ahead and close this out and accept my apologies.

APPROVED

One note, though, since the packaging guidelines have evolved since I first looked at this.  You can remove the %defattr bit from %files since it is unneeded on everything Fedora and EPEL supports.  If you do not intend to build for EPEL5, you can remove the BuildRoot: line, the entire %clean section and the first line of %install.  You should also be able to remove the %build section since it's empty.

Comment 15 Paulo Roma Cavalcanti 2012-05-21 01:56:43 UTC
Thanks, Jason, for finishing this review.

New Package CVS Request
=======================
Package Name: gtkwhiteboard
Short Description: GTK Wiimote Whiteboard
Owners: roma
Branches: F-15 F-16 F-17 EL-5 EL-6
InitialCC: roma

Comment 16 Gwyn Ciesla 2012-05-21 17:55:59 UTC
Git done (by process-git-requests).


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