I'd like to add support for using a separate copy of geoPHP outside of this module. Assuming that the main development of geoPHP is still going to happen on Github, I think we should allow site builders to pick an alternate copy of the code base. Even if @phayes decides to move his main dev to Drupal.org, there's probably still a few forks floating around the ether that people might still want to use.

This is on my todo list for the plane ride to Denver tomorrow, so expect a patch soon.

CommentFileSizeAuthor
#2 1488902-library-support.patch5.11 KBBrandonian

Comments

phayes’s picture

phayes commented

Ah that makes sense. So basically *If* the library version exists we use that, if not then we use the bundled version.

Brandonian’s picture

Brandonian commented
StatusFileSize
new 1488902-library-support.patch5.11 KB

Attached is my initial patch at addressing the issue. I've changed geophp_load to return the path of the library regardless of where it comes from, and added logic to handle 3 potential cases.

  • GeoPHP already loaded, returns path of library.
  • Library module installed and geoPHP found in outside folder, return path of remote library
  • None of the above, attempt to load bundled copy of geoPHP.

I've also revamped the requirements page to show the version of the loaded library, and to throw a warning if it's older than the bundled version. Also added a case if the library isn't functional at all. Seems unlikely to ever trigger (Bundled library would have to be corrupted along with no remote library), but added the case for completeness.

Also, the patch makes it look like I completely rewrote the module. This isn't the case: the line endings in the module are Windows instead of Unix. I can reroll without those changes if it makes it easier to review the patch.

phayes’s picture

phayes commented

I'm OK with you completely rewriting the module. :-)

phayes’s picture

phayes commented

Commited! I've also added you as a maintainer (no expectations, just makes things easier)

phayes’s picture

phayes commented
Status: Active » Fixed
kevinquillen’s picture

kevinquillen commented

Yes, there are other modules that require just the library, and some that require the module (which comes bundled with geoPHP) causing duplicate class declaration errors.

It's probably best to unbundle it from the module, and let it be downloaded to the libraries folder, yes?

kevinquillen’s picture

kevinquillen commented
Status: Fixed » Active

Also, this should be rolled into a new release, because it is not in the 1.1 release. Then, it can be marked fixed.

Brandonian or phayes, can you create a 1.2 release for geophp containing your patch? You might also want to tick off the box that shows the dev snapshot, because even though this may be pushed to the dev, there does not appear a way to pull it.

Brandonian’s picture

Brandonian commented

I have some stability issues that I want to take care of before pushing 1.2. To the point of duplicate libraries, the new version should take care of this.

kevinquillen’s picture

kevinquillen commented

Okay. Well til then. I went in a circle until I saw the patch date and release date. :D

Brandonian’s picture

Brandonian commented

Fixed. Also, we have a 1.3 release. (1.3 instead of 1.2 b/c I'm not a fantastic debugger...)

Brandonian’s picture

Brandonian commented
Status: Active » Fixed

Probably would help to actually mark as fixed...

kevinquillen’s picture

kevinquillen commented

Excellent

kevinquillen’s picture

kevinquillen commented
Status: Fixed » Active

I still get this error. It happens very randomly, and I have not been able to replicate it successfully but I have seen it at least 3 times today.

phayes’s picture

phayes commented

what error are you getting precisly?

kevinquillen’s picture

kevinquillen commented

"Cannot redeclare class 'GeoAdapter'"

phayes’s picture

phayes commented

Hi @kevinquillen,

Can you please tell me what version of geoPHP, geofield, geocoder, and/or views_geojson you have installed?

phayes’s picture

phayes commented
Status: Active » Postponed (maintainer needs more info)
phayes’s picture

phayes commented
Status: Postponed (maintainer needs more info) » Closed (fixed)

Haven't gotten any feedback. Closing...

Please open a new bug report if you are still having this problem.