Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 180068 - Review Request: GeoIP - Library for mapping IP/hostname to a country/city/organization
Summary: Review Request: GeoIP - Library for mapping IP/hostname to a country/city/org...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Thorsten Leemhuis (ignored mailbox)
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-02-05 11:01 UTC by Michael Fleming
Modified: 2014-01-21 23:24 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-02-18 05:53:37 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Michael Fleming 2006-02-05 11:01:01 UTC
Spec Name or Url: http://www.enlartenment.com/extras/geoip.spec
SRPM Name or Url: http://www.enlartenment.com/extras/geoip-1.3.14-1.src.rpm
Description: GeoIP is a C library that enables the user to find the country that any IP
address or hostname originates from. It uses a file based database that is
accurate as of March 2003. This database simply contains IP blocks as keys, and
countries as values. This database should be more complete and accurate than
using reverse DNS lookups.

Comment 1 Ralf Corsepius 2006-02-10 08:53:43 UTC
NEEDSWORK

Blockers:
- Packaging of shared libs is messed up. 
What you call "superfluous symlinks" is essential.

Non-Blockers:
- The tarball is named GeoIP, therefore the rpm also should use this name.

- Shipping static libs. You should ship shared libraries, instead (--disable-static)

- You should append --disable-dependency-tracking to %configure to speed up
building.



Comment 2 Michael Fleming 2006-02-10 11:25:20 UTC
New spec and URL implementing the above suggestions:

http://www.enlartenment.com/extras/GeoIP.spec
http://www.enlartenment.com/extras/GeoIP-1.3.14-2.src.rpm

Built OK for me in mock (Core 4), rpmlint seems mostly happy (some warnings re:
the .so symlinks, but that's about all.)

Comment 3 Ralf Corsepius 2006-02-15 00:33:31 UTC
(In reply to comment #2)
> (some warnings re:
> the .so symlinks, but that's about all.)

This is a blocker, please fix (lib*.so must go to *-devel, lib*.so.* to non-devel)


Also arguable:
Obsoletes: geoip
Provides: geoip

This should probably be:
Obsoletes: geoip < %{version}-%{release}
Provides: geoip = %{version}-%{release}

Also missing in the *-devel package:
Provides: geoip-devel = %{version}-%{release}

Alternatively, you could consider to drop supporting "geoip".


Comment 4 Michael Fleming 2006-02-18 02:11:15 UTC
Another round o' SRPMS :-)

http://www.enlartenment.com/extras/GeoIP.spec
http://www.enlartenment.com/extras/GeoIP-1.3.14-3.src.rpm

(In reply to comment #3)
> (In reply to comment #2)
> > (some warnings re:
> > the .so symlinks, but that's about all.)
> 
> This is a blocker, please fix (lib*.so must go to *-devel, lib*.so.* to non-devel)

Done.

> 
> 
> Also arguable:
> Obsoletes: geoip
> Provides: geoip
> 
> This should probably be:
> Obsoletes: geoip < %{version}-%{release}
> Provides: geoip = %{version}-%{release}

Done, with a similar pairing for -devel to ensure it's handled reasonably cleanly.

> Also missing in the *-devel package:
> Provides: geoip-devel = %{version}-%{release}
> 
> Alternatively, you could consider to drop supporting "geoip".

Doable, but given that both I and Rudolf Kastl have shipped packages as "geoip"
in the past I felt it best to go with the above suggestions and give the user a
clean upgrade path (working on the principle of least surprise)

Michael.

Comment 5 Ralf Corsepius 2006-02-18 04:46:06 UTC
APPROVED

Comment 6 Michael Fleming 2006-02-18 05:53:37 UTC
Imported into CVS and a devel branch build kicked off.

Comment 7 Warren Togami 2006-02-18 07:40:35 UTC
Interesting, would something like this be useful to aid in smarter auto-mirror
selection for yum?


Comment 8 Michael Fleming 2006-02-18 07:52:05 UTC
Probably, I know the PHP devs use it for mirror selection so it's definitely
possible.

Would you like me to package up the Python bindings too? They're fairly small
and would seem very appropriate.

(I've got the Apache module submitted and have done the Perl bindings before,
but that latter spec needs some *serious* love before I let reviewers rip into it.)

Comment 9 Warren Togami 2006-02-18 08:02:19 UTC
Python bindings would be very appropriate and useful.  Apache module sounds
useful too, but it should be a separate package.  Perl is not too useful for
Fedora specifically, but some people might like it.

Comment 10 Ralf Corsepius 2006-02-18 08:40:47 UTC
(In reply to comment #9)
> Python bindings would be very appropriate and useful.
If these are a packaged as a separate add-on package, yes.
Please file separate review request.

>  Apache module sounds
> useful too, but it should be a separate package.  Perl is not too useful for
> Fedora specifically, but some people might like it.
I love this kind of biased missionary statements. Perl might not be much of RH's
interest (Yours), but it definitely is in Fedora Project's interest (e.g. mine).



Comment 11 Michael Fleming 2006-02-18 09:13:47 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > Python bindings would be very appropriate and useful.
> If these are a packaged as a separate add-on package, yes.
> Please file separate review request.

Yes, I'll be making seperate requests once I'm satisfied they're sane and fit
for Extras consumption.

For GeoIP-Python, I'm running a local build of a quick spec built off the python
template. It's not a huge package so should not take long to build (and
hopefully review)

> >  Apache module sounds
> > useful too, but it should be a separate package.  Perl is not too useful for
> > Fedora specifically, but some people might like it.
> I love this kind of biased missionary statements. Perl might not be much of RH's
> interest (Yours), but it definitely is in Fedora Project's interest (e.g. mine).

I get the impression Warren was referring to it being of lesser interest to
Fedora in the sense of the core package engineers / tools developers et. al - it
would see less usage as a significant amount of "innards and internals" code
(yum, anaconda etc.) is in Python rather than Perl. I'm sure it's useful to at
least some Fedora / Extras users :-P

My primary reason for mentioning it (GeoIP perl bindings) was as a nice
complement to Extras' AWStats package - the geoip plugin uses it for more
accurate country lookups, which I've got working quite nicely over here with
minimal effort.



Comment 12 Nicolas Mailhot 2006-02-18 13:20:53 UTC
I seem to remember spamassassin could use something like the perl GeoIP bindings

Comment 13 Kevin Fenzi 2006-12-22 03:18:46 UTC
Changing the summary for tracking purposes. 


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