Page MenuHomePhabricator

Upgrade node-mapnik to 3.7.x
Closed, ResolvedPublic

Description

We're currently using node-mapnik 3.5.x compiled against system Mapnik. dpkg reports this is 3.0.12+ds-2~bpo8+2 and apt says it's coming from apt.wikimedia.org.

The latest version of node-mapnik is 3.7.0 and neither it nor 3.6.0 compile against Mapnik 3.0.12.

I'm asking Debian GIS about backports for the system Mapnik.

To upgrade, our options are

  1. Rely on binary blobs from S3. This is the default when you run npm install, and probably won't work with the glibc on Jessie.
  2. Build Mapnik ourselves, probably using Debian GIS packaging
  3. Have Debian GIS backport it

Event Timeline

Pnorman created this task.Mar 1 2018, 9:44 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 1 2018, 9:44 PM
Pnorman triaged this task as Normal priority.Jun 26 2018, 4:49 PM

This should become easier when we're on stretch

Mholloway moved this task from Backlog to To-do on the Maps-Sprint board.Jul 3 2018, 4:57 PM
Mholloway raised the priority of this task from Normal to High.Jul 14 2018, 12:15 AM
Mholloway added subscribers: Jhernandez, MSantos.

I think this is going to be necessary to complete T195316 and T172090 (and also helpful for T198828). In the case of T172090, I don't know in practical terms how we can force the node-mapnik 3.6.2 being pulled in via kartotherian@0.0.38 > tilelive-tmsource@0.7.0 > @mapbox/tilelive-bridge@2.5.1 > mapnik@3.6.2 version back down to 3.5.x. The way to do that using the npm version we're running in production would be to create and adjust an npm-shrinkwrap file, but npm shrinkwrap fails in this case due to unmet peer dependency errors (T195316). So we have something of a chicken-and-egg problem trying to resolve these without upgrading mapnik.

So, I think we should go ahead and upgrade it. I've got a fork of kartotherian with the dependencies adjusted to agree on 3.6.x at https://github.com/mdholloway/kartotherian/tree/deps, and it seems to work well on my local machine. Since according to the discussion in the description the upgrade should be easier on stretch, maybe the upgrade could happen concurrently with the stretch rollout.

On a side note, a couple of questions came up while fiddling with the dependencies:

  1. Why are we currently using a fork of tilelive-vector?
  2. What relationship does kartotherian/maki have to mapbox/maki, if any?

I'm also going to be bold and bump the priority given how this intersects with other high-priority issues we're working on.

Jhernandez added a parent task: Restricted Task.Jul 16 2018, 1:51 PM

I've added it to kanban so that you can pick it up when you are ready. Prio sounds good to me.

Mholloway moved this task from To-do to In progress on the Maps-Sprint board.

On IRC, @Pnorman suggested going higher than 3.6.x, to 3.7.x or 4.0.x, as long as we're going through the trouble of upgrading. That sounds good to me.

I've got a POC branch for 3.7.x at https://github.com/mdholloway/kartotherian/tree/deps-3.7 similar to the one above for 3.6.x. It seems to work well. We can't go straight to 4.0.x yet because our @mapbox dependencies haven't yet adopted it and are still on 3.7.x.

If going for 3.7.x is agreeable to everyone, I'll do the same for tilerator and do all the necessary bumping and publishing so we can get this pushed out to the new stretch maps beta tester (deployment-maps04). It'll also take a couple of patches in the deploy repos for scap, which I'll do shortly.

If going for 3.7.x is agreeable to everyone

@Mholloway I am okay with that.

Tilerator POC branch for mapnik 3.7.x: https://github.com/mdholloway/tilerator/tree/deps-3.7

I cloned your POC and tested, everything looks good to me. You can go ahead with the PR too.

Progress update:

The following repos have been updated, with version bumps as needed, but the updates are not yet published in npm:

  • kartotherian/core
  • kartotherian/maki
  • kartotherian/snapshot
  • kartotherian/tilelive-vector

The following will need updating/publishing when the dependency updates are published:

  • kartotherian/kartotherian
  • kartotherian/tilerator

To be continued tomorrow morning.

OK, so: I haven't been able to find the a way to successfully build mapnik from source when updating to node-mapnik 3.6 or 3.7, and on further reflection, I'm not sure this is actually quite the blocker to our other work that I thought it was:

It would still be nice to upgrade mapnik, but it's worth noting that node-mapnik 3.5.14 is the version packaged for Stretch in the official Debian repos. 3.7.2 is packaged for Buster, which suggests that we might have an easier time building 3.7 against the system mapnik when we upgrade to Buster in production, whenever that happens.

What version of mapnik are you trying to build node-mapnik 3.7.2 against?

I'm looking at the instructions here:

https://github.com/mapnik/node-mapnik#source-build

IIUC, without any intervention from us, the mapnik version this is trying to build against when working with a stock debian:stretch base image will be this one, 3.0.12:

https://packages.debian.org/source/stretch/mapnik

I noticed in the README that "The master branch of node-mapnik is not compatible with 3.0.x series of Mapnik. To build again Mapnik 3.0.x, use v3.0.x branch," so I've tried specifying appropriate commits from the v3.0.x branch of node-mapnik (for 3.6.3, or the head of that branch) in addition to the published 3.6.2 and 3.7.2. Nothing so far has worked.

Looking further now, I see from node-mapnik's package.json @3.7.2 that it wants mapnik_version v3.0.20 or greater, so I guess it is no surprise that that's not working. 3.6.2 wants mapnik 3.0.15. (And 3.6.1 wants 3.0.14, and 3.6.0 wants 3.0.13.)

The thing about mapnik 3.0.x compatibility is confusing to me because it seems to contradict mapnik_version in package.json ever since it was added in 3dba76a. Maybe I'm missing something.

In any case it looks like we will need to find a backport of a later version of mapnik to get this working.

And looking at the Docker output, 3.0.12+ds-3 is indeed the version being pulled down:

Get:328 http://cdn-fastly.deb.debian.org/debian stretch/main amd64 libmapnik3.0 amd64 3.0.12+ds-3 [2169 kB]
Get:356 http://cdn-fastly.deb.debian.org/debian stretch/main amd64 libmapnik-dev amd64 3.0.12+ds-3 [674 kB]
Get:365 http://cdn-fastly.deb.debian.org/debian stretch/main amd64 mapnik-doc all 3.0.12+ds-3 [2137 kB]
Get:366 http://cdn-fastly.deb.debian.org/debian stretch/main amd64 mapnik-utils amd64 3.0.12+ds-3 [228 kB]

So basically I've re-confirmed everything you (@Pnorman) wrote in the task description...

Did anything come of this?

I'm asking Debian GIS about backports for the system Mapnik.

The thing about mapnik 3.0.x compatibility is confusing to me because it seems to contradict mapnik_version in package.json ever since it was added in 3dba76a. Maybe I'm missing something.

Oh, maybe this is because mapnik_version has been referencing mapnik git commits rather than v3.0.x version references ever since this commit. Probably more of a node-mapnik dev concern.

Mholloway added a subscriber: Gehel.

Let's talk to @Gehel next week (if not sooner) about backporting mapnik 3.0.20 for Stretch.

Mholloway lowered the priority of this task from High to Normal.Jul 23 2018, 7:21 PM

Desirable but not blocking other work at this time.

Mholloway renamed this task from Upgrade Mapnik to Upgrade Mapnik to 3.7.x.Jul 24 2018, 6:25 PM
Mholloway renamed this task from Upgrade Mapnik to 3.7.x to Upgrade node-mapnik to 3.7.x.

Kartotherian and its dependencies are all upgraded and they were tricky to test because we can't have more than one version of node mapnik running in the same ecosystem.

I am also bumping the version of the libraries to v1.0.0 because of an upcoming task (T187478)

WIP: tilerator

MSantos moved this task from Needs review to Done on the Maps-Sprint board.Aug 14 2018, 4:58 PM
Jhernandez closed this task as Resolved.Aug 17 2018, 1:29 PM

@MSantos Can you have a look into the parent security tasks that are in TODO in the kanban board and verify if we need to do anything else for them? Thanks!