Request for Review: libstatgrab

Wed Jul 6 14:54:41 UTC 2005

On Wed, 2005-07-06 at 09:18 +0200, Oliver Falk wrote:
> Hi! I'm still awaiting review for the libstatgrab package.
> 
> libstatgrab: Make system statistics

Review of libstatgrab:

Bad:

- BuildRoot not set using FE default. It should be:  
  %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
- BuildRequires contains unnecessary items. Specifically:
  gcc, make, sed (see 
  http://fedoraproject.org/wiki/PackagingGuidelines#Exceptions )
- Requires contains non-existant package: ldconfig
  ldconfig is in glibc, but you don't need to put that as a requires   
  either.
- LGPL License is only correct for the libraries. Subpackage tools is 
  under the GPL. The tools package should include the COPYING (GPL 
  text).
- COPYING.LGPL (LGPL License text) is not included in library package.
- Typo in libstatgrab-tools description (should be "shipped" not 
  "shiped")
- Don't check for $RPM_BUILD_ROOT = / in %install or %clean.
- You're overcomplicating the %post and %postun sections. Just use:
  %post -p /sbin/ldconfig
  %postun -p /sbin/ldconfig
- The devel package needs pkgconfig as a Requires.
- Most of the groups used are invalid
- The examples package is full of trash (.c, .o, .libs), and none of it 
  is docs.
- The fancy formatting confuses the crap out of rpmlint. When I remove 
  it, I get better results.
- You bzip2'd a ~50 line patch, resulting in an amazing savings of 1866 
  B. I have no idea why you did that, but I undid this, so that I could 
  actually look at the patch.
- In the statgrab-utils package, the binaries aren't getting stripped 
  (and handled by the debuginfo package) because of their permissions. 
  Do these binaries really need to be setuid to function? In my 
  testing, these files do NOT need to be setuid to function correctly, 
  so I went ahead and chmod'd 755 those files.

I reworked the package spec extensively to resolve the above issues (the
end result is attached to this email). 

With the new spec, the only rpmlint output is:
statgrab-tools-0.11.1-2.i386.rpm:
W: statgrab-tools
devel-file-in-non-devel-package /usr/bin/statgrab-make-mrtg-config

The devel-file-in-non-devel-package warning can be ignored.

Good:

- meets PackagingGuidelines, PackageNamingGuidelines. 
- source matches upstream, the licensing is correct. 
- spec name matches package name, spec is in Am. English and legible. 
- package successfully compiles and builds on x86 (FC4). 
- no missing BuildRequires, no locales to handle. 
- shared libraries have %post/%postun, static libs and .so are in 
  -devel. 
- pkgconfig file is in -devel, has no extra Requires. 
- package is not relocatable
- %clean section is fine.
- No .la files packaged
- Package is code not content.
- macro use is consistent
- no duplicate files
- no unowned directories created

If the new spec seems reasonable to you, then I'll give it my approval.

Please note that I really don't intend to rewrite every spec I review, I
did so in this case so that it could be a valuable learning experience.

~spot
-- 
Tom "spot" Callaway: Red Hat Senior Sales Engineer || GPG ID: 93054260
Fedora Extras Steering Committee Member (RPM Standards and Practices)
Aurora Linux Project Leader: http://auroralinux.org
Lemurs, llamas, and sparcs, oh my!
-------------- next part --------------
%define shortname	statgrab

Summary:		Make system statistics
Name:			libstatgrab
Version:		0.11.1
Release:		2%{?dist}
Source0:		ftp://ftp.i-scream.org/pub/i-scream/%{name}/%{name}-%{version}.tar.gz
Patch0:			%{name}.nochmod.patch
License:		LGPL
Group:			System Environment/Libraries
Url:			http://www.i-scream.org/%{name}/
BuildRoot:		%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
BuildRequires:		libtool glibc-devel ncurses-devel binutils

%description
Libstatgrab is a library that provides cross platform access to statistics
about the system on which it's run. It's written in C and presents a selection
of useful interfaces which can be used to access key system statistics. The
current list of statistics includes CPU usage, memory utilisation, disk usage,
process counts, network traffic, disk I/O, and more. 

The current list of platforms is Solaris 2.x, Linux, and FreeBSD 4.x/5.x.
The aim is to extend this to include as many operating systems as possible. 

The package also includes a couple of useful tools. The first, saidar,
provides a curses-based interface to viewing the current state of the 
system. The second, statgrab, gives a sysctl-style interface to the
statistics gathered by libstatgrab. This extends the use of libstatgrab
to people writing scripts or anything else that can't easily make C 
function calls. Included with statgrab is a script to generate an MRTG
configuration file to use statgrab. 

%package -n %{shortname}-tools
Summary: Tools from %{name} to monitoring the system
Group: Applications/System

%description -n %{shortname}-tools
This package contains a few tools shiped with libstatgrab.
Eg. A tool called saidar, which shows various system
information like top, but - of course - OTHER informations.

%package devel
Summary: The development files from %{name}
Group: Development/Libraries
Requires: %{name} = %{version}-%{release}
Requires: pkgconfig

%description devel
This package contains header files and man pages for those
use to develop libstatgrab based applications.

%package examples
Summary: The example files from %{name}
Group: Development/Tools
Requires: %{name} = %{version}-%{release}

%description examples
This package contains various examples used to show how
to develop libstatgrab based applications.

%prep
%setup -q
%patch0 -p0

%build
%configure --with-ncurses
make %{?_smp_mflags}

%install
rm -rf $RPM_BUILD_ROOT
make install DESTDIR=$RPM_BUILD_ROOT
cd examples/.libs
install -m 755 cpu_usage disk_traffic load_stats network_iface_stats \
	       network_traffic os_info page_stats process_snapshot   \
	       process_stats user_list vm_stats $RPM_BUILD_ROOT%{_bindir}
chmod 755 $RPM_BUILD_ROOT%{_bindir}/statgrab-make-mrtg-config
chmod 755 $RPM_BUILD_ROOT%{_bindir}/saidar
chmod 755 $RPM_BUILD_ROOT%{_bindir}/statgrab

%clean
rm -rf $RPM_BUILD_ROOT

%post -p /sbin/ldconfig

%postun -p /sbin/ldconfig

%files -n %shortname-tools
%defattr(-,root,root)
%doc COPYING
%{_bindir}/saidar
%{_bindir}/statgrab
%{_bindir}/statgrab-make-mrtg-config
%{_bindir}/statgrab-make-mrtg-index
%{_mandir}/*/statgrab*

%files
%defattr(-,root,root)
%doc AUTHORS INSTALL README ChangeLog NEWS COPYING.LGPL
%{_libdir}/*.so.*

%files devel
%defattr(-,root,root)
%{_libdir}/*.so
%{_libdir}/*.a
%{_libdir}/*.la
%{_includedir}/*.h
%{_libdir}/pkgconfig/%name.pc
%{_mandir}/*/sg_*

%files examples
%defattr(-,root,root)
%{_bindir}/cpu_usage
%{_bindir}/disk_traffic
%{_bindir}/load_stats
%{_bindir}/network_iface_stats
%{_bindir}/network_traffic
%{_bindir}/os_info
%{_bindir}/page_stats
%{_bindir}/process_snapshot
%{_bindir}/process_stats
%{_bindir}/user_list
%{_bindir}/vm_stats

%changelog
* Wed Jul  6 2005 Tom "spot" Callaway <tcallawa at redhat.com> 0.11.1-2
- a lot of fixes for Fedora Extras

* Thu May 19 2005 Oliver Falk <oliver at linux-kernel.at>		- 0.11.1-1.1
- Specfile cleanup

* Sun Apr 03 2005 Oliver Falk <oliver at linux-kernel.at>		- 0.11.1-1
- Update

* Fri Mar 25 2005 Oliver Falk <oliver at linux-kernel.at>		- 0.11-2.1
- Fix rpmlint warnings

* Tue Feb 15 2005 Oliver Falk <oliver at linux-kernel.at>		- 0.11-2
- Don't require coreutils. They are normally installed on Fedora, but
  not available on RH 8, where the tools are usually also installed.
  Yes, rebuilding with nodeps would also do it, but it's not fine...

* Tue Feb 15 2005 Oliver Falk <oliver at linux-kernel.at>		- 0.11-1
- Initial build for Fedora Core