Quantisan / docker-clojure

Official Docker image for Clojure
https://hub.docker.com/_/clojure/
MIT License
204 stars 34 forks source link

Combine tools.deps and Leiningen images #229

Open alexander-yakushev opened 7 months ago

alexander-yakushev commented 7 months ago

This is my take on #224. There are a lot of changes, so we might as well start the review and discuss the details early.

Summary:

  1. Leiningen and tools.deps images no longer differ in their contents. All types of images install both Leiningen and tools.deps, and both are available on the PATH (as lein and clj/clojure, respectively).
  2. dockerfile.lein and dockerfile.tools-deps namespaces still exist and serve as building blocks for the new dockerfile.combined namespace. The combined namespace assembles the parts from Leiningen and tools.deps to install both in one RUN/layer. The number of overall layers in the resulting images is minimized.
  3. Images are still separated by their primary build tool. The only difference between lein and tools-deps images is their ENTRYPOINT. lein images continue to have lein as their entrypoint, tools-deps continue to have clj. tools-deps remain a "default" build tool, meaning that tags without build tool specified will have clj entrypoint.
  4. Downloading Leiningen artifacts is now done with curl. wget is no longer needed, so we don't install or purge it.
  5. We don't provide tags that specify both Leiningen and tools.deps version. Preliminary, it is unlikely that somebody will require both to be of the precise version.
  6. We currently support only the latest versions of both build tools. If, in the future, we need to provide images for multiple versions of some build tool, the versions of different build tools probably won't be dot-producted. Meaning that while the version of the primary build tool is fixed, the other build tool will still be of the latest version. In any case, the code to support multiple build tool versions is not in this PR, so it can be rethought later.
  7. latest tag no longer requires a separate docker build context, and now points to the image with default JDK, distro, and build tool (temurin-21-tools-deps-bookworm to be exact).
  8. Generated manifest remains almost the same, except for adding new types of tags – JDK version only (temurin-17, temurin-22, etc.). I don't know if not having those was intentional or an oversight. If it's the former, I can remove that.
  9. I did some cleanup around the code in places where I had to make changes. For example, I made cfg/build-tools (previously: installer-hashes) a dynamic variable, because it had to be dragged 5-6 levels through the call stack to the single place where it is actually needed.
  10. Tests fail for now because I changed plenty of code. I'd rather fix them once we agree on the implementation.

Will be happy to iterate on the PR now or let it sit until the decision on #224 is made.