Bug 237344 - Review Request: supervisor - System for Allowing the Control of Process State on UNIX
Review Request: supervisor - System for Allowing the Control of Process Stat...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Toshio Kuratomi
Fedora Package Reviews List
:
Depends On: 237354
Blocks:
  Show dependency treegraph
 
Reported: 2007-04-20 17:50 EDT by Mike McGrath
Modified: 2007-11-30 17:12 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-04-22 12:40:14 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
toshio: fedora‑review+


Attachments (Terms of Use)
Spec patch with two changes (2.19 KB, patch)
2007-04-20 18:32 EDT, Toshio Kuratomi
no flags Details | Diff

  None (edit)
Description Mike McGrath 2007-04-20 17:50:02 EDT
Spec: http://mmcgrath.net/~mmcgrath/supervisor/supervisor.spec
SRPM: http://mmcgrath.net/~mmcgrath/supervisor/supervisor-2.1-1fc6.src.rpm
Description:
The supervisor is a client/server system that allows its users to control a
number of processes on UNIX-like operating systems.
Comment 1 Toshio Kuratomi 2007-04-20 18:32:21 EDT
Created attachment 153225 [details]
Spec patch with two changes

Initial spec file patch with two changes:

1) Use python_sitelib to get the directory to install the files.  This works on
multilib machines like x86_64.

2) Rename the init script from supervisor to supervisord (I renamed this in
Source1 as well which means you'll want to rename the actual file.)  This makes
the lock file and the init script match which makes rpmlint happy.  I'm not
cetain that it's necessary but
http://www.mail-archive.com/cooker@linux-mandrake.com/msg18193.html indicates
that SysVinit may need the names to match in order to properly decide whether
to start services when changing runlevel.

Full review to follow.
Comment 3 Toshio Kuratomi 2007-04-21 23:55:31 EDT
3982619435d792d333dc6ed56abdcec0  supervisor-2.1-2fc6.src.rpm

Needswork:
* The package will need to BuildRequire: python-devel or it won't build on FC7.

NonBlocker:
* The license is a bit varied.  It looks like ZPL/BSD covers it but perhaps
  a note to see the license file would be good:
  License: ZPL/BSD - see LICENSE.txt
* Adding -p to the install commands in the spec file will preserve timestamps
  and may help to prevent generation of spurious .rpmnew files when the package
  is upgraded.

Good:
* rpmlint: 
  E: supervisor non-standard-dir-perm /var/log/supervisor 0770

  The log directory can be 0770 to protect the files written to it from being
  read by others.  /var/log/messages and /var/log/httpd are examples of this
  precedent.

  W: supervisor invalid-license ZPL/BSD
  W: supervisor invalid-license ZPL/BSD

  Zope Public License and BSD is fine.

  W: supervisor strange-permission supervisord.init 0755
  This is an init file so 0755 is fine.

* Package is named according to the package naming guidelines.
* Licenses are open source and the tag matches the actual license of the code.
* License file is included in the package.
* Spec file is readable.
* No locales.
* Not a shared library.
* Package owns all its files and no others.
* Package uses macros consistently.
* Package contains code not content.
* Builds on x86_64 for FC6.
* Scriptlets follow the documented standards for packages including initscripts.
* Runs with the python-meld3 from Bug #237354 and a small daemon.

Fix the NEEDSWORK items when you import and this is APPROVED.
Comment 4 Mamoru TASAKA 2007-04-22 00:14:30 EDT
(In reply to comment #3)
> * rpmlint: 
>   E: supervisor non-standard-dir-perm /var/log/supervisor 0770
> 
>   The log directory can be 0770 to protect the files written to it from being
>   read by others.  /var/log/messages and /var/log/httpd are examples of this
>   precedent.

I just watched your comment and have not looked into
this package at all, however why should this log file
be executable?
Comment 5 Toshio Kuratomi 2007-04-22 00:27:06 EDT
(In reply to comment #4)
> I just watched your comment and have not looked into
> this package at all, however why should this log file
> be executable?

/var/log/supervisor is a directory rather than a file.  That's why it's executable.
Comment 6 Mamoru TASAKA 2007-04-22 01:05:46 EDT
Ah, then no problem.
Comment 7 Mike McGrath 2007-04-22 12:40:14 EDT
Added -p to installs and buildrequires python-devel.  Is building for rawhide now.

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