Bug 234326
Summary: | Review Request: bandsaw - A syslog monitoring program for the GNOME desktop | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Damien Durand <splinux25> |
Component: | Package Review | Assignee: | 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: | rawhide | CC: | 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
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 ping Without further progress, will be closed in 1 week. Sorry fot the delay new spec: http://glive.tuxfamily.org/fedora/bandsaw/bandsaw.spec new srpm: http://glive.tuxfamily.org/fedora/bandsaw/bandsaw-0.3.0-2.fc7.src.rpm ping? It looks like you lost your reviewer when you neglected to respond. I suppose you'll need to wait for another. 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. 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! 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. (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 ??}. Damien, ping? 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 ? This request will be closed if no response from the reporter is received within one week. Closing. |