Request for Review: libstatgrab

Oliver Falk oliver at linux-kernel.at
Wed Jul 6 15:44:33 UTC 2005


Hi Spot!

On 07/06/2005 04:54 PM, Tom 'spot' Callaway wrote:
> 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.

But thanks a lot for rewriting it.... I'm currently a bit busy so it 
helped a lot!!!

Just recreated the srpm and uploaded the spec:
http://filelister.linux-kernel.at/mod_perl?current=/packages/FC_EXTRAS_APPROVAL/libstatgrab

I just changed a few identation things, nothing relevant.

You may have a look at it again and approve it now.

Best,
  Oliver




More information about the fedora-extras-list mailing list