Bug 185535 - Review Request: lurker
Summary: Review Request: lurker
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2006-03-15 16:56 UTC by John Dennis
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-02-22 19:23:21 UTC
Type: ---
Embargoed:
j: fedora-review-


Attachments (Terms of Use)

Description John Dennis 2006-03-15 16:56:57 UTC
Spec: http://people.redhat.com/jdennis/lurker.spec
SRPM: http://people.redhat.com/jdennis/lurker-1.3-4.src.rpm

Description:

I have packaged the Lurker (http://lurker.sourceforge.net) mail archiver.

Lurker is a mailing list archiver with a web interface. It features
fast complex searches, chronological threading, message threading
navigation, mime handling, file attachment support, multi language
suport, web server caching for efficiency, and it has customizable
output via it's use of XML/XSLT.

Lurker is currently the mail archiver used on Debian as well as a number of other sites, to date there has been no RPM for it, this corrects that.

I did have modify the current configure.ac so that I could produce an installation which conformed to the preferred Red Hat directory layout, that patch will be submitted upstream, very briefly it adds the following arguments to configure (their values default to match upstream):

--with-wwwdir         [web server installation root]
--with-cgidir=DIR     [where cgi scripts are installed]
--with-dbdir=DIR      [location of database files)]

The RPM also adds an httpd conf file which is absent in upstream where it is incumbant on person to edit the httpd.conf and from traffic on the mailing list seems to be the source of much user confusion, hopefully this will eliminate what has seemed to be a tedious installation step in the past.

Comment 1 Jason Tibbitts 2006-04-13 15:38:11 UTC
Some quick comments:

Please use a complete URL in Source0 so that spectool can automatically fetch
the source.  If Sourceforge wasn't throwing "Internal server error" at the
moment I'd give more detail (and I could test out a build of the package).

gcc-c++ is not permitted in BuildRequires:.

I imagine that RPM will figure out what you have in Requires:, but I can't
verify that at the moment.

It's much cleaner to refer to %{SOURCE1} rather than
$RPM_SOURCE_DIR/lurker-httpd.conf.in.  There's rarely a good reason to ever use
$RPM_SOURCE_DIR.

Is %{buildsubdir} guaranteed to be defined?

I think that world-writable directory is going to be a blocker.  If the end user
wants to open the permissions up, that would be their business.  I think modern
MTAs can be configured to deliver as the appropriate user so this shouldn't be a
big deal in practise.

You define %{mta_owner} and %{mta_group} but don't reference them anywhere.

I find that defining a macro for something that's referenced only once in the
spec (like, say, %{httpdconffile}) is a bit confusing, but that's just personal
taste.

Comment 2 Jason Tibbitts 2006-04-23 05:14:08 UTC
BTW, recent guideline changes permit gcc-c++ in BuildRequires:, even though it's
probably cleaner to leave it out.

Comment 3 Jason Tibbitts 2006-04-27 15:31:11 UTC
Also, the current version seems to be 2.1; there are what look to be serious
security implications for any version older than 2.1.  (Sorry for spamming, but
I was starting on a review when I noticed.)

Comment 4 John Dennis 2006-04-27 17:07:30 UTC
Thank you for your careful review Jason and your thoughtful comments. I'm sorry
I haven't responded sooner but I've got higher priority tasks than lurker
packaging which have captured my attention.

FYI, it was my plan to address the issue of a world writable database files by
adding the owner and group to the configure script and passing it configure via
%{mta_owner} and %{mta_group}. I do realized it is not strictly required to have
this be available to configure because the RPM %files section can enforce the
permissions but it is my belief that making this explicit is a win in terms of
readability, upstream use, and those who may use configure outside the context
of rpm. This is why these two variables appeared in the spec file, once
configure could accept them they would be passed and set in the %files section.

As for modern MTA configuration with respect to mail delivery I can attest as
the previous maintainer for postfix, dovecot and mailman this is an area of
endless user confusion :-( (Did you know postfix will deliver under the user id
associated with the alias file the alias was found in?) Now throw into the mix
per user procmail :-( It can be a mess and the source of lots of bug reports and
list questions. I believe the best solution is to be absolutely explicit over
the expected owner and group and thus will allevate 1 axis of confusion. As
indicated above I will modify the spec file to make this absolutely explicit and
locked down.

With respect to requiring a C++ compiler, the code is C++ so that seemed
necessary. It sounds like you may be more familar with this particular issue
than I am so I will defer to you on this topic.

You are correct, %{buildsubdir} is not guarenteed to exist, I'll fix that.

I'm not sure I share your opinion that RPM_SOURCE_DIR should never be used. I
appreciate that SOURCEn will always properly refer to it but I find use of
SOURCEn to reduce readability and comprehenion of spec files. Whenever I
encouter this use when reading a spec file I have to mentally expand the macro
to understand what the spec file is doing, I find it much more comprensible if
the operation is explicit. Hmm... I suppose the same argument could be used
against %{httpdconffile} :-)

At the time I packaged up lurker only the 1.3 version was available. Looks like
the 2.0 release occurred shortly after I completed my work but before I got my
review request in. I agree, 2.1 should be version in extras. I'll go back and do
that, but it won't be immediate, I've got other tasks on my plate. I'm also
going to have to figure out the differences between 1.3 and 2.x.



Comment 5 Mamoru TASAKA 2006-11-27 07:39:24 UTC
Well, what is the current status of this review request?

Comment 7 Jason Tibbitts 2007-02-22 19:23:21 UTC
No response in ages; I'm closing this.


Note You need to log in before you can comment on or make changes to this bug.