Skip to content

Dockerfile cleanup #2861

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

Merged
merged 3 commits into from
Dec 12, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 27 additions & 18 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,32 +1,41 @@
FROM ocaml/opam:alpine-3.20-ocaml-5.2 as build
FROM ocaml/opam:alpine-3.20-ocaml-5.2 AS build

# Install system dependencies
RUN sudo apk update && sudo apk add --update libev-dev openssl-dev gmp-dev oniguruma-dev inotify-tools curl-dev autoconf
RUN sudo apk -U upgrade --no-cache && sudo apk add --no-cache \
autoconf \
curl-dev \
gmp-dev \
inotify-tools \
libev-dev \
oniguruma-dev \
openssl-dev

# Branch freeze was opam-repo HEAD at the time of commit
RUN cd opam-repository && git pull origin c45f5bab71d3589f41f9603daca5acad14df0ab0 && opam update
RUN cd ~/opam-repository && git fetch -q origin master && git reset --hard c45f5bab71d3589f41f9603daca5acad14df0ab0 && opam update

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has always confused me. I think we've changed this line a couple of times. Back and forth between what we have now and what you're suggesting. Your line downloads some more doesn't it? Is there another difference?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my view, here, to avoid docker cache miss, we should just do git checkout to a hash that is known to be already in opam-repo history.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it downloads more, as per the docs:

More precisely, git pull runs git fetch with the given parameters and then depending on configuration options or command line flags, will call either git rebase or git merge to reconcile diverging branches.

So here by splitting git pull into git fetch and git reset, we're avoiding conflict resolution, though it's not clear to me why there would be any conflicts in the first place. Corruption could happen.

The base image already ships the opam repository, I tend to set this hash to the one already present in the image, so that the git reset operation is mostly free. It can be needed from time to time to update to a newer opam-repository, or just set it explicitly for reproducibility/debugging purposes. The base images are updated weekly, I don't know how often the deployed image is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deployed image is updated fairly frequently. Several times a week, often more. If we can take advantage of this, it would be nice. That's why we are considering using a docker image pin instead of an opam repo pin

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both may be needed, as the docker image is updated weekly, you might want to rely on a dependency that's been released during the week but not yet available in the base image.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No we don't want to do that :-) We're laggards. We want to use old stuff that has been around for some time, preferably a long time. We run away from anything new, fresh, fancy or cool :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be new, fresh, fancy, or cool bug fixes!

WORKDIR /home/opam

# Install Opam dependencies
ADD ocamlorg.opam ocamlorg.opam
# Install opam dependencies
COPY --chown=opam ocamlorg.opam .
RUN opam install . --deps-only

# Build project
COPY --chown=opam:opam . .
COPY --chown=opam . .
RUN opam exec -- dune build @install --profile=release

# Launch project in order to generate the package state cache
RUN cd opam-repository && git checkout master && git pull origin master && opam update
ENV OCAMLORG_REPO_PATH opam-repository
ENV OCAMLORG_PKG_STATE_PATH package.state
RUN cd ~/opam-repository && git checkout master && opam update
ENV OCAMLORG_PKG_STATE_PATH=package.state \
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, if you pulled in the previous stage, you don't need to do it again.

OCAMLORG_REPO_PATH=opam-repository
RUN touch package.state && ./init-cache package.state

FROM alpine:3.20 as run

RUN apk update && apk add --update libev gmp git
FROM alpine:3.20

RUN chmod -R 755 /var
RUN apk -U upgrade --no-cache && apk add --no-cache \
git \
gmp \
libev

COPY --from=build /home/opam/package.state /var/package.state
COPY --from=build /home/opam/opam-repository /var/opam-repository
Expand All @@ -38,12 +47,12 @@ RUN git clone https://github.com/ocaml-web/html-compiler-manuals /manual

RUN git config --global --add safe.directory /var/opam-repository

ENV OCAMLORG_REPO_PATH /var/opam-repository/
ENV OCAMLORG_MANUAL_PATH /manual
ENV OCAMLORG_PKG_STATE_PATH /var/package.state
ENV DREAM_VERBOSITY info
ENV OCAMLORG_HTTP_PORT 8080
ENV DREAM_VERBOSITY=info \
OCAMLORG_HTTP_PORT=8080 \
OCAMLORG_MANUAL_PATH=/manual \
OCAMLORG_PKG_STATE_PATH=/var/package.state \
OCAMLORG_REPO_PATH=/var/opam-repository/

EXPOSE 8080

ENTRYPOINT /bin/server
ENTRYPOINT ["/bin/server"]
Loading