Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: nagios-plugins-bdii - Nagios Probe for the BDII
https://bugzilla.redhat.com/show_bug.cgi?id=754137
Summary: Review Request: nagios-plugins-bdii - Nagios Probe for the BDII Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: unspecified Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: Laurence.Field@cern.ch QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: ---
Spec URL: http://lfield.web.cern.ch/lfield/nagios-plugins-bdii.spec SRPM URL: lfield.web.cern.ch/lfield/nagios-plugins-bdii-1.0.11-1.el5.src.rpm Description: Nagios Probe for the BDII (Already in Fedora/EPEL)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #1 from Laurence.Field@cern.ch 2011-11-15 09:01:38 EST --- Steve, Please could you take a look at this package too. Thanks :)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
Steve Traylen steve.traylen@cern.ch changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |steve.traylen@cern.ch Blocks| |177841(FE-NEEDSPONSOR)
--- Comment #2 from Steve Traylen steve.traylen@cern.ch 2011-11-15 10:38:37 EST --- Please remember to add the FE-NEEDSPONSOR block above until you have a sponsor.
Steve.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #3 from Steve Traylen steve.traylen@cern.ch 2011-11-15 16:25:10 EST --- Quick first pass:
1) rpmlint of the .spec file
./nagios-plugins-bdii.spec:24: W: rpm-buildroot-usage %build make compile prefix=%{buildroot}
It's odd that the build depends upon the buildroot. The build root is temporary location only used during the build and not in the final package, does the build and compile really depend upon this?
./nagios-plugins-bdii.spec: W: no-cleaning-of-buildroot %install
Do you plan to target EPEL5 as well? in which case your %install needs a clean.
2) The nagios-plugins-bdii-1.0.11-1.fc16.x86_64.rpm package contains just one file.
/usr/lib64/nagios/plugins/contrib/check_bdii_entries
You don't require anything that contains or create the contrib directory yourself, see http://fedoraproject.org/wiki/Packaging/Guidelines#FileAndDirectoryOwnership
more over I see no reason why this is in crontrib, no other nagios sensors appear to be, please can you be consistent to nagios-plugins which must be the reference I would guess.
2.5) Having absolutely no documentation is odd. Add a README.Fedora or ask upstream to include a README at the very least.
3) Url: vs URL: URL: is more conventional and it's an acronym after all.
4)The SPEC file says ASL 2.0 but there is no indication as to any license within the source code as far as I can see. Contact upstream and ask them to add a license or at the very least have them clarify the license, if that's all they do include the email reply.
5) BuildRequires: openldap-devel Include the architecture you require as well to avoid pointless package installs during resolution, ie.
BuildRequires: openldap-devel%{?_isa}
6) Requires: openldap is not required since this is determined automatically and
7) The CFLAGS are incorrect: /usr/bin/gcc -Wall -O2 -g -DLDAP_DEPRECATED -lldap src/check_bdii_entries.c -o /builddir/build/BUILD/nagios-plugins-bdii-1.0.11/build/check_bdii_entries
see http://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #4 from Laurence.Field@cern.ch 2011-11-16 04:51:09 EST --- Here is a new version
Spec URL: http://lfield.web.cern.ch/lfield/nagios-plugins-bdii.spec SRPM URL: lfield.web.cern.ch/lfield/nagios-plugins-bdii-1.0.12-1.el5.src.rpm
Changes:
1) I have removed the use of buildroot for the compile and now remove the buildroot before the install as I am also targeting EPEL5.
2) I have put the plugin in /usr/lib64/nagios/plugins/ and added this directory to the package. I am now consistent with other nagios plugins.
rpm -ql nagios-plugins-ldap /usr/lib64/nagios/plugins/check_ldap /usr/lib64/nagios/plugins/check_ldaps
2.5) Having absolutely no documentation is odd. This is consistent with other plugins (see above).
3) I have changed Url to URL in the specfile.
4) Upstream has added a license header to the source resulting in a new version.
5) BuildRequires is now openldap-devel%{?_isa}
6) Requires: openldap has been removed from the specfile.
7) The setting of CFLAGS has been removed.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #5 from Steve Traylen steve.traylen@cern.ch 2011-11-16 06:07:30 EST --- Add a protocol to your SRPM URL next time, easier to copy paste.
Please reread http://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
CFLAGS is not meant to be unset. Steve
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #6 from Laurence.Field@cern.ch 2011-11-16 06:26:34 EST --- I didn't unset it, just removed the setting of it in the Makefile. It how uses the one from the default environment.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #7 from Steve Traylen steve.traylen@cern.ch 2011-11-16 07:13:39 EST --- Look at some other examples of CFLAGS being set within Fedora .spec files. e.g
http://pkgs.fedoraproject.org/gitweb/?p=dpm-dsi.git;a=blob;f=dpm-dsi.spec;h=...
Steve.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #8 from Laurence.Field@cern.ch 2011-11-16 07:49:21 EST --- Sorry, I misunderstood. I have uploaded a new specfile.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #9 from Steve Traylen steve.traylen@cern.ch 2011-11-16 07:53:36 EST --- Please actually check the resulting compiler flags being used rather than edit and hope.
They have not changed.
/usr/bin/gcc -lldap src/check_bdii_entries.c -o /afs/cern.ch/user/s/straylen/rpmbuild/BUILD/nagios-plugins-bdii-1.0.12/build/che ck_bdii_entries
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #10 from Laurence.Field@cern.ch 2011-11-16 09:49:39 EST --- I have updated the specfile. Here is the result from the test.
rpmbuild --rebuild build/SRPMS/nagios-plugins-bdii-1.0.12-1.el5.src.rpm 2>/dev/null | grep gcc /usr/bin/gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -lldap src/check_bdii_entries.c -o /usr/src/redhat/BUILD/nagios-plugins-bdii-1.0.12/build/check_bdii_entries
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
Steve Traylen steve.traylen@cern.ch changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Status Whiteboard| |BuildFails Flag| |fedora-review?
--- Comment #11 from Steve Traylen steve.traylen@cern.ch 2011-11-22 16:41:16 EST ---
Having added the
%dir %{_libdir}/nagios/plugins/
directory to your package the same story is true for
%{_libdir}/nagios
I have since learnt that the nagios-common package is the trick here.
$ rpm -ql nagios-plugins
contains
/usr/lib64/nagios/plugins
and you should drop %dir %{_libdir}/nagios/plugins/ and instead require this package.
This package is a common package for all nagios plugins and contains very little.
Steve.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
Steve Traylen steve.traylen@cern.ch changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |steve.traylen@cern.ch Status Whiteboard|BuildFails |
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #12 from Laurence.Field@cern.ch 2011-11-29 09:39:14 EST --- I have updated the specfile so that the package requires nagios-common and the %dir line has been removed. A mistake in the description was also addressed. (Was pointed out by A. Elwell)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #13 from Steve Traylen steve.traylen@cern.ch 2011-11-29 13:51:18 EST --- Please provide a link to a new RPM and .spec file with increased release number.
Steve.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #14 from Laurence.Field@cern.ch 2011-11-30 04:05:56 EST --- Here is a new version with an increased release number
Spec URL: http://lfield.web.cern.ch/lfield/nagios-plugins-bdii.spec SRPM URL: http://lfield.web.cern.ch/lfield/nagios-plugins-bdii-1.0.12-2.el5.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #15 from Steve Traylen steve.traylen@cern.ch 2011-11-30 19:08:01 EST ---
1)
Changelog does not match release, and the spec file has changed with no corresponding changelog entry reflecting the change made in the .spec file.
Please run rpmlint at every release:
$ rpmlint ../RPMS/x86_64/nagios-plugins-bdii-* nagios-plugins-bdii.x86_64: W: incoherent-version-in-changelog 1.0.12-1.el6 ['1.0.12-2.el6', '1.0.12-2'] nagios-plugins-bdii.x86_64: W: no-documentation
2) Can you take a look at or report upstream the compilation warnings, not a requirement but worth following up.
rc/check_bdii_entries.c: In function 'main': src/check_bdii_entries.c:168: warning: implicit declaration of function 'ldap_init' src/check_bdii_entries.c:168: warning: assignment makes pointer from integer without a cast ....
2) The %description is rather terse and includes BDII which is not described anywhere and is not obvious to people in general I believe.
3) The tar ball in the .src.rpm does not match the instructions to create it.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #16 from Steve Traylen steve.traylen@cern.ch 2011-11-30 19:11:49 EST ---
Can you at least request that upstream adds a LICENSE file to their tar ball please.
http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
Steve.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #17 from Laurence.Field@cern.ch 2011-12-02 10:33:28 EST --- Spec URL: http://lfield.web.cern.ch/lfield/nagios-plugins-bdii.spec SRPM URL: http://lfield.web.cern.ch/lfield/nagios-plugins-bdii-1.0.13-1.el5.src.rpm
Upstream have provided a new version (1.0.13) which addresses the compilation warnings and includes a LICENSE file in the source. The description has also been changed so that it is consistent with the other nagios-plugin packages. The tar ball in the srpm should now be identical what is obtained using the instructions specified in the spec file.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #18 from Laurence.Field@cern.ch 2011-12-02 10:37:35 EST --- Sorry wrong link.
SRPM URL: http://lfield.web.cern.ch/lfield/nagios-plugins-bdii-1.0.13-1.el6.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #19 from Laurence.Field@cern.ch 2011-12-05 10:50:57 EST --- Scratch build result
http://koji.fedoraproject.org/koji/taskinfo?taskID=3563946
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
Steve Traylen steve.traylen@cern.ch changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #20 from Steve Traylen steve.traylen@cern.ch 2011-12-07 13:34:08 EST ---
Source matches after s/_1_0_12/_1_0_13/
svn export http://svnweb.cern.ch/guest/gridinfo/nagios-plugins-bdii/tags/R_1_0_13 nagios-plugins-bdii-1.0.13
I believe the %description should be longer but it is not a requirement.
Check it basically runs:
/usr/lib64/nagios/plugins/check_bdii_entries Error: Hostname ( -H option ) not specified.
Usage: check_bdii_entries -H host [OPTION]
Options: -p The port of the BDII -b The base of the BDII -f The filter to use for the search -f Note: Shortcuts 'site', 'service' and 'stats' can be
Fine
Only problem
As there is now a license file please include it :
https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
Please fix the above before import.
PACKAGE APPROVED
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
Steve Traylen steve.traylen@cern.ch changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841(FE-NEEDSPONSOR) |
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #21 from Laurence.Field@cern.ch 2011-12-08 05:37:29 EST --- The description is consistent with other nagios plugins
rpm -qli nagios-plugins-ldap Description: Provides check_ldap support for Nagios.
rpm -qli nagios-plugins-http Description: Provides check_http support for Nagios.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #22 from Laurence.Field@cern.ch 2011-12-12 06:11:14 EST --- Spec URL: http://lfield.web.cern.ch/lfield/fedora/nagios-plugins-bdii.spec SRPM URL: http://cern.ch/lfield/fedora/nagios-plugins-bdii-1.0.14-1.fc16.src.rpm
Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=3579249
New version to address issues raised.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #23 from Laurence.Field@cern.ch 2011-12-12 06:13:20 EST --- New Package SCM Request ======================= Package Name: nagios-plugins-bdii Short Description: Nagios Plugin - check_bdii_entries Owners: lfield Branches: f16 el5 el6 InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
Laurence.Field@cern.ch changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #24 from Jon Ciesla limburgher@gmail.com 2011-12-12 08:45:25 EST --- Git done (by process-git-requests).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #25 from Fedora Update System updates@fedoraproject.org 2011-12-12 10:20:52 EST --- nagios-plugins-bdii-1.0.14-1.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/nagios-plugins-bdii-1.0.14-1.fc16
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #26 from Fedora Update System updates@fedoraproject.org 2011-12-12 10:26:55 EST --- nagios-plugins-bdii-1.0.14-1.el5 has been submitted as an update for Fedora EPEL 5. https://admin.fedoraproject.org/updates/nagios-plugins-bdii-1.0.14-1.el5
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #27 from Fedora Update System updates@fedoraproject.org 2011-12-12 10:32:48 EST --- nagios-plugins-bdii-1.0.14-1.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/nagios-plugins-bdii-1.0.14-1.el6
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #28 from Fedora Update System updates@fedoraproject.org 2011-12-12 14:26:57 EST --- nagios-plugins-bdii-1.0.14-1.el6 has been pushed to the Fedora EPEL 6 testing repository.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Fixed In Version| |nagios-plugins-bdii-1.0.14- | |1.fc16 Resolution| |ERRATA Last Closed| |2011-12-22 17:45:01
--- Comment #29 from Fedora Update System updates@fedoraproject.org 2011-12-22 17:45:01 EST --- nagios-plugins-bdii-1.0.14-1.fc16 has been pushed to the Fedora 16 stable repository.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|nagios-plugins-bdii-1.0.14- |nagios-plugins-bdii-1.0.14- |1.fc16 |1.el6
--- Comment #30 from Fedora Update System updates@fedoraproject.org 2011-12-29 17:00:00 EST --- nagios-plugins-bdii-1.0.14-1.el6 has been pushed to the Fedora EPEL 6 stable repository.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|nagios-plugins-bdii-1.0.14- |nagios-plugins-bdii-1.0.14- |1.el6 |1.el5
--- Comment #31 from Fedora Update System updates@fedoraproject.org 2011-12-29 17:01:31 EST --- nagios-plugins-bdii-1.0.14-1.el5 has been pushed to the Fedora EPEL 5 stable repository.
https://bugzilla.redhat.com/show_bug.cgi?id=754137
Steve Traylen steve.traylen@cern.ch changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs+ |fedora-cvs?
--- Comment #32 from Steve Traylen steve.traylen@cern.ch --- Package Change Request ====================== Package Name: nagios-plugins-bdii New Branches: epel7 Owners: stevetraylen
https://bugzilla.redhat.com/show_bug.cgi?id=754137
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs? |fedora-cvs+
https://bugzilla.redhat.com/show_bug.cgi?id=754137
--- Comment #33 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla