Bug 423821 - Review Request: nagios-plugins-rsync - Nagios plugin to monitor remote rsync servers
Summary: Review Request: nagios-plugins-rsync - Nagios plugin to monitor remote rsync ...
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-12-13 18:35 UTC by Jima
Modified: 2009-03-15 07:24 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-03-15 07:24:55 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Jima 2007-12-13 18:35:22 UTC
Spec URL: http://beer.tclug.org/fedora/nagios-plugins-rsync/nagios-plugins-rsync.spec
SRPM URL: http://beer.tclug.org/fedora/nagios-plugins-rsync/nagios-plugins-rsync-1.01-0.1.fc9.src.rpm
Description:

This plugin allows you to check rsync servers' availability, as well
as (optionally) individual modules' availability. It also supports
authentication on modules.

--
 Not exactly my finest work, but necessarily so to play well with all the other plugins.

rpmlint isn't thrilled:
nagios-plugins-rsync.x86_64: W: no-documentation

 "Upstream" consists of a single perl script on nagiosexchange.org.  No LICENSE file, but the license info is in the file, and since it's a script, that's still plaintext.  So probably not a blocker, I'd think.

nagios-plugins-rsync.x86_64: E: no-binary
nagios-plugins-rsync.x86_64: E: only-non-binary-in-usr-lib
nagios-plugins-rsync-debuginfo.x86_64: E: empty-debuginfo-package

 These three are a result of it being a perl script build as arch-specific.  It's arch-specific so it can live in %{_libdir}/nagios/plugins/ with all the other Nagios plugins.  Some other non-binary scripts already live there (like nagios-plugins-disk_smb), so I'm not the first jackass to do this (just the most recent). :-)

 Not an awesome package, but there's some use for it in Fedora Infrastructure, so it'd be "nice" if it were in Fedora proper.

 Thanks!

Comment 1 Peter Lemenkov 2008-07-01 06:55:41 UTC
I'll review it.

Comment 2 Peter Lemenkov 2008-07-03 10:20:37 UTC
Few initial remarks:

* URL changed to
http://www.nagiosexchange.org/cgi-bin/page.cgi?g=Detailed%2F2094.html;d=1

* No need to create directory - install can do it.
install -D -p -m 755 check_rsync $RPM_BUILD_ROOT/%{nagios_plugins_dir}/check_rsync

e.g. you may remove the following line:

install -d -m 755 $RPM_BUILD_ROOT/%{nagios_plugins_dir}

* Missing requires. At least:

Requires: perl(Test::Simple)

Consider applying these suggested changes and I'll review it.

Comment 3 Jima 2008-07-03 13:30:25 UTC
Thanks, Peter!

I've updated the package accordingly, and ran a test build (populating a rawhide
cache took a bit), just to ensure it still works.  A lot can happen in 7 months,
like packaging policy changing. :-)

Spec URL:
http://beer.tclug.org/fedora/nagios-plugins-rsync/nagios-plugins-rsync.spec
SRPM URL:
http://beer.tclug.org/fedora/nagios-plugins-rsync/nagios-plugins-rsync-1.01-0.2.fc10.src.rpm

The rpmlint output is still the same; that's sadly not an easily avoided issue.

Comment 4 Peter Lemenkov 2008-07-03 14:04:42 UTC
Some additional remark - you should add "BuildArch: noarch" to suppress some
rpmlint messages. This one will be still exist:

nagios-plugins-rsync.noarch: E: only-non-binary-in-usr-lib

IMHO it may be safely ignored.

BTW what the difference between this particular plugin and this one:

http://www.nagiosexchange.org/cgi-bin/page.cgi?g=1560.html;d=1

Comment 5 Jima 2008-07-03 14:31:50 UTC
If it's noarch, %{_libdir} doesn't work sanely, and it doesn't reliably end up
the same place as all the other (arch-specific and noarch) Nagios plugins.  The
same package (using /usr/lib or /usr/lib64) will be used on 32- and 64-bit
hosts, and it's going to be the wrong place on one of them.  (Unless I'm
completely missing something, anyway.)  Pretending the package is arch-specific
is a hack, but it's a hack to play nice with the rest of nagios-plugins.  (Which
I did elaborate on in my first comment...)

The difference between the two is that the one you cited didn't exist when I
filed this review request.  Of course, I'm not seeing any actual code for that
one, either, so I'm not sure I can agree that it's "better" than this one (as
touted by its author).  The page has a dead link, and the month-old page hasn't
had an update in nearly three.  So I guess I can add "this one has published
code" to my answer. :-)

I'll readily acknowledge that this plugin is probably sub-par quality. 
Unfortunately, it fulfills a niche for which there aren't really, as far as I've
seen, alternatives.

Comment 6 Peter Lemenkov 2008-07-17 11:35:41 UTC
Ok, I asked in fedora-devel maillist and this seems to be a current nagios
package drawback (I mean arch/noarch issue with %{_libdir}). Seems that we may
safely ignore it. All we need is to supply "%define debug_package %{nil}" in
spec-file (see link below)

http://peter.fedorapeople.org/nagios-plugins-rsync.spec


Comment 7 Peter Lemenkov 2008-07-23 10:02:25 UTC
Ping!

Comment 8 Peter Lemenkov 2008-08-20 10:06:24 UTC
Again, ping!

Comment 9 Peter Lemenkov 2008-09-27 17:39:03 UTC
Ping.
Jima, are you still interested in packaging this? There is only a little way to go and push this package to Fedora.

Comment 10 Fedora Update System 2008-10-08 05:37:28 UTC
nagios-plugins-check_sip-1.2-5.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/nagios-plugins-check_sip-1.2-5.fc9

Comment 11 Fedora Update System 2008-10-08 05:37:32 UTC
nagios-plugins-check_sip-1.2-5.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/nagios-plugins-check_sip-1.2-5.fc8

Comment 12 Peter Lemenkov 2008-10-08 05:49:51 UTC
Oops, sorry for noise

Comment 13 Jima 2008-10-08 12:05:36 UTC
Nah, thanks for the noise.  I need to deal with this.

I've bumped this package up to 1.02, released January of this year.  (Wow.)  Obviously, I also included your fix, which kills one of the rpmlint errors.

Spec URL:
http://beer.tclug.org/jima/fedora/nagios-plugins-rsync/nagios-plugins-rsync.spec
SRPM URL:
http://beer.tclug.org/jima/fedora/nagios-plugins-rsync/nagios-plugins-rsync-1.02-0.1.fc9.src.rpm

This brings rpmlint's output down to:

nagios-plugins-rsync.x86_64: W: no-documentation
nagios-plugins-rsync.x86_64: E: no-binary
nagios-plugins-rsync.x86_64: E: only-non-binary-in-usr-lib

(ditto on i386)

Comment 14 Peter Lemenkov 2008-10-09 13:10:58 UTC
REVIEW:

MUST Items:

- rpmlint is not silent:

[petro@Sulaco SPECS]$ rpmlint ../RPMS/ppc/nagios-plugins-rsync-1.02-0.1.fc9.ppc.rpm 
nagios-plugins-rsync.ppc: W: no-documentation
nagios-plugins-rsync.ppc: E: no-binary
nagios-plugins-rsync.ppc: E: only-non-binary-in-usr-lib
1 packages and 0 specfiles checked; 2 errors, 1 warnings.
[petro@Sulaco SPECS]$ 

Fortunately, these messages may be safely omitted.

+ The package is named according to the Package Naming Guidelines .
+ The spec file name matches the base package %{name}, in the format %{name}.spec
+ The package meets the Packaging Guidelines .
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines .
+ The License field in the package spec file matches the actual license.
+ The source package doesn't include the text of the license(s) in its own file.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package matches the upstream source, as provided in the spec URL.

[petro@Sulaco SOURCES]$ md5sum check_rsync*
bebe128f15e073abf414930e594a7984  check_rsync
bebe128f15e073abf414930e594a7984  check_rsync.new
[petro@Sulaco SOURCES]$ 

+ The package  successfully compiles and builds into binary rpms on at least one supported architecture (ppc).
+ All build dependencies are listed in BuildRequires.
+ No locales
+ No shared libraries
+ The package isn't designed to be relocatable
+ The package doesn't create directories.
+ The package does not contain any duplicate files in the %files listing.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT .
+ The package consistently uses macros, as described in the macros section of Packaging Guidelines .
+ The package contains code, or permissable content.
+ No large documentation files
+ The package doesn't include anything as %doc.
+ No header files.
+ No static libraries.
+ No pkgconfig(.pc) files.
+ No library files.
+ No devel packages
+ No .la libtool archives.
+ Not a GUI app
+ The package does not own files or directories already owned by other packages. + At the beginning of %install, the package runs rm -rf $RPM_BUILD_ROOT
+ All filenames in rpm packages are valid UTF-8.

SHOULD Items:

- SHOULD: The packager SHOULD query upstream to include license text(s) as a separate file.

This package is 

APPROVED

Comment 15 Jima 2008-10-09 13:54:05 UTC
Pinging upstream, will hold off on importing.

Comment 16 Peter Lemenkov 2008-11-15 10:29:28 UTC
Ping again, Jima!
We shouldn't wait so long until upstream will deside to add LICENSE. In any case - that's not a blocker :)

Comment 17 Peter Lemenkov 2008-12-09 14:30:57 UTC
Ping!

Comment 18 Peter Lemenkov 2009-02-02 13:44:59 UTC
Ping!
Jima, are you still interested in maintaining this?

Comment 19 Peter Lemenkov 2009-03-01 08:53:11 UTC
Ping.

Comment 20 Peter Lemenkov 2009-03-15 07:24:55 UTC
Seems, that initial reporter lost interest.


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