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

Build a truly multi platform docker image #660

Merged
merged 9 commits into from
Nov 7, 2024

Conversation

podhrmic
Copy link
Contributor

@podhrmic podhrmic commented Oct 25, 2024

Dockerfile.deps Outdated
Comment on lines 20 to 26
RUN opam switch create with_coq $COMPILER_VERSION && \
eval `opam env --switch=with_coq` && \
opam repo add --yes --this-switch coq-released https://coq.inria.fr/opam/released && \
opam pin --yes -n coq-struct-tact https://github.com/uwplse/StructTact.git && \
opam repo add --yes --this-switch iris-dev https://gitlab.mpi-sws.org/iris/opam.git && \
opam pin --yes -n coq-sail-stdpp https://github.com/rems-project/coq-sail.git#f319aad && \
opam pin --yes -n coq-cheri-capabilities https://github.com/rems-project/coq-cheri-capabilities.git
Copy link
Contributor

@dc-mak dc-mak Nov 4, 2024

Choose a reason for hiding this comment

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

This is unnecessary, very slow and should be deleted.

Dockerfile.deps Outdated
RUN apt-get update
RUN apt-get upgrade -y
RUN apt-get install -y opam libgmp-dev libmpfr-dev
RUN apt-get install -y build-essential libgmp-dev z3 opam cmake pkg-config python3-distutils
Copy link
Contributor

@dc-mak dc-mak Nov 4, 2024

Choose a reason for hiding this comment

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

I believe pkg-config is required for zarith, and build-essential comes via opam, but the rest I'm not sure about?
Also, there's a known-bug with the default Z3 version in Ubuntu 22.04 so maybe a more recent binary release, or installing via opam, is more sensible (opam has version 4.13.0-3).

Copy link
Contributor

@dc-mak dc-mak left a comment

Choose a reason for hiding this comment

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

What's the intended use case for this Dockerfile?

Dockerfile.deps Outdated
Comment on lines 35 to 45
# Install Cerberus
RUN opam switch $COMPILER_VERSION && \
eval `opam env --switch=$COMPILER_VERSION` && \
opam pin --yes --no-action add cerberus-lib . && \
opam pin --yes --no-action add cerberus . && \
opam install --yes cerberus

# Run Cerberus CI tests
RUN opam switch $COMPILER_VERSION && \
eval `opam env --switch=$COMPILER_VERSION` && \
cd tests; USE_OPAM='' ./run-ci.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

These are also unnecessary, you should be able to skip straight to the CN section - it will pick up the dependencies automatically.

Dockerfile.deps Outdated
RUN opam switch $COMPILER_VERSION && \
eval `opam env --switch=$COMPILER_VERSION` && \
opam pin --yes --no-action add cn . && \
opam install --yes cn ocamlformat.0.26.2
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the Dockerfile being used for development or as an application? You won't need ocamlformat as an application, but you will for development (and the later source code formatting).

Copy link
Contributor

@dc-mak dc-mak Nov 4, 2024

Choose a reason for hiding this comment

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

opam install -y ./cerberus.opam /cerberus-lib ./cn.opam should also work without pinning first.

Dockerfile.deps Outdated
Comment on lines 62 to 67
# Check CN code formatting
RUN opam switch $COMPILER_VERSION && \
eval `opam env --switch=$COMPILER_VERSION` && \
USE_OPAM='' cd backend/cn && dune build @fmt

# Run CN CI tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto here.

@podhrmic
Copy link
Contributor Author

podhrmic commented Nov 4, 2024

What's the intended use case for this Dockerfile?

To run CN on OpenSUT code in a CI: https://github.com/GaloisInc/VERSE-OpenSUT (an example job is here). And more broadly to enable easy CN distribution, such that you don't need to install opam and build everything from scratch.

I think I can make the resulting image much smaller (just the compiled CN binary + basic OS dependencies should be sufficient).

@dc-mak
Copy link
Contributor

dc-mak commented Nov 6, 2024

Ok in that case, it's more as an application than as dev environment. Simply doing opam install -y ./cerberus.opam ./cerberus-lib.opam ./cn.opam should suffice after system deps, opam and correct ocaml version have been installed.

That by itself should be fairly minimal. I don't think CN is linked statically, and so it will need its library dependencies.

@podhrmic
Copy link
Contributor Author

podhrmic commented Nov 6, 2024

Correct, it is more an application in our use case. Do you think the steps outlined here would work for creating a statically linked binary? I am trying to avoid needing opam/ocaml for various reasons (see GaloisInc/VERSE-Toolchain#48)

@dc-mak
Copy link
Contributor

dc-mak commented Nov 6, 2024

It might work, but we have no experience with trying that and will be unable to help in case it doesn't. I don't see an explicit rejection of the OCaml platform on the link you sent, and opam is availabe on RHEL. But obviously you can proceed as you need to; we can only support with build instructions as given.

@podhrmic podhrmic force-pushed the 659-docker-image-manifest branch from 3450832 to 3d9a7b6 Compare November 6, 2024 19:41
@podhrmic
Copy link
Contributor Author

podhrmic commented Nov 6, 2024

I tried running opam install -y ./cerberus.opam ./cerberus-lib.opam ./cn.opam but it resulted in larger images that when using the Makefile steps.

I also added a RedHat image to the build.

@podhrmic podhrmic mentioned this pull request Nov 6, 2024
@podhrmic
Copy link
Contributor Author

podhrmic commented Nov 7, 2024

@dc-mak I believe the MR is now ready. The docker build works, but the image is not being pushed into the registry - I think this will be resolved for master builds. I also added more CI badges to the README.

RUN opam install --deps-only ./cerberus-lib.opam ./cn.opam

RUN eval `opam env` \
&& make install \
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is not necessary.


ENV OPAMCONFIRMLEVEL=unsafe-yes
RUN opam init --disable-sandboxing
RUN opam install dune lem
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is unnecessary too.

@dc-mak dc-mak merged commit e4de4e4 into rems-project:master Nov 7, 2024
4 checks passed
@podhrmic podhrmic deleted the 659-docker-image-manifest branch November 7, 2024 16:41
vzaliva pushed a commit to vzaliva/cerberus that referenced this pull request Dec 4, 2024
* Simplify the docker build, and properly build a multiplatform image with docker build actions

* Temporarily disable scheduled docker build for faster testing

* Clean up the release docker image

* Add a separate job for RedHat docker image

* Added RedHat dockerfile

* Revert "Temporarily disable scheduled docker build for faster testing"

This reverts commit 5afd280.

* Try to specify github tokens

* Add CI badges:

* Remove un-needed lines per @dc-mak 's suggestions
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.

[BUG] the docker multiplatform image not correctly built
2 participants