Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
For instance:
Fatal error: Cannot redeclare geoip_country_code_by_name() in [...]/sites/all/modules/geoip/lib/geoip.inc on line 376
This happens when the geoip PECL extension is enabled on the server.
GeoIP API currently only supports the pure PHP lib that comes with the module and not the geoip PECL extension.
This is not stated on the project page nor in the INSTALL.txt.
Comments
Comment #1
drewish CreditAttribution: drewish commentedCool, I had no idea there was a pecl extension. Ideally we'd make them compatible but they seem to have different enough interfaces that it might be tricky. We should document the incompatibility and probably do a warning in hook_requirements.
Comment #2
anrikun CreditAttribution: anrikun commentedBy the way, I think that the pure PHP API (http://www.maxmind.com/download/geoip/api/php/) should not ship with the module (it is a 3rd party library) but be downloaded separately by user into
sites/all/libraries
.This way, user could choose between using the pure PHP API or the PECL extension.
Comment #3
drewish CreditAttribution: drewish commentedIncluding their library doesn't preclude supporting the pecl library. It's an include that could be conditional. But I think you must have missed the part about them being totally different APIs. I'm open to patches that would add support but that would really be a feature request. The real bug here is the undocumented incompatibility.
Comment #4
anrikun CreditAttribution: anrikun commentedSorry, I know this is a different issue. I was just too lazy to open a separate one :-)
Comment #5
NewZeal CreditAttribution: NewZeal commentedThe ad_geoip module (http://drupal.org/project/ad_geoip) uses the PECL extension.
Comment #6
roball CreditAttribution: roball commentedWhen you use PHP's geoip extension, there is no need to use this Drupal module. Thus I have removed the module from our server. Completely breaking all sites that have this module enabled is quite critical.
Comment #7
ddanier CreditAttribution: ddanier commentedI created a small patch to allow usage of geoip even if the geoip-extension is loaded. Note this is only a workaround to get this running. The geoip-extension should be used by geoip if possible and the PHP-implementation should only be a fallback.
Even with the geoip-extension I see the need for this module, as it allows to integrate geoip better into Drupal. Not sure how far the current implementation goas here, but I like having a module to define the API Drupal uses...
Comment #8
d.clarke CreditAttribution: d.clarke commentedIn case others are looking for anrikun's request to remove the library from the code, please see #1159090: Lib files should be removed from the module
With regards to adding compatibility with the geoip php extension, I've attached a patch that should accomplish this functionality. in the attached patch, if the geoip extension is installed, then it'll use that extension, otherwise it'll load the MaxMind geoip-api-php library and use it's calls.
This was written for the D7 version as patched in issue #887268: Port GeoIP API to D7 and then backported so there are three patch files included:
Comment #9
d.clarke CreditAttribution: d.clarke commentedMarking the ticket as "Needs review"
Comment #10
bojanz CreditAttribution: bojanz at Centarro commented