Bug 181974 - Review Request: mod_geoip - Apache module for GeoIP lookups
Review Request: mod_geoip - Apache module for GeoIP lookups
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-02-18 01:18 EST by Michael Fleming
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-02-20 17:39:57 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Michael Fleming 2006-02-18 01:18:07 EST
Spec Name or Url: http://www.enlartenment.com/extras/mod_geoip.spec
SRPM Name or Url: http://www.enlartenment.com/extras/mod_geoip-1.1.7-2.src.rpm
Description: 
mod_geoip is an Apache module for finding the country that a web request
originated from.  It uses the GeoIP library and database to perform
the lookup.  It is free software, licensed under the Apache license.
Comment 1 Jason Tibbitts 2006-02-19 22:41:08 EST
Looks clean and builds properly in mock (i386, development).
rpmlint is silent.
The package meets the naming and packaging guidelines.
License is appropriate and matches source.
Source file matches upstream.
The specfile is properly named and is legible and understandable.
BuildRequires: are proper.

I find the Requires: a bit odd.  What's the point of having httpd-mmn = missing
/usr/include/httpd/.mmn doesn't exist?  In any case, not a blocker.

Approved.
Comment 2 Ralf Corsepius 2006-02-19 23:25:32 EST
The tarball seems to be called "mod_geoip2".

I know too little about this package, but why isn't the rpm called the same rsp.
why doesn't the rpm provide a virtual property of this name?
Comment 3 Michael Fleming 2006-02-20 03:53:31 EST
I'll answer both questions in one reply -  Both are quite valid questions.

Jason: The httpd-mmn Requires: is to ensure that the module version matches that
of the Apache version you're going to install against. (Something one of the
more experienced packagers (Ville?) pointed out while I was working on mod_security)

Ralf: I called it mod_geoip to avoid potential confusion ("Where's/What is
mod_geoip1?) and ensure some level of clarity.

Upstream has different tarballs for Apache 1.3.x and 2.x, the latter is
mod_geoip2 but, given that FC doesn't package the older branch anyway, I went
for plain "mod_geoip" so even the most inebriated of users could guess what it
was for :-D.

As for a virtual property, I've never seen anyone package it elsewhere  - and if
they've called it mod_geoip they'll get cleanly upgraded anyway provided other
deps are met.

I'll import and build unless anyone has strong "blocker" objections to the name.
Comment 4 Jason Tibbitts 2006-02-20 09:34:00 EST
I implicitly understood your reasoning for using mod_geoip and not mod_geoip2
and didn't see it as a problem.  The name of the generated module is
mod_geoip.so and the  upstream name of the package is mod_geoip:
http://www.maxmind.com/app/mod_geoip 

FYI, Mandriva seems to ship this package as mod_geoip as well:
http://rpmfind.net/linux/RPM/mandriva/devel/cooker/cooker/media/contrib/Mandriva.html

About the Requires:, I understand why it's there but I didn't quite get what use
a Requires: of "httpd-mmn = missing" (in the case that /usr/include/httpd/.mmn
doesn't exist) is going to be.  I suppose it's just defensive programming, which
is no problem with me.
Comment 5 Ralf Corsepius 2006-02-20 09:50:45 EST
(In reply to comment #4)
> I implicitly understood your reasoning for using mod_geoip and not mod_geoip2
> and didn't see it as a problem.  The name of the generated module is
> mod_geoip.so and the  upstream name of the package is mod_geoip:
> http://www.maxmind.com/app/mod_geoip 
I asked because of the mod_perl vs. mod_perl2 idocy.
Comment 6 Ville Skyttä 2006-02-20 12:11:57 EST
"httpd-mmn = missing" comes from some Core packages, so you'll have to ask there
to get a definite answer.  But at least it allows the specfile syntax to be ok
even when httpd-devel is not installed, enabling for example rpm -q --specfile
queries in those scenarios.  If the "|| echo missing" part would not be there,
it wouldn't work.
Comment 7 Michael Fleming 2006-02-20 17:39:57 EST
Imported source RPM into CVS, will request a build when I get home from work.

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