Review needed

[Tom 'spot' Callaway]

On Thu, 2005-06-30 at 16:34 +0200, Andreas Thienemann wrote:
> On Thu, 30 Jun 2005, Matthew Miller wrote:
> 
> > I think it's a lot to do with what people have a personal interest in --
> > *and* personal knowledge of. Same thing happened with my python SOAP
> > packages....
> 
> Yeah. That's most likely the reason, but it shows a major problem in the
> whole process.
> 
> A well. Tom said he'd take a look at it and it's supposed to be on his 
> todo list... So we'll see.

Review of suphp:

Your spec needed a fair amount of cleanup. Specifically:

- You named the package mod_suphp, when it should just be suphp.
 - This also makes the spec file name the same as the package name.
- I removed all the commented out lines that weren't comments.
- You were using a non-standard BuildRoot. Replaced with standard BR.
- I had to correct the apache2 module generation process because %{name}
is no longer mod_suphp.
- You were stripping the apache2 module, this prevents debuginfo from
being useful. Removed this.
- There is no need to check that $RPM_BUILD_ROOT is /. Its defined in
the spec. Removed all of those checks.
- Made use of %{__install} consistent (sometimes, you just used
"install")
- Copy of GPL license not in %doc

I've attached a new spec that makes those changes. The remainder of the
review is against THAT spec.

rpmlint checks:
suphp-0.5.2-4.i386.rpm
W: suphp unstripped-binary-or-object /usr/sbin/suphp
E: suphp setuid-binary /usr/sbin/suphp root 04755
E: suphp non-standard-executable-perm /usr/sbin/suphp 04755

The setuid binary and nonstandard permissions are expected, and the
unstripped binary is safe to ignore (debuginfo isn't grabbing it
anyway).

All rpmlint checks safe to ignore.

Good:

- Meets PackageNamingGuidelines
- Meets PackagingGuidelines
- Sources match upstream versions
- Package successfully builds on x86 (FC4)
- Package has no locales to check
- Package has no shared libraries to check
- Package has no scriptlets to check
- Spec file is in American English
- Package is not relocatable
- Package owns all directories it creates
- Package has no duplicate files
- Package has no "safe" BuildRequires
- Package lists all necessary BuildRequires
- License OK (GPL)
- License file included in %doc
- License in package matches license field in spec
- Permissions on files are set properly, %defattr used
- Package has a %clean section.
- Macro usage is consistent throughout the spec file
- No headers, static libs, or .so files for -devel
- No pkgconfig files for -devel
- No need for a -docs package
- No .la static libraries

With the changes in the attached spec, you can consider this package
approved.

~spot
-- 
Tom "spot" Callaway: Red Hat Sales Engineer || GPG Fingerprint: 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 --------------
Summary: A tool for executing PHP scripts with the permissions of their owners
Name: suphp
Version: 0.5.2
Release: 4
License: GPL
Group: System Environment/Daemons
Source0: http://www.suphp.org/download/suphp-%{version}.tar.gz
Source1: suphp.conf
URL: http://www.suphp.org/
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
Requires: httpd >= 2.0, php
BuildRequires: httpd-devel >= 2.0

%description
suPHP is an apache module for executing PHP scripts with the permissions of
their owners. It consists of an Apache module (mod_suphp) and a setuid root
binary (suphp) that is called by the Apache module to change the uid of the
process executing the PHP interpreter.

%prep
%setup -q -n suphp-%{version}

%build
%configure \
	--with-apache-user=apache \
	--with-min-uid=500 \
	--with-min-gid=500 \
	--with-php=/usr/bin/php \
	--with-logfile=/var/log/httpd/suphp_log \
	--with-setid-mode=owner \
	--disable-checkpath

pushd src
make %{_smp_mflags} suphp
popd

pushd src/apache2
%{_sbindir}/apxs -c mod_suphp.c
mv .libs/mod_suphp.so .
popd

%install
rm -rf $RPM_BUILD_ROOT
mkdir -p $RPM_BUILD_ROOT%{_libdir}/httpd/modules
mkdir -p $RPM_BUILD_ROOT%{_sbindir}

%{__install} -c -m 4755 src/suphp $RPM_BUILD_ROOT%{_sbindir}/suphp
%{__install} -m755 src/apache2/mod_suphp.so $RPM_BUILD_ROOT%{_libdir}/httpd/modules

# Install the config file
mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/httpd/conf.d
%{__install} -m 644 %{SOURCE1} \
   $RPM_BUILD_ROOT%{_sysconfdir}/httpd/conf.d/

%clean
rm -rf $RPM_BUILD_ROOT

%files
%defattr(-,root,root)
%doc README COPYING
%{_sbindir}/suphp
%{_libdir}/httpd/modules/*.so
%config(noreplace) %{_sysconfdir}/httpd/conf.d/*.conf

%changelog
* Thu Jun 30 2005 Tom "spot" Callaway <tcallawa at redhat.com> 0.5.2-4
- spec file cleanups for Fedora Extras

* Sat Nov 13 2004 Andreas Thienemann <andreas at bawue.net> 0.5.2-3
- Added "--disable-checkpath" in order to allow /~user URLs

* Sat Nov 13 2004 Andreas Thienemann <andreas at bawue.net> 0.5.2-2
- Fixed the wrong path in the logfile directive

* Sat Nov 13 2004 Andreas Thienemann <andreas at bawue.net> 0.5.2-1
- initial package