Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=433547
Summary: Review Request: nagios-nsca - nagios passive check daemon Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: wart@kobold.org QAContact: extras-qa@fedoraproject.org CC: fedora-package-review@redhat.com,notting@redhat.com
Spec URL: http://www.kobold.org/~wart/fedora/nagios-nsca.spec SRPM URL: http://www.kobold.org/~wart/fedora/nagios-nsca-2.7.2-3.src.rpm Description: NSCA is a daemon that lives on the nagios server and receives passive check results over the network. It supports encrypted communications between the nsca client and nsca server.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nagios-nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
------- Additional Comments From xavier@bachelot.org 2008-02-20 06:37 EST ------- fwiw, I was also working on nsca lately. I uploaded my package and spec if you want to take a look (this is still a work in progress) : http://washington.kelkoo.net/fedora/SPECS/nsca.spec http://washington.kelkoo.net/fedora/SRPMS/nsca-2.7.2-2.fc8.src.rpm
I did not had time to look closely at your package but I will as soon as I have a bit of time.
Regards, Xavier
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nagios-nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
------- Additional Comments From wart@kobold.org 2008-02-21 15:26 EST ------- Hi Xavier,
It looks like we started from the same DAG rpm. :) Don't forget that the /etc/nagios/*.cfg files need to be mode 0600 and readable only by root because they contain encryption passwords.
Since you've also worked on the same package, would you like to review and/or comaintain this with me?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nagios-nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
xavier@bachelot.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |xavier@bachelot.org
------- Additional Comments From xavier@bachelot.org 2008-02-22 04:48 EST ------- (In reply to comment #2)
Hi Xavier,
Hi Wart,
It looks like we started from the same DAG rpm. :) Don't forget that the /etc/nagios/*.cfg files need to be mode 0600 and readable only by root because they contain encryption passwords.
Since you've also worked on the same package, would you like to review and/or comaintain this with me?
Sure, that's the plan, but I'm short on time at the moment so I just pointed you at my unfinished work so you may (or may not) cherry-pick from it. I'll get back to it next week hopefully.
Regards, Xavier
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nagios-nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
------- Additional Comments From xavier@bachelot.org 2008-02-28 12:42 EST ------- Hi Wart,
Here are my preliminary comments on your package :
Summary is not capitalized.
I like nsca-client much better than what I used. Is it really useful to suffix the packages with nagios- though ? This would clear rpmlint output from nagios-nsca.i386: W: incoherent-subsys /etc/rc.d/init.d/nsca $prog nagios-nsca.i386: W: incoherent-init-script-name nsca
nagios, nrpe and all are in the group Applications/System. You probably want to use the same.
I don't have the BR: on libmcrypt-devel and my package build fine. Is it necessary to keep it ?
I have some requires on /sbin/service and /sbin/chkconfig, taken from nrpe packages, not sure this is needed, can't find anything in the packaging guidelines. Requires(preun): /sbin/service, /sbin/chkconfig Requires(post): /sbin/chkconfig, /sbin/service Requires(postun): /sbin/service
NetSaint can probably be stripped out of the description.
Upstream ships an initscript (generated at build time actually) that only lacks reload, it might not worth to replace it completely.
0600 on the conf files is good.
No doc in nsca-client, add %doc Changelog LEGAL README SECURITY
Nothing more for today, let me know what you think.
Regards, Xavier
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nagios-nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
------- Additional Comments From xavier@bachelot.org 2008-03-03 06:41 EST ------- 2 enhancements to my earlier comments:
I don't have the BR: on libmcrypt-devel and my package build fine. Is it necessary to keep it ?
Indeed, this is needed... Sorry, temporary brain failure.
I have some requires on /sbin/service and /sbin/chkconfig, taken from nrpe packages, not sure this is needed, can't find anything in the packaging
guidelines.
Requires(preun): /sbin/service, /sbin/chkconfig Requires(post): /sbin/chkconfig, /sbin/service Requires(postun): /sbin/service
Yes, this is needed, here's the reference : http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-97754e2c64661...
I'll post an updated spec later.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nagios-nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
------- Additional Comments From wart@kobold.org 2008-03-09 21:43 EST ------- Here's another iteration based on the package that you mailed me (thanks!). I think we should keep 'NetSaint' in the description because we're not disabling any netsaint functionality. I also merged the sed script that was modifying the init script into the existing init script patch. It seemed silly to use both a patch and a sed script for the same file:
http://www.kobold.org/~wart/fedora/nsca.spec http://www.kobold.org/~wart/fedora/nsca-2.7.2-4.fc8.src.rpm
I haven't tried the new init script myself; I'll take your word that it works as expected.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nagios-nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
------- Additional Comments From xavier@bachelot.org 2008-03-10 14:39 EST ------- (In reply to comment #6)
Here's another iteration based on the package that you mailed me (thanks!). I think we should keep 'NetSaint' in the description because we're not disabling any netsaint functionality.
Ok
I also merged the sed script that was modifying the init script into the existing init script patch. It seemed silly to use both a patch and a sed script for the same file:
Agreed.
http://www.kobold.org/~wart/fedora/nsca.spec http://www.kobold.org/~wart/fedora/nsca-2.7.2-4.fc8.src.rpm
I haven't tried the new init script myself; I'll take your word that it works as expected.
I need to diff the old one and the new one to make sure it's ok.
I see you actually used my spec as the new base, I hope this won't disqualify me for the formal review. I'll put it in the next comment. Also this is my first formal review, so I'll ask my sponsor to take a look.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nagios-nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
------- Additional Comments From xavier@bachelot.org 2008-03-10 14:45 EST ------- + source files match upstream: 33a98e7975f633a9489d7a8938ed6131
+ package meets naming and versioning guidelines.
+ specfile is properly named, is cleanly written and uses macros consistently.
+ dist tag is present.
+ build root is correct. %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
+ license field matches the actual license.
+ license is open source-compatible. License text not included upstream.
+ latest version is being packaged.
+ BuildRequires are proper.
+ compiler flags are appropriate.
+ package builds in mock ( ).
+ package installs properly
+ debuginfo package looks complete.
+ rpmlint is not silent, but the 2 warnings are expected : nsca.i386: E: non-readable /etc/nagios/nsca.cfg 0600 nsca-client.i386: E: non-readable /etc/nagios/send_nsca.cfg 0600
+ final provides and requires are sane: Provides: config(nsca) = 2.7.2-4.el5 Requires(interp): /bin/sh /bin/sh /bin/sh Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 Requires(post): /bin/sh /sbin/chkconfig Requires(preun): /bin/sh /sbin/chkconfig /sbin/service Requires(postun): /bin/sh /sbin/service Requires: /bin/sh config(nsca) = 2.7.2-4.el5 libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1) libc.so.6(GLIBC_2.3) libc.so.6(GLIBC_2.3.4) libc.so.6(GLIB C_2.4) libmcrypt.so.4 libnsl.so.1 nagios rtld(GNU_HASH)
Provides: config(nsca-client) = 2.7.2-4.el5 Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 Requires: config(nsca-client) = 2.7.2-4.el5 libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1) libc.so.6(GLIBC_2.3) libc.so.6(GLIBC_2.3.4) libc.so.6(GLIBC_2.4) libmcrypt.so.4 libnsl.so.1 rtld(GNU_HASH)
+ no shared libraries are added to the regular linker search paths. - owns the directories it creates. nsca-client should own /etc/nagios because it doesn't require nagios. nsca doesn't because it requires nagios.
+ doesn't own any directories it shouldn't. + no duplicates in %files. + file permissions are appropriate. + scriptlets follow guidelines. + code, not content. + documentation is small, so no -docs subpackage is necessary. + %docs are not necessary for the proper functioning of the package. + no headers. + no pkgconfig files. + no libtool .la droppings. + not a GUI app.
just add %dir %{_sysconfdir}/nagios for the client and it should be good to go.
additional note : nsca-2.7.2-initreload.patch might need to be renamed now it does more than just add reload to the init script.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nagios-nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
------- Additional Comments From xavier@bachelot.org 2008-03-11 13:02 EST ------- I went ahead with the 2 changes above and fixed the initscript at the same time (broken reload, missing condrestart and both of them missing from the usage message).
http://washington.kelkoo.net/fedora/SPECS/nsca.spec http://washington.kelkoo.net/fedora/SRPMS/nsca-2.7.2-5.fc8.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nagios-nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
------- Additional Comments From sstarr@platform.com 2008-03-17 18:52 EST ------- You depending on the nagios package for both the client and server. Only server needs nagios itself installed.
What about the nagios user? Since your compiling NSCA requiring the nagios using being present you'll need the user. This package should attempt to create the user and otherwise silently ignore if it exists already.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nagios-nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
------- Additional Comments From xavier@bachelot.org 2008-03-18 04:53 EST ------- (In reply to comment #10)
You depending on the nagios package for both the client and server. Only server needs nagios itself installed.
Only nsca (the daemon) Requires: nagios. nsca-client doesn't.
What about the nagios user? Since your compiling NSCA requiring the nagios using being present you'll need the user. This package should attempt to create the user and otherwise silently ignore if it exists already.
The nagios user is not required at compile time, the --with-nsca-user and --with-nsca-grp option are only used to generate the default config. The nagios user will only be needed at run time and then nsca Requires: nagios so it will be already there.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nagios-nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
------- Additional Comments From tibbs@math.uh.edu 2008-03-18 17:56 EST ------- Xavier asked me to have a quick look. Sorry I couldn't get to it yesterday.
A few meta-issues first:
Xavier, if you indeed are reviewing this, you should assign the ticket to yourself and set the fedora-review flag to '?'.
The summary of the ticket should match the submitted package name. Was it de
I don't have any particular issues with the reviewer providing fixed packages, but Michael does need to respond to the stuff in comments 8 and 9 before this can move anywhere.
I am unfamiliar with nagios and related software, so I can really only look at the basic packaging, which looks fine. I'm looking at the package linked in comment 9. The scriptlets aren't exactly the same as the recommended ones, but the differences are inconsequential. Do note, however, that the "|| :" bit only has an effect as the last executed command in a scriptlet; its sole purpose is to prevent the scriptlet from returning a nonzero exit code.
Also note that I tend to just drop the bits of my review template that don't apply, such as debuginfo and compiler flag bits for a noarch package, and the GUI app bit for a non-gui app. They're just placeholders to remind me to check things when they apply.
Finally, I see some source files that are obviously GPLv2+, and others (nsca.c, send_nsca.c) which say GPLv2. Someone should ping upstream and see if they intend GPLv2 only for the final product. Otherwise I don't think GPLv2+ is accurate for the license tag.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
xavier@bachelot.org changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |xavier@bachelot.org Status|NEW |ASSIGNED Summary|Review Request: nagios-nsca |Review Request: nsca - |- nagios passive check |nagios passive check daemon |daemon | Flag| |fedora-review?
------- Additional Comments From xavier@bachelot.org 2008-03-18 19:35 EST ------- Thanks for your comments Jason. I've already addressed the issues from comment 8 in the package in comment 9. So the last remaining issues would be the preun and postun scriptlets and the License: tag. Michael, could you please look at the scriptlets[1] and contact upstream about the license issue ?
FWIW, I'm interested in co-maintaining this package, at least for EPEL.
[1] http://fedoraproject.org/wiki/Packaging/ScriptletSnippets?action=show&re...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
------- Additional Comments From wart@kobold.org 2008-03-18 19:47 EST ------- Sorry for the silence. Xavier has pretty much been proactive in updating the review package as needed. If he'd like to become the primary maintainer and have me as a comaintainer, I'd be more than happy to hand this review over to him.
In the meantime I'll ping upstream about the licensing issue.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
------- Additional Comments From xavier@bachelot.org 2008-03-18 20:00 EST ------- I believe I can't be the primary maintainer and do the review at the same time. You submitted the review, so you should be the primary maintainer. Like I wrote, I'm genuinely interested in having this in EPEL, that's why I have been so active. Sorry to be pushy, I certainly don't want to look as making a take over on the package. Actually, I think we did a pretty good complementary job :-)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
------- Additional Comments From xavier@bachelot.org 2008-04-01 11:10 EST ------- Wart, still around ? Did upstream get back to you wrt license ? Also, do you want me to fix the scriplets to be as in the guidelines ? I'd like to get this package built and released asap.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
------- Additional Comments From wart@kobold.org 2008-04-01 23:37 EST ------- Upstream never responded to the licensing question, so I've changed the license to GPLv2 to best match the sources. I also updated the scriplets to use || : on the last command.
http://www.kobold.org/~wart/fedora/nsca-2.7.2-6.fc8.src.rpm http://www.kobold.org/~wart/fedora/nsca.spec
I think we're almost there...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
xavier@bachelot.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
------- Additional Comments From xavier@bachelot.org 2008-04-02 04:47 EST ------- looks good, APPROVED.
I'm willing to co-maintain this package for EPEL. I don't mind being comaintainer for Fedora too if you wish so. My username is xavierb.
Cheers, Xavier
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
wart@kobold.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
------- Additional Comments From wart@kobold.org 2008-04-03 16:07 EST ------- New Package CVS Request ======================= Package Name: nsca Short Description: Nagios passive check daemon Owners: wart, xavierb Branches: F-8 EL-4 EL-5 InitialCC: Cvsextras Commits:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
------- Additional Comments From kevin@tummy.com 2008-04-03 16:19 EST ------- cvs done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: nsca - nagios passive check daemon
https://bugzilla.redhat.com/show_bug.cgi?id=433547
wart@kobold.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
------- Additional Comments From wart@kobold.org 2008-04-03 17:54 EST ------- Imported and built for Rawhide. Builds for F-8 and EL-{4,5} are coming along soon.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla