This service will be undergoing maintenance at 00:00 UTC, 2017-10-23 It is expected to last about 30 minutes
Bug 178230 - Review Request: oneko : Cat chases the cursor
Review Request: oneko : Cat chases the cursor
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Wart
David Lawrence
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-01-18 13:30 EST by Tom "spot" Callaway
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-03-07 19:11:23 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Tom "spot" Callaway 2006-01-18 13:30:38 EST
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 13:56:03 EST
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-18 22:03:18 EST
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 17:33:43 EST
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 01:37:54 EST
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 06:02:01 EST
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 13:09:49 EST
(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 13:57:42 EST
Ping spot.
Comment 8 Tom "spot" Callaway 2006-03-07 14:09:45 EST
-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 14:16:54 EST
All issues addressed.

APPROVED

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