I've updated the default location of nagios-plugins to /usr/lib/nagios/plugins

SPEC: http://preview.iesabroad.org/~mmcgrath/nagios/nagios.spec
SRPM: http://preview.iesabroad.org/~mmcgrath/nagios/nagios-1.3-2.src.rpm

I tried to find a sponsor & reviewer, but a bunch of people are having trouble
looking up that hostname, myself included. Please advise.

Sorry about that, our upstream provider has had an outage since about 8:00 this
morning.  I've relocated these files:

http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
http://mmcgrath.net/~mmcgrath/nagios/nagios-plugins-1.4.2-3.src.rpm

- Source0 should be a proper URL
- Prereq should be Requires(pre)
- -devel should also require by release
- Commented-out logrotate script refers to cacti user and group, but these are
not created anywhere nor is there a dependency on anything that does
- Typo at --libexecdir
- Typo at --enable-enbedded-perl
- Is there a reason why you disable the initscript on runlevel 4?
- Requires(pre): %{_sbindir}/useradd
- Requires(preun): /sbin/service /sbin/chkconfig
- Requires(post): /sbin/chkconfig /sbin/service %{_sbindir}/usermod
- Requires(postun): /sbin/service
- Recommend reordering %files so that the various directories are grouped together
- Any reason why %defattr is used so many times?
- Any reason why -devel doesn't just own %{_includedir}/%{name}?

Thanks for helping me review nagios.  I've implemented all the changes from your
comments except for one.  When I try to run chkconfig --add nagios with the
following chkconfig comment:

# chkconfig: - 345 99 01

I get:

service nagios does not support chkconfig

However, when I run chkconfig against the same comment (minus the 4) it works fine:

# chkconfig: - 35 99 01

Any ideas?

    -Mike

(In reply to comment #5)
[trimmed for brevity]
> # chkconfig: - 345 99 01
> 
> service nagios does not support chkconfig
> 
> # chkconfig: - 35 99 01
> 
> Any ideas?

Too many arguments to chkconfig.  Check what chkconfig did.  I bet it created a
S35nagios in runlevels 2, 3, 4, and 5, and a K99nagios in 0, 1, and 6, and the
01 was completely ignored.

Use

# chkconfig: - 99 01

Ignore that last comment, I have no idea how I expected that to behave :-) It's
all fixed now (and disabled by default since it ships with non-working configs)

http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-4.src.rpm
http://mmcgrath.net/~mmcgrath/nagios/nagios.spec

- Source0 should be http://dl.sourceforge.net/nagios/%{name}-%{version}.tar.gz
- Extra space on --libexecdir
- sed substitutions should be done in %build
- The sed substitution on p1.pl could be replaced with an insert (non-blocker)
- All of the files you list in the 2 %defattr sections (with the exception of
p1.pl) already have those permissions, so you can combine them and replace with
%defattr(-,root,root) and then merge redundant entries (e.g., %{_datadir}/%{name})
- The permissions on the doc dir are wrong, but I suspect that is because of the
%defattr
- %{_sysconfdir}/logrotate.d/nagios is not marked %config(noreplace)

Thanks for your help on this, these changes have been made:

http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-5.src.rpm

* is '%{_localstatedir}/%{_lib}' really sane? nagios seems to be the
  only package using /var/lib64 and FHS 2.3 mentions only /lib64 +
  /usr/lib64.

  Have the libexec + cgi-bin files really have to reside under /var or
  would /usr a better place for them (dynamically created programs are
  looking a little bit hairy to me)?

* the '%{_localstatedir}' macro is a little bit problemetic because
  its value differs between distributions (e.g. mandriva uses /var/run
  for it). Not a problem when packaged for Fedora only but '%_var'
  would be more clear.

* I would add some '-p' (preserve timestamp) options to the %_install
  operations

* I would add '|| :' to some operations (server-restart + usermod) in
  the scriptlets; at least in the %postun one

Most of these decisions I got from examples from other spec files.  For example
the httpd.spec and php.spec files use _localstatedir instead of _var.

I've never used -p with install, what benefits would we gain from this?

I've added || : to usermod but I believe service returns true regardless of
whether httpd restarts.

(In reply to comment #11)
> I've added || : to usermod but I believe service returns true regardless of
> whether httpd restarts.

It will however fail if it can't find the service.

Thats interesting I didn't know that :-D, I've included the || : changes.

SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-6.src.rpm
SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec

Okay, you obviously missed the memo that talked about consolidating
%{_datadir}/%{name} into a single entry. Also, since you own everything in
%{_sbindir} you can just use %{_sbindir}/*.

Sorry bout that, I've made that change now:

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-7.src.rpm

rpmlint output:

RPMS/nagios-1.3-7.i386.rpm:
E: nagios non-standard-uid /var/log/nagios/archives nagios
E: nagios non-standard-gid /var/log/nagios/archives nagios
E: nagios non-readable /etc/nagios/private/resource.cfg-sample 0640
E: nagios non-standard-uid /var/spool/nagios nagios
E: nagios non-standard-gid /var/spool/nagios nagios
E: nagios non-standard-dir-perm /var/spool/nagios 02775
E: nagios non-standard-uid /var/log/nagios nagios
E: nagios non-standard-gid /var/log/nagios nagios
E: nagios non-standard-dir-perm /etc/nagios/private 0750

- All ignorable

E: nagios subsys-not-used /etc/rc.d/init.d/nagios

- Bogus, since the initscript does touch /var/lock/subsys/nagios

RPMS/nagios-debuginfo-1.3-7.i386.rpm:
RPMS/nagios-devel-1.3-7.i386.rpm:
W: nagios-devel no-documentation

- Ignorable; there isn't anything appropriate for it

SRPMS/nagios-1.3-7.src.rpm:
E: nagios configure-without-libdir-spec

- Ignorable, since the default as per configure --help is fine

- Spec looks reasonable
- File matches upstream
- Builds under mock in FC4
- The rest looks good

APPROVED

STOP STOP STOP...


there is still no rational for usage of '/var/lib64'

You're right, my bad. Further review required.

I've moved everything that was in %{_localstatedir}/{%_lib} to %{_libdir}.  I
should have done that earlier.

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-8.src.rpm

Also, be aware that people will run nagios-plugins using a single nagios config
entry on multiple machines. So it is very important that @USER1@ will work on
all architectures. This is a limitation of nagios, but the nagios-plugins should
accomodate those. What I have done is:

install in /usr/lib/nagios/plugins
create a softlink of /var/lib/nagios to /usr/lib/nagios
create a softlink of /usr/lib/nagios/plugins to /usr/lib/nagios/libexec

also: {_sbindir}/useradd -d %{_datadir}/%{name}

I am not sure what _datadir resolves to, but I believe either /usr/lib/ or
/var/lib, which means that you've put nagios' homedir in the same place as the
plugins. Since ssh checks to see if the user is owning his own homedir, this
requires nagios to have read/write to the plugins directory. I think it is
better to have nagios' homedir elsewhere (eg in /home/nagios), so that it can
have readonly access to its plugins, while not breaking automated logins with
ssh for remote plugin execution.

Additionally, /home can be NFS mounted, so that the ssh key distribution works 
over a large set of machines without the need to manually install nagios' ssh
key everywhere in /usr/lib/nagios or /var/lib/nagios (if that is not nfs
mounted, since it could be that there is a mix of linux/distro/OSes around the
network so that /usrlib/nagios cannot easilly be an NFS export)

In fedora %{_datadir} translates to /usr/share/  perhaps I should create
/var/lib/nagios as a rw directory for Nagios as its home directory.  I thought
about using /var/log/nagios because it's already there and nagios has rw access
to it but that doesn't seem appropriate for a home directory.  

These issues are more nagios-plugins related but I think nagios-plugins and
nagios should have the same home directory for both plugins.

Any thoughts?

Upon inspection of my /etc/passwd file I noticed a number of accounts that use
/var/spool/name as their home directory (mail for example). Nagios already has
rw access to its spool directory so I've made it the nagios home directory as well.

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-9.src.rpm

I had some duplicate build requires:
zlib-devel, libjpeg-devel and libpng-devel are provided by by gd-devel

http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-10.src.rpm

Is %{_datadir}/nagios/html/docs required or could it be marked %doc?

The HTML Doc's are used by the webinterface.  There's a "Documentation" in the
nav. frame that uses it.  I could move it to /usr/share/doc/nagios-1.2/ but I'd
have to create an alias in the apache conf.

It doesn't necessarily have to be moved; it is possible to mark files and dirs
as %doc even though they're outside of the normal path for docs. The tricky part
would be splitting the %files entry.

I did not know that :-P  How's this:

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-11.src.rpm

You're missing the %{_datadir}/%{name}/html directory itself, but yes, just like
that. Although you could consolidate all those new entries into
%{_datadir}/%{name}/html/[^d]* since docs is the only one that starts with d.

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-12.src.rpm

Expressions are our friends.

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-13.src.rpm

I was manually stripping some of the binaries instead of letting
find-debuginfo.sh do it.

Random comments:
There are still spelling problems in the description:
- progam, servicees (?), Nagios spelled both with capital N and small N.
- also sentences missing ending dots.

The logrotate file should also become Source1.

Also, the paragraph "This package provide core programs for nagios. The web
interface, documentation, and development files are built as separate packages"
is now completely wrong.

SPEC: http://mmcgrath.net/~mmcgrath/nagios/nagios.spec
SRPM: http://mmcgrath.net/~mmcgrath/nagios/nagios-1.3-14.src.rpm

-Fixed spelling errors and description
-Moved logrotate to a source file

I know I've said this before but thanks for everyones help on this, we'll be
using it on the Fedora Infrastructure servers soon (whenever it and the plugins
get approved).

Looks good to me.

APPROVED