Request for review / approval: mod_security.

[Oliver Falk]

Michael Fleming wrote:
> On Fri, 08 Jul 2005 16:20:01 +0200. Oliver Falk waffled thusly:
[ ... ]
> > On 07/08/2005 03:56 PM, Michael Fleming wrote:
> > > Hi folks,
> > > 
> > > I've undoubtedly put this one up before a while back, but 
> I've made 
> > > a couple of small changes and added some useful rules in 
> the default 
> > > config.
> > > 
> > > Therefore I'm putting this up for further review and approval (I 
> > > don't recall seeing much last time - I felt such a package would 
> > > generate more interest :-)).
> 
> <snip>
> 
> > > SRPM:
> > > 
> http://www.enlartenment.com/extras/mod_security/mod_security-1.8.7-2
> > > .src.rpm
> > > 
> > > Please have a look and feel free to send feedback or improvements.
> > 
> > You don't need an approval if the package was allready 
> > approved and is 
> > in cvs... Spec looks OK for me and and it seems you havn't 
> > change very much...
> 
> I've also been running it locally for quite a while so it 
> does work as advertised :-D

Fine.

> > But OK, I'd like to mention a few things:
> >   - BuildRoot, as described in the PackagingGuidelines, pls.
> >     (http://fedoraproject.org/wiki/PackagingGuidelines).
> >     %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
> 
> D'oh! Fixed :-)

Good.

> >   - Why does the new package not depend on http(/-devel) >= 
> > 2.0.38, as the old package in CVS does?
> 
> I'm a little torn on this one - httpd-devel is indeed needed 
> (and is listed in the BR unless my brain has completely gone 
> to mush) but the explicit version I could probably drop.
> 
> There are some packages out there that are actually httpd 
> 1.3.x branch (for old-schoolers who just don't want to 
> upgrade to Apache 2 yet. Each to their own I guess) and the 
> version check would stop attempts to use it resulting in 
> their Apache 1.3 install blowing up on them.

But old apaches, are called apache (do provide webserver, but not 'httpd'
and not 'httpd-devel') and your package will not work this way... Believe
me, I'm running 1.3 apache installs. :-) So for compatibility reasons, it's
useless. Also FC/FE doesn't have apache 1.3.x packages...

> On the other hand if they're particular enough to downgrade 
> it they can pay attention to the version this package is 
> actually intended for.

However, if you like to drop the explicit version dependency, that's OK.

> >   - Don't forget the %{dist} tag in the release, if you 
> > intend to cvs commit this.
> 
> Fixed. (%{?dist})
> 
> > 
> >   - setup can be written as setup -q (the -n 
> > modsecurity-%{version} is 
> > not needed)
> 
> Unfortunately the upstream developer has recently been using 
> modsecurity (as opposed to mod_security) in the tarball and 
> directory structure, thus it's use (I'd prefer to maintain 
> the standard mod_<mumble> naming convention used by Apache 
> modules). It's not given me any trouble even if it looks a 
> bit out- of-place.

OK, if there was the need to do so, then it makes sense, let it as it is.

> > Best,
> >   Oliver
> 
> Updated SRPM
> http://www.enlartenment.com/extras/mod_security/mod_security-1
> .8.7-3.src.rpm
> 
> I do have a CVS branch in -devel for this, I'll ensure it's 
> synced so I don't confuse myself and others :-) (I'd 
> requested review before but never got formal build approval)

There's no need for a 'build approval', you only need a cvs import approval,
after the package is in CVS, you can - more or less - do what you want...
:-)

Please don't forget to request CVSSync if you intend to build the package
for other branches as well (others than the 'devel'):
http://fedoraproject.org/wiki/Extras/CVSSyncNeeded

And if you havn't done yet, also request a bugzilla entry:
http://fedoraproject.org/wiki/Extras/BugzillaAdmin

>From my point, you've got the approval to import the new spec and request
build! Even if this approval is not needed....

Best,
 Oliver