readthedocs / readthedocs-docker-images

Docker image definitions used by Read the Docs
115 stars 70 forks source link

New image structure based on design document #166

Closed humitos closed 2 years ago

humitos commented 3 years ago

Dockerfile(s) required to build the new build images (ubuntu20-*) that use asdf to install languages requirements (python, node, rust, go)

Design documents:

Implementation:

How to build this image

  1. Clone this PR
  2. run docker build -t readthedocs/build:ubuntu-20.04 .

TODO

Closes #170 Closes #60 Closes #158 Closes #107 Closes #90 Closes #86 Closes #108 Closes #91 Closes #64 Closes #130 Closes #177 Closes #176 Closes #48

humitos commented 3 years ago

Good feedback here!

Couple apt-get update with rm -rf /var/lib/apt/lists/* in the same layer (although I don't think that plays along well with our multi-layered apt-get install - we would need to do update && install && rm -rf /var/lib/... in every layer, which will slow down the builds a bit)

docker run -u root -it readthedocs/build:ubuntu20 /bin/bash
root@42d3d0f8fe1a:/home/docs# du -sh /var/lib/apt/lists/
29M /var/lib/apt/lists/

I don't think we are spending a lot of disk space. However, I agree it could be a good optimization. As you already noticed, we will need to call apt-get update on each apt-get install command that we run.

On the other hands, packages (.deb files) are cleaned up automatically when using official Ubuntu images because they comes with a post-install hook for this:

docs@927b877e91c3:~$ cat /etc/apt/apt.conf.d/docker-clean
DPkg::Post-Invoke { "rm -f /var/cache/apt/archives/*.deb /var/cache/apt/archives/partial/*.deb /var/cache/apt/*.bin || true"; };
APT::Update::Post-Invoke { "rm -f /var/cache/apt/archives/*.deb /var/cache/apt/archives/partial/*.deb /var/cache/apt/*.bin || true"; };
Dir::Cache::pkgcache ""; Dir::Cache::srcpkgcache "";
docs@927b877e91c3:~$

Use --no-install-recommends to be more explicit about what are we installing and why

I'm not sure about this one. I think it's a good idea to know exactly what we are installing. However, if add this option now, I wonder if all the PDFs will keep building correctly (e.g. the right font, or stylesheets).

humitos commented 2 years ago

@agjohnson I addressed all your comments and made the tests run on CircleCI. Is there any blocker to merge this PR?

pradyunsg commented 2 years ago

Hurray! 🎉