Bug 234326

Summary: Review Request: bandsaw - A syslog monitoring program for the GNOME desktop
Product: [Fedora] Fedora Reporter: Damien Durand <splinux25>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED NOTABUG QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: dtimms, lxtnow
Target Milestone: ---Keywords: Reopened
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-09-24 13:20:19 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 201449    

Description Damien Durand 2007-03-28 14:45:40 UTC
Spec URL: http://glive.tuxfamily.org/fedora/bandsaw/bandsaw.spec
SRPM URL: http://glive.tuxfamily.org/fedora/bandsaw/bandsaw-0.3.0-1.src.rpm
Description: Band Saw is a syslog monitoring program for the GNOME desktop. It 
allows the user to setup filters that define which messages should 
generate alerts. Combined with syslog's remote logging functionality it 
provides a scalable and easily deployed monitoring solution

Comment 1 Bernard Johnson 2007-04-18 03:31:11 UTC
Doesn't mock build.


checking whether make sets $(MAKE)... yes
checking whether to enable maintainer-specific portions of Makefiles... no
checking for gconftool-2... /usr/bin/gconftool-2
Using config source xml:merged:/etc/gconf/gconf.xml.defaults for schema installation
Using $(sysconfdir)/gconf/schemas as install directory for schema files
checking for scrollkeeper-config... no
configure: error: Couldn't find scrollkeeper-config. Please install the
scrollkeeper package: http://scrollkeeper.sourceforge.net
error: Bad exit status from /var/tmp/rpm-tmp.60534 (%build)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.60534 (%build)

Error building package from bandsaw-0.3.0-1.fc6.src.rpm, See build log
ending
done


Also, I noticed that you don't have a call to update-desktop-database in %post
and %postun.  Please see:
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets?highlight=%28scriptlets%29#head-de6770dd9867fcd085a73a4700f6bcd0d10294ef

Comment 2 Bernard Johnson 2007-05-01 20:44:49 UTC
ping

Comment 3 Bernard Johnson 2007-05-09 21:02:22 UTC
Without further progress, will be closed in 1 week.

Comment 5 Damien Durand 2007-06-08 12:51:06 UTC
ping?

Comment 6 Jason Tibbitts 2007-07-10 01:37:53 UTC
It looks like you lost your reviewer when you neglected to respond.  I suppose
you'll need to wait for another.

Comment 7 David Timms 2007-07-28 14:56:28 UTC
bandsaw review:

(In reply to comment #4)
> new spec: http://glive.tuxfamily.org/fedora/bandsaw/bandsaw.spec
Disclaimer: I am not a reviewer nor sponsor, and this is my first "review like"
submission.

>BuildRequires:  pygtk2-devel, gnome-python2-devel, gnome-doc-utils, gettext,
desktop-file-utils, scrollkeeper
My personal preference is to limit each line to 80 chars. You can have multiple
BR entries, perhaps splitting of the last two/3 items.

> %post
> update-desktop-database &> /dev/null ||:
- I don't know if it makes a difference, but the snippet shows || :

yum localinstall the .src.rpm emits the following:
warning: user damien does not exist - using root
warning: group damien does not exist - using root
  Installing: bandsaw                      ###################### 
[29/30]warning: user damien does not exist - using root
warning: group damien does not exist - using root
- I think this is not a problem (on the fedora buils sys), but can be solved by
installing mock and building the src.rpm as the mock user ?

New lines:
- please be consistent with the new line approach {double} you took between BR
and %description, but haven't continued with all the way through.

spelling: postum

MUST Items:
.x rpmlint result:
W: bandsaw non-conffile-in-etc /etc/gconf/schemas/bandsaw.schemas
E: bandsaw no-binary
  - Please use the output of rpmlint -i for more info to solve these.

./ named according to the Package Naming Guidelines: matches upstream project
and source download name.
./ spec file name matches the base package bandsaw.spec
. package must meet the Packaging Guidelines.
./ package must be licensed with an open-source compatible license:
  - web site indicates GPL and upstream source includes GPLv2.
./ License field in the package spec file must match the actual license:
  - GPL
./ source package includes the text of the license(s) in its own file, so text
of the license(s) for the package must be included in %doc:
  - COPYING is included as required.
./ The spec file must be written in American English.
.? spec file for the package MUST be legible:
  - at this stage, it is not obvious to me the need for:
%define debug_package %{nil}
%{!?python_sitearch: %define python_sitearch %(%{__python} -c "from
distutils.sysconfig import get_python_lib; print get_python_lib(1)")}
  Can you point to an existing fedora spec that uses similar ?
./ source in .src.rpm matches upstream md5sum:
  - md5sum bandsaw-0.3.0.tar.gz /usr/src/redhat/SOURCES/bandsaw-0.3.0.tar.gz 
22312a8bccc283d29db55074c69b6073  bandsaw-0.3.0.tar.gz
22312a8bccc283d29db55074c69b6073  /usr/src/redhat/SOURCES/bandsaw-0.3.0.tar.gz
./ successfully compiles and builds into binary rpms: i386 {athlon}
.? If the package does not successfully compile, build or work on an
architecture - :
  - only tried on i386{i686/athlon} and no excludearchs listed.
  - have you tested on x86_64 or other arch ?
.? build dependencies must be listed in BuildRequires:
  - no listed BR is in the auto included list, the package built on my system
after yum localinstall the .src.rpm installed lots of -devel rpms.
  - yet to try mock build.
.? spec file MUST handle locales properly:
  - neither find_lang macro nor %{_datadir}/locale are used.
./ has no shared library files
./ not relocatable and does not use Prefix: /usr
.? package must own all directories that it creates:
  - doesn't install anything currently.
./ A package must not contain any duplicate files in the %files listing:
  - does not appear to.
.? Permissions on files must be set properly. Executables should be set with
executable permissions, for example. Every %files section must include a
%defattr(...) line.
.? must have a %clean section, containing rm -rf %{buildroot} (or $RPM_BUILD_ROOT):
  - Included. Is there a preference for the %{x} style ?
.? Each package must consistently use macros:
  - debug_package doesn't seem to be used. What is it's purpose ?
./ The package must contain code, or permissable content.
  - contains a GUI app
./ Large documentation files:
  - total doc is 35kB
.x %doc files must not affect the runtime of the application:
  - currently no files are installed at all.
./ Header files must be in a -devel package:
  - no header files.
./ Static libraries must be in a -static package:
  - no static libraries.
./ has no pkgconfig(.pc) files.
./ library files with a suffix: no libraries
./ devel packages must require the base package: 
  - no -devel package
./ Packages must NOT contain any .la libtool archives
  - no .la's
.x Packages containing GUI applications must include a %{name}.desktop file, and
that file must be properly installed with desktop-file-install
>+ desktop-file-install --vendor= --delete-original --dir
/var/tmp/bandsaw-0.3.0-2.fc7-root-root/usr/share/applications
/var/tmp/bandsaw-0.3.0-2.fc7-root-root//usr/share/applications/bandsaw.desktop
/var/tmp/bandsaw-0.3.0-2.fc7-root-root/usr/share/applications/bandsaw.desktop:
warning: The 'Application' category is not defined by the desktop entry
specification.  Please use one of "AudioVideo", "Audio", "Video", "Development",
"Education", "Game", "Graphics", "Network", "Office", "Settings", "System",
"Utility" instead
  - Application category is not defined in:
http://standards.freedesktop.org/menu-spec/latest/apa.html. Please remove.
  - GTK;Monitor; would be additional suitable categories
  - no GenericName= is defined
  - .desktop does not get installed.
  
.? Packages must not own files or directories already owned by other packages. 
.? At the beginning of %install, each package MUST run rm -rf %{buildroot} (or
$RPM_BUILD_ROOT):
  - it includes $RPM_BUILD_ROOT as required, but then uses eg %{_datadir} in the
same command. I think it would make sense to keep to the % method.
./ All filenames in rpm packages must be valid UTF-8.

SHOULD Items:
./ If the source package does not include license text(s) as a separate file
from upstream, the packager SHOULD query upstream to include it: included.
./ The description and summary sections in the package spec file should contain
translations for supported Non-English languages:
  - no other translations available 

.todo package builds in mock.
  - package does not install anything currently; I'll get to this once other
issues have been taken care of.

.?  The package should compile and build into binary rpms on all supported
architectures:
  - this is a python/gtk/glade program. Is python bytecode crossplatform, ie
will it just work on any platform ? If so should it be noarch ?
  
.x package functions as described
  - the package did not install it's bits. typing bands{tab} there is no
autocomplete, and updatedb shows only my /usr/src/ bandsaw files.

.  If scriptlets are used, those scriptlets must be sane. This is vague, and
left up to the reviewers judgement to determine sanity.
.  Usually, subpackages other than devel should require the base package using a
fully versioned dependency.
./ no pkgconfig(.pc) files: python.
./ no file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin.

Comment 8 David Timms 2007-08-11 06:02:11 UTC
Damien, have you had a chance to peruse my pre-review ?
It is possibly not obvious that for each element listed:
.x = Not ok
.? = Either I don't understand enough to confirm that the element is acceptable,
or I don't understand what the spec is doing. I would request some comment to
explain if you believe that item is OK.
./ = tick - OK. No work required. Usually a comment was included after: to
describe what I found.
If you need bigger hints, please say so in this review request, and I'll try to
help.

(In reply to comment #7)
> .? build dependencies must be listed in BuildRequires:
>   - no listed BR is in the auto included list, the package built on my system
> after yum localinstall the .src.rpm installed lots of -devel rpms.
>   - yet to try mock build.
./ package builds in mock: see attached build.log
On that machine {fedora 7}, the mock built package installs OK.

When run as a user I see:
$ bandsaw 
Traceback (most recent call last):
  File "/usr/bin/bandsaw", line 31, in <module>
    import egg.trayicon
  File "/usr/lib/python2.5/warnings.py", line 62, in warn
    globals)
  File "/usr/lib/python2.5/warnings.py", line 83, in warn_explicit
    for item in filters:
ImportError: could not import gtk

  - should the package work when run as a user ?

. when run as root:
# bandsaw 
/usr/bin/bandsaw:31: DeprecationWarning: the module egg.trayicon is deprecated;
equivalent functionality can now be found in pygtk 2.10
  import egg.trayicon

As root, I get the config dialog, {after I make the fifo file, and add to the
/etc/syslog.conf}, and it then runs the notify icon. Clicking this opens the UI.
Even when I cause stuff that gets appended to /var/log/messages, no items are
added to the gui. Hence, I can't tell for sure whether it is working OK, what
should I be seeing ? {what is good syslog config that gives lots of messages
into bandsaw ?}
===
.x rpm install should be silent:
  - On my normal machine if I rpmbuild -ba the spec, the resultant yum install:
...
Installing:
 bandsaw                 i386       0.3.0-2.fc7     
/usr/src/redhat/RPMS/i386/bandsaw-0.3.0-2.fc7.i386.rpm  256 k

Transaction Summary
=============================================================================
Install      1 Package(s)         
Update       0 Package(s)         
Remove       0 Package(s)         

Total download size: 256 k
Is this ok [y/N]: y
Downloading Packages:
Running Transaction Test
Finished Transaction Test
Transaction Test Succeeded
Running Transaction
  Installing: bandsaw                      ######################### [1/1] 
gconfd-2: no process killed

Installed: bandsaw.i386 0:0.3.0-2.fc7
Complete!

Comment 9 Mamoru TASAKA 2007-08-11 15:55:33 UTC
David, if you want to do this style of (pre-)review, please also
write the summary of your (pre-)review, especially what you
think is the problem with this package so that everyone (not only
you and the submitter but also someone else) can check your review
easily...

From very quick check of this package and your comment:
(In reply to comment #7)
> (In reply to comment #4)
> > new spec: http://glive.tuxfamily.org/fedora/bandsaw/bandsaw.spec

> > %post
> > update-desktop-database &> /dev/null ||:
> - I don't know if it makes a difference, but the snippet shows || :
  - You can check what "|| :" means by "man bash". This is to 
    ensure that %post stage will exit successfully even if the
    command failed with some reason.
    http://fedoraproject.org/wiki/Packaging/ScriptletSnippets

> MUST Items:
> .x rpmlint result:
> W: bandsaw non-conffile-in-etc /etc/gconf/schemas/bandsaw.schemas
   - For GConf schemas file, this rpmlint can be removed.

> E: bandsaw no-binary
>   - Please use the output of rpmlint -i for more info to solve these.
   - This package seems to be noarch package so actually this must
     be fixed.

> %{!?python_sitearch: %define python_sitearch %(%{__python} -c "from
> distutils.sysconfig import get_python_lib; print get_python_lib(1)")}
>   Can you point to an existing fedora spec that uses similar ?
   http://fedoraproject.org/wiki/Packaging/Python

> .x Packages containing GUI applications must include a %{name}.desktop file, and
> that file must be properly installed with desktop-file-install
> >+ desktop-file-install --vendor= --delete-original --dir
> /var/tmp/bandsaw-0.3.0-2.fc7-root-root/usr/share/applications
> /var/tmp/bandsaw-0.3.0-2.fc7-root-root//usr/share/applications/bandsaw.desktop
> /var/tmp/bandsaw-0.3.0-2.fc7-root-root/usr/share/applications/bandsaw.desktop:
> warning: The 'Application' category is not defined by the desktop entry
> specification.  Please use one of "AudioVideo", "Audio", "Video", "Development",
> "Education", "Game", "Graphics", "Network", "Office", "Settings", "System",
> "Utility" instead
>   - Application category is not defined in:
> http://standards.freedesktop.org/menu-spec/latest/apa.html. Please remove.
  - Please fix this, Damien

>   - this is a python/gtk/glade program. Is python bytecode crossplatform, ie
> will it just work on any platform ? If so should it be noarch ?
  - This is noarch.

(In reply to comment #8)

> When run as a user I see:
> $ bandsaw 
> Traceback (most recent call last):
>   File "/usr/bin/bandsaw", line 31, in <module>
>     import egg.trayicon
>   File "/usr/lib/python2.5/warnings.py", line 62, in warn
>     globals)
>   File "/usr/lib/python2.5/warnings.py", line 83, in warn_explicit
>     for item in filters:
> ImportError: could not import gtk
 - Lots of dependencies are missing. Approprate "Requires" should
   be added.

Setting to NEEDINFO as reply from the submitter is missing.

Comment 10 David Timms 2007-08-12 00:23:41 UTC
(In reply to comment #9)
> David, if you want to do this style of (pre-)review, please also
> write the summary of your (pre-)review, especially what you
> think is the problem with this package so that everyone (not only
> you and the submitter but also someone else) can check your review
> easily...
Thanks, Mamoru, will do.

> > > update-desktop-database &> /dev/null ||:
> > - I don't know if it makes a difference, but the snippet shows || :
>   - You can check what "|| :" means by "man bash". This is to 
>     ensure that %post stage will exit successfully even if the
>     command failed with some reason.
>     http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
From man bash, I understand that || is an token that is a control operator, and
that : is the null command and always returns exit code of zero. It would seem
then that the " " between the two parts is necessary {this was the point I was
querying the submitter about}. {Off topic - how can you search man or internet
for such a construct ??}.

Comment 11 Mamoru TASAKA 2007-08-20 17:13:31 UTC
Damien, ping?

Comment 12 David Timms 2007-08-30 12:50:22 UTC
Damien: Do you think you will get time to continue working on this package to
bring it up to par, or would you rather someone else proceed from here ?

Comment 13 Mamoru TASAKA 2007-09-12 15:27:57 UTC
This request will be closed if no response from the reporter
is received within one week.

Comment 14 Mamoru TASAKA 2007-09-24 13:20:19 UTC
Closing.