Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 178230 - Review Request: oneko : Cat chases the cursor
Summary: Review Request: oneko : Cat chases the cursor
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Wart
QA Contact: David Lawrence
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-01-18 18:30 UTC by Tom "spot" Callaway
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-03-08 00:11:23 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Tom "spot" Callaway 2006-01-18 18:30:38 UTC
Spec Name or Url: http://www.auroralinux.org/people/spot/review/oneko.spec
SRPM Name or Url: http://www.auroralinux.org/people/spot/review/oneko-1.2-1.src.rpm
Description: 
A cat (neko) chases the cursor (now a mouse) around the screen while you
work. Alternatively, a dog chases a bone.

Note to reviewers: This package is for FC-5. If you want to build for FC-4, you'd need to replace all the BuildRequires with: xorg-x11-devel.

Comment 1 Wart 2006-01-18 18:56:03 UTC
To build on FC-4 you must also change the binary location in %files:

%{_exec_prefix}/X11R6/bin/oneko

I'll do a formal review later today once my devel box is available.

Comment 2 Wart 2006-01-19 03:03:18 UTC
MUST items:

- rpmlint output is clean
- name is appropriate
- spec filename matches package name
- macro use in spec file is consistent (other than /usr/include; see below)
- Build root is correct
- Summary and description are ok.
- No static libs
- Desktop file looks ok and is installed correctly.
- RPM_OPT_FLAGS passed to make.
- Not relocatable
- Contains code, not content
- Package does not create any directories that it should own.
- spec file is in english and is legible
- Compiles and builds on devel (did not test in mock)
- No shared libraries
- No duplicate %files
- Permissions set correctly
- No -devel package needed

SHOULD items:

- No license file included.  Please query upstream to include one, even
  if it just says that the software is public domain.
- Builds on x86_64-FC4 (with noted changes) and i386-FC5
- Packages runs on both architectures

Non-blocking issues:

%{?_smp_mflags} not passed to make, but there is only one file to compile
so it would have no effect.

The tarball includes a japanese manpage.  Why not include that in the package?

The application itself has some small problems that should be reported upstream:
1) There is no way to turn off oneko.  Once it's started, it runs until you
   log out of X or /usr/bin/kill it manually.  I suspect that this was the
   purpose of the (removed) BSD daemon.
2) If you /usr/bin/kill oneko, the root window cursor reverts to an 'X', not
   a diagonal-arrow as is the default for gnome.

Source does not match upstream due to the removal of the copyrighted images.
All other files were verified the same with 'diff'.

MUSTFIX:

* Several of the included documentation files are in Japanese.  This should
probably be indicated in the filename somehow.

* The 'make' line in the specfile uses "-I /usr/include".  Please change this
to"-I %{_includedir}".

* The patch to the manpage has a grammatical problem:
"a cat wite tiger-like stripe" should be "a cat with tiger-like stripes"
The manpage has other grammatical problems that I'm willing to ignore, but
this line was explicitly fixed by the patch, so it should at least be
grammatically correct.  :)

* BR: xorg-x11-proto-devel is redundant since it's required by libX11-devel.

* 'install' is missing '-p'

Questions:

The spec file says the license is "Public Domain", but I could not find
reference to any license information in the tarball or on the website.
Is this the proper license to use if the author hasn't specified one?
Have you asked upstream what kind of license it falls under?


Comment 3 Tom "spot" Callaway 2006-01-19 22:33:43 UTC
The upstream maintainers have vanished into the ether... see the licensing
discussion here: http://www.monkey.org/openbsd/archive/ports/0010/msg00009.html

I tend to go with what Debian decided, which is public domain.

- Added the Japanese manpage.
- The cursor change seems temporary, as X seems to eventually put the cursor
back to normal... someone who understands X's internals better than me is
welcome to try and fix oneko.
- The removed "BSD Daemon" was a set of graphics for oneko that turned it from a
cat into the BSD logo, has nothing to do with its non existent shutdown.
- Doc files renamed with ".jp" extension where appropriate.
- Used %{_includedir}
- Removed _i386_ define (as it will not be always true)
- fix typo
- remove redundant BR
- add -p to install

New package here:

SRPM: http://www.auroralinux.org/people/spot/review/oneko-1.2-2.src.rpm
SPEC: http://www.auroralinux.org/people/spot/review/oneko.spec


Comment 4 Wart 2006-01-20 06:37:54 UTC
I'm comfortable with the license as PD given this licensing discussion.

Only one comment:  The japanese man page does not need the '.jp' extension since
it already lives in a 'ja' subdirectory.

Everything else looks good as per comment #2.  Fix the japanese manpage name and
I'll approve it.

Comment 5 Michael Schwendt 2006-01-21 11:02:01 UTC
Small side-note here:

> The 'make' line in the specfile uses "-I /usr/include".
> Please change this to"-I %{_includedir}".

Preferably, neither one. /usr/include is in default search path list.
By adding -I %{_includedir}, you defeat the purpose of a user defined
search path list which takes precedence over the default search path list.


Comment 6 Wart 2006-01-21 18:09:49 UTC
(In reply to comment #5)
> Small side-note here:
> 
> > The 'make' line in the specfile uses "-I /usr/include".
> > Please change this to"-I %{_includedir}".
> 
> Preferably, neither one. /usr/include is in default search path list.
> By adding -I %{_includedir}, you defeat the purpose of a user defined
> search path list which takes precedence over the default search path list.

I verified that "-I %{_includedir}" isn't needed to build this package.  It
should be removed.

Comment 7 Wart 2006-03-07 18:57:42 UTC
Ping spot.

Comment 8 Tom "spot" Callaway 2006-03-07 19:09:45 UTC
-3 removes the includedir from CFLAGS, and renames the japanese man page:

SRPM: http://www.auroralinux.org/people/spot/review/oneko-1.2-3.src.rpm
SPEC: http://www.auroralinux.org/people/spot/review/oneko.spec

Comment 9 Wart 2006-03-07 19:16:54 UTC
All issues addressed.

APPROVED


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