Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improves docker setup for basic usage to get started more easily #713

Merged
merged 1 commit into from
May 8, 2024

Conversation

daniel-j-h
Copy link
Contributor

Hi! 👋 Finally giving your project a try and because it's 2024 ain't nobody got time for building C++ from source, I checked out the docker images. In this changeset I make various improvements to the docker setup

  1. Using the stable debian bookworm as a base image as bullseye is outdated
  2. Not installing the development libraries with the tilemaker binary as they're only needed for building
  3. Compiling in parallel so that the docker build doesn't take 10+ minutes to build
  4. Adding a default command of --help so that if folks do docker run they get to see the tilemaker help
  5. Updated the docker readme and added a usage section in the default readme to use docker by default

I would highly recommend defaulting to telling users to just use the docker image and not having to compile tilemaker from source. There are so many compilation problems caused by dependencies to install, compiler and linker versions, and so on.

Just from a first look

If we could dumb down the getting started instructions to

docker run .. tilemaker input.pbf --output output.pbf

that would make it easier for users and hopefully less likely for us to get support tickets.

@daniel-j-h
Copy link
Contributor Author

FYI There are a few related docker PRs I'm seeing

and right now it looks like we're only pusblishing a single master tag

@systemed
Copy link
Owner

systemed commented May 8, 2024

This is great - thank you!

I know practically nothing about docker - the support that we have so far is entirely down to other contributors. So I'm very happy to merge this but I may be leaning on you and others for support in the future.

it's 2024 ain't nobody got time for building C++ from source

Call me old-school 😉 (but also it's seriously fast on Apple silicon...).

@daniel-j-h daniel-j-h force-pushed the improves-docker-setup branch from 50b9e4f to c1bd731 Compare May 8, 2024 16:18
@daniel-j-h
Copy link
Contributor Author

Got it, I believe it's fair to say that Docker will not go away so can recommend looking into it.

In the context of native projects like tilemaker the benefit I see

  • on the dev side is being able to compile in a self-contained development environment with a specific compiler against one set of libraries, so you don't have to support the combination of gcc, clang x library versions but e.g. whatever is in Debian LTS
  • on the user side the benefit is you don't have to compile anything at all but can just run the pre-built docker image

If you want to give this PR a try locally

docker build -t tilemaker .
docker run -it --rm tilemaker

I just rebased against master and added the tilemaker binary to PATH so #465 can be closed, too.

Like a said in #713 (comment) in the future a second iteration would be publishing not just a single tilemaker docker image but tagging them e.g. by date or version or commit hash. This can be a follow-up pull request.

Happy to get this in?

@systemed systemed merged commit 122ee1b into systemed:master May 8, 2024
7 checks passed
@systemed
Copy link
Owner

systemed commented May 8, 2024

Definitely - thanks!

@leonardehrenfried
Copy link
Contributor

Now that this one is also merged, would it be possible to take a look at #685 ?

@daniel-j-h daniel-j-h deleted the improves-docker-setup branch May 25, 2024 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants