Page MenuHomePhabricator

"geojson" plugin is missing in Mapnik package
Closed, ResolvedPublic

Description

Could not create datasource for type: 'geojson' (searched for datasource plugins in '/usr/lib/mapnik/3.0/input')

is shown when using snapshot, e.g.
http://localhost:16001/img/osm-intl,1,0,0,1000x1000.png?domain=www.mediawiki.org&title=Help:Extension:Kartographer&groups=_548ad9e792478e08584ac04c187e4de43a8028cc

I suspect this is due to Mapnik packaging not including geojson plugin.

We only have the following plugins in there:

gdal.input  ogr.input  pgraster.input  postgis.input  raster.input  shape.input  sqlite.input

Event Timeline

Yurik created this task.Dec 1 2016, 8:20 PM
Restricted Application added a project: Discovery. · View Herald TranscriptDec 1 2016, 8:20 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

The debian libmapnik3.0 package includes the geojson plugin (/usr/lib/mapnik/3.0/input/geojson.input). Can we not use the debian packaging instead of writing our own?

Yurik updated the task description. (Show Details)Dec 1 2016, 9:07 PM
Yurik updated the task description. (Show Details)Dec 1 2016, 9:27 PM

So, the packaging appears to be reasonable, the next step is to see what the build log says and if it's trying to build geojson and failing, or not trying at all.

We are not "writing our own" packaging; I merely reused Debian's packaging and built on a clean jessie system (the standard practice of backporting). The problem, however, seems to be this from plugins/input/geojson/build.py:

can_build = False

if env.get('BOOST_LIB_VERSION_FROM_HEADER'):
    boost_version_from_header = int(env['BOOST_LIB_VERSION_FROM_HEADER'].split('_')[1])
    if boost_version_from_header >= 56:
        can_build = True

if not can_build:
    print 'WARNING: skipping building the optional geojson datasource plugin which requires boost >= 1.56'
else:
[…]

jessie ships with Boost 1.55; backporting Boost is a herculean effort that I neither have the time to do, nor I think anyone should do.

I''m happy to continue helping on this but this is proving a bit too much of an effort and I can't continue down the rabbit hole. I'm also not familiar with neither the Kartotherian requirementents (and whether Mapnik and/or GeoJSON are hard requirements) nor the Mapnik stack itself (to e.g. fix those Boost incompatibilities).

My suggestion at this point would be for Discovery to fully own this -- either figure out a way for the stack of their choosing to run on a non-bleeding edge system (but on a reasonably new distribution, such as jessie (either by modifications to the stack or the heavy load of backporting) or partially or fully replace their stack. Always happy to continue helping out in a limited capacity and be your partner in this :)

Gehel added a subscriber: Gehel.Dec 6 2016, 1:52 PM

Thanks @faidon ! I just came to the same conclusion (except that it of course took me a bit more time). I should have checked your answer first.

As I understand it, mapnik is a pretty hard requirement, and upgrading it seems necessary to be nodejs 6 compatible. We'll do some brainstorming and see what kind of solution we can come up with...

1.56.0 is where boost::geometry came in.

Replacing Mapnik would mean completely replacing the stack, and there are no alternatives to Mapnik which have its capabilities yet.

Why we've been able to use Geojson so far is we've been relying on the binary blob supplied by Mapbox which has Mapnik in it. I'm not sure if it's statically linked to a more recent version of Boost or if all the Boost libraries are used are header only, but it doesn't depend on having Boost on the system. Of course, it's also impossible to reproduce and can break for unfixable reasons.

I don't see any great options, but the choices here seem to be a) drop GeoJSON support which means dropping features, or b) Drop nodejs 6 support and continue to use the binary blob

Mapnik uses Boost libraries which are not header only, so my thoughts at including just the more recent boost headers to the Mapnik source doesn't work (i.e it's a depends, not just a dev depends).

I'm also not familiar with neither the Kartotherian requirementents (and whether Mapnik and/or GeoJSON are hard requirements)

They are. Mapnik is a core component of the stack, and geojson support is required for the geojson overlay, critical to the snapshot service.

nor the Mapnik stack itself (to e.g. fix those Boost incompatibilities).

The problem is boost::geometry just doesn't exist for the jessie version.

I looked at a few options

  1. Backporting boost. The problem here isn't boost, it's that boost has >200 direct rdepends. This is not practical.
  2. Including the boost::geometry headers in the Mapnik source tree and disabling the version check. Although Mapnik depends on compiled boost libraries, boost::geometry is header only. This used to be done at one point because Mapnik was relying on pre-release Boost headers, so it's probably possible.
  3. Removing use of boost::geometry from the geojson plugin. This is probably not practical.
  4. Forward porting an old version of the geojson plugin to Mapnik 3.0.12. I have no idea how the API has changed, so no idea on how much work this is. This would be from some version between v2.2.0 and v3.0.0, a ~two year period without any releases.
  5. Doing something with static linking and more recent versions. This is essentially what we're doing right now, where Mapbox's binary blob for Mapnik includes most of the dependencies needed. RUNTIME_LINK and PLUGIN_LINKING vars may be related to this. This would have a fair amount of ongoing work, so I'd like to avoid it.
  6. Using the ogr plugin. Although we need geojson support, it doesn't follow we need the geojson plugin. If the ogr plugin is working on the backport, this would be easiest.
  7. Not upgrading to node 6 and continuing to use the binary blob. This defers the problem, but at some point we'll end up needing to build our own Mapnik to avoid the binary blob, because something will break eventually.

Obviously we're going to do 7 in the short term. My inclination is for 6, using the ogr plugin. @faidon, re there any error messages in the build log related to ogr or gdal? If we run into problems with ogr or find we need features of the geojson plugin (e.g. rtrees), I'd be inclined to try including the headers with 2.

Gehel added a comment.Dec 7 2016, 9:15 AM

@Pnorman it looks like ogr plugin is built without issue. The build log (reproduced from @faidon's work) is P4582.

Documenting from discussion and thoughts, it sounds like the best option is to switch from the geojson plugin to the ogr plugin. This will be slower, particularly for very large geojson files, but it shouldn't matter for most stuff.

Yurik removed a project: Maps.Dec 15 2016, 4:37 AM
Deskana moved this task from Backlog to To-do on the Maps-Sprint board.Jan 25 2017, 7:36 PM
MaxSem claimed this task.Feb 8 2017, 7:03 PM
MaxSem moved this task from To-do to In progress on the Maps-Sprint board.

1.56.0 is where boost::geometry came in.

1.47, actually.

Okay, so here is a simple solution: that plugin actually builds with Boost 1.55 if the build script is tweaked to allow it. 1.56 is required since this commit, however it just cleans up old b/c code without introducing anything new into the plugin. The usage of Boost.Geometry is very limited (rtree only). I therefore propose to build the package with the below patch and see if it works. If it doesn't, I'll prepare an undo of that commit in geojson only.

diff --git a/plugins/input/geojson/build.py b/plugins/input/geojson/build.py
index a5d738f..7f6f67f 100644
--- a/plugins/input/geojson/build.py
+++ b/plugins/input/geojson/build.py
@@ -25,11 +25,11 @@ can_build = False

 if env.get('BOOST_LIB_VERSION_FROM_HEADER'):
     boost_version_from_header = int(env['BOOST_LIB_VERSION_FROM_HEADER'].split('_')[1])
-    if boost_version_from_header >= 56:
+    if boost_version_from_header >= 55:
         can_build = True

 if not can_build:
-    print 'WARNING: skipping building the optional geojson datasource plugin which requires boost >= 1.56'
+    print 'WARNING: skipping building the optional geojson datasource plugin which requires boost >= 1.55'
 else:

     Import ('plugin_base')

For the reference, the list of changes between geometry 1.55 and 1.56 can be viewed with git clone https://github.com/boostorg/geometry.git && cd geometry && git log --grep '\[geometry\]' boost-1.55.0..boost-1.56.0

In the meantime I've made some tweaks to the docker build script and to service-runner and hopefully with @MaxSem patch and this gerrit https://gerrit.wikimedia.org/r/#/c/337200/ we will get a working build.

I therefore propose to build the package with the below patch and see if it works.

We can try it, but I don't believe it'll work. The deps/boost/geometry/... stuff in Mapnik that was removed in that commit were pre-release files which were added in Boost 1.56.

If it doesn't, I'll prepare an undo of that commit in geojson only.

Rather than a partial revert, I'd first try adding rtree.hpp, rtree_leaf.hpp, helpers.hpp, and rtree_node.hpp from a more recent version of boost.

We can try it, but I don't believe it'll work. The deps/boost/geometry/... stuff in Mapnik that was removed in that commit were pre-release files which were added in Boost 1.56.

I'm saying that it actually builds for me ;) 3.0.12.

ksmith reassigned this task from MaxSem to Gehel.Feb 13 2017, 7:34 PM
ksmith added a subscriber: ksmith.

We believe this is now up to ops.

MaxSem moved this task from In progress to Needs review on the Maps-Sprint board.Feb 14 2017, 11:12 PM
Gehel reassigned this task from Gehel to MaxSem.Feb 16 2017, 3:11 PM

@MaxSem the new libmapnik package 3.0_3.0.12+ds-2~bpo8+2 (Debian naming is weirder than whatever Oracle could come up with) is now deployed (manually) on maps-test* servers. I'll let you do some validation before uploading this to our apt repo and deploying on production servers.

MaxSem closed this task as Resolved.Feb 23 2017, 10:30 PM

My testing indicates that this package works. Let's continue on the main bug.

Deskana moved this task from Needs review to Done on the Maps-Sprint board.Apr 19 2017, 4:26 PM