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

Combine tools.deps and Leiningen images #229

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alexander-yakushev
Copy link
Contributor

@alexander-yakushev alexander-yakushev commented Apr 24, 2024

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.

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.

1 participant