Update import Docker container to use OSMF PPA packages

[skylerbunny]

Fixes # (id of the issue to be closed)
N/A
Changes proposed in this pull request:
Adds build arguments to the 'import' Docker container so that a custom osm2pgsql library can be built if the user so chooses, and explains the arguments to do this in the DOCKER.md file.

(I have run into a potential bug in osm2pgsql, and being able to specify an arbitrary version or branch for the version I wish to test against, to see how the osm2pgsql program imports it to display in openstreetmap-carto, is very useful in this scenario.)

Test rendering with links to the example places:
N/A

[pnorman]

My view is if you're doing this kind of advanced stuff, use a standard setup, not docker. This adds complexity that needs to be maintained. This also doesn't have the ability to build against osm2pgsql changes to test, only to build against a released commitish.

Looking at the code, it will fail to build many versions of osm2pgsql that we support because the build commands have changed.

[pnorman]

Don't pull because you've already got the latest, and don't checkout, specify a commitish on the clone

[skylerbunny]

Changed as suggested, done in one step.

[pnorman]

Build with release flags.

[skylerbunny]

Done.

[pnorman]

Not removing lists drastically inflates the image size here

[skylerbunny]

Re-enabled removing lists here.

[skylerbunny]

I've made the requested changes, but I see your overall point. If this doesn't really add to the utility of the docker setup, feel free not to approve it.

(It could of course be further abstracted by allowing a username to be passed to the git clone as another build argument, thus any fork and any commit could be tested against - that's not difficult to do - but the base issue is whether this is something you want in here.)

[skylerbunny]

As an additional comment, one thing I did notice here is that the Ubuntu xenial pull (as I recall) gets osm2pgsql 0.88.0 (2015). It's one reason I came up with this. I notice that stretch-backports in Debian seems to keep more or less up to date, but that's not what the import container uses.

[pnorman]

As an additional comment, one thing I did notice here is that the Ubuntu xenial pull (as I recall) gets osm2pgsql 0.88.0 (2015). It's one reason I came up with this. I notice that stretch-backports in Debian seems to keep more or less up to date, but that's not what the import container uses.

I'd be in favour of pulling in a more recent backport, as well as switching from Ubuntu to Debian.

[kocio-pl]

@skylerbunny Are you ready to take care of that?

I'm reluctant to switch distributions. Ubuntu seems to be more popular, hence easier to test and do maintenance when needed. In bionic we have 0.94.0, so I think updating to newest Ubuntu LTS would be the best way.

[skylerbunny]

One case for not moving to Debian would be that OSM-operations uses Ubuntu ( see openstreetmap/operations#220 ).

I've already been talking to them, and in practice (if I recall correctly) what they intend to do is to upgrade their four render servers to 18.04 in the next couple weeks, then use the stretch-backports version of https://packages.debian.org/source/stretch-backports/osm2pgsql , modified in any way necessary to do that, possibly just pulling the .deb file in directly if the existing system prereqs are sufficient to do so.*

* That port may require a build from source to work with Ubuntu, but if so, that could be orchestrated in the Dockerfile (build it, install the package, delete the build directories).

Did you want me to look into what it would take to do a straight-up container build with 18.04 and osm2pgsql (0.96.0+ds-1~bpo9+1)?

[kocio-pl]

Good point.

I'm not sure if we want to play with backports - we can probably just use OSMF PPA once it's there. For now I've just created simple change which works (explicit npm installation was needed), but Kosmtik plugins are not available, so you should tune it anyway - see master...kocio-pl:bionic.

[kocio-pl]

BTW - osm2pgsql packages for bionic are there already, so you can use this PPA now:

https://launchpad.net/~osmadmins/+archive/ubuntu/ppa

[skylerbunny]

Oh good! Okay, since this was done by operations (and the person I talked to), this PPA appears to be 'their solution to the cross-distro backports problem'. Thanks. I hadn't found this.

[skylerbunny]

If you find it easiest at this point to simply include the packages from this PPA in a different PR (along with the distro bump), I'd say go for it. It looks like a fairly straightforward upgrade with both pieces.

(If so, I'm all for closing this PR in favor of yours via master...kocio-pl:bionic ).

[kocio-pl]

I've learned about existence of this repo there: openstreetmap/chef#155 (comment).

[kocio-pl]

Well, for now please just add this PPA for xenial, to avoid Kosmtik plugin problems (which I prefer not to run into right now). Bionic update can be done later and your change would be useful then. OK?

[skylerbunny]

Changed the PR to do only what @kocio-pl requested, and changed the title of the PR appropriately.

[kocio-pl]

Thanks! It works as expected. Could you also squeeze commits into one, for sake of making things in a proper way?

BTW: could somebody check it on her laptop? It's a basic tool for coding and testing the style, so I'd like to be sure that coders are happy.

[skylerbunny]

Could you also squeeze commits into one, for sake of making things in a proper way?

All fixed up, commit 039af20.

[pnorman]

This gets us from osm2pgsql 0.88.1-1 to 0.96.0+ds-1build1ubuntu16.04.1ppa2

I normally encourage recent versions of osm2pgsql, but are there any bugs we know we're getting that impact the docker setup that this fixes?

Going to 18.04 as mentioned above would be worthwhile if we're going to use Ubuntu on the Dockerfile.

[skylerbunny]

but Kosmtik plugins are not available, so you should tune it anyway

Be careful about the change as you have it in master...kocio-pl:bionic . Lines 10-13 shouldn't be present at all for bionic, because nodejs (8.x) is a part of the core ubuntu:bionic docker image. All that needs to be done is to add npm (which is a completely separate package in bionic by design) to the list in line 7.

I have no idea what running https://deb.nodesource.com/setup_6.x will do on a system that already has nodejs 8.x installed but not npm. It may be your problem.

[skylerbunny]

@pnorman My experience is that this (exact) commit, 039af20 , is not causing problems for me, nor does it require an upgrade to bionic, because both Xenial and Bionic packages exist in the OSMF PPA.

That said:

1. I've toyed enough with the attempt to make Dockerfile and Dockerfile.import both bionic to say 'It's not completely trivial and probably wants its own new issue to develop a PR against'.

I normally encourage recent versions of osm2pgsql, but are there any bugs we know we're getting...

2. If you mean 'Does moving from 0.88 to 0.96 actually serve a visible purpose other than a version bump?' -- yes. To name one, this multipolygon relation, https://www.openstreetmap.org/relation/7144791 , renders all of its polygon parts correctly in 0.96, but not in 0.88, with all other components the same.

[kocio-pl]

I have no idea what running https://deb.nodesource.com/setup_6.x will do on a system that already has nodejs 8.x installed but not npm. It may be your problem.

Probably it takes the newest available version, effectively skipping 6.x. IIRC Kosmtik had some problems with version 8.x, but that needs some further research.

My code was just a fast hack to see how far we could go this way.

[skylerbunny]

Opened #3308 so that issue can be explored fully there (and it is distinct from this, which is an osm2pgsql upgrade against Xenial.)

[kocio-pl]

I think this is pretty small and rather safe change, so I prefer to have it already, so people could have better import to test before we make bigger update. Thanks!