Bug 180068 - Review Request: GeoIP - Library for mapping IP/hostname to a country/city/organization
Review Request: GeoIP - Library for mapping IP/hostname to a country/city/org...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Thorsten Leemhuis (ignored mailbox)
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-02-05 06:01 EST by Michael Fleming
Modified: 2014-01-21 18:24 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-02-18 00:53:37 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-05 06:01:01 EST
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 03:53:43 EST
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 06:25:20 EST
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-14 19:33:31 EST
(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-17 21:11:15 EST
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-17 23:46:06 EST
APPROVED
Comment 6 Michael Fleming 2006-02-18 00:53:37 EST
Imported into CVS and a devel branch build kicked off.
Comment 7 Warren Togami 2006-02-18 02:40:35 EST
Interesting, would something like this be useful to aid in smarter auto-mirror
selection for yum?
Comment 8 Michael Fleming 2006-02-18 02:52:05 EST
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 03:02:19 EST
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 03:40:47 EST
(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 04:13:47 EST
(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 08:20:53 EST
I seem to remember spamassassin could use something like the perl GeoIP bindings
Comment 13 Kevin Fenzi 2006-12-21 22:18:46 EST
Changing the summary for tracking purposes. 

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