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

Remove opam-repository from the dockerfiles created by dockerfile-opam #87

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kit-ty-kate
Copy link
Contributor

@kit-ty-kate kit-ty-kate commented Jan 30, 2022

See ocurrent/docker-base-images#99

To me having opam-repository is a straight downside in every regard. For the few specialized CI systems that require it for some reason they can clone it by hand and rely on the docker cache to offset the time cost.

This is a breaking change and should be announced on Discuss before merging of course. As well as carefully removing uses of opam-repository in the CI systems and Dockerfiles that rely on it.

TODO:

  • opam-health-check does not rely on it (always starts with rm -rf opam-repository)
  • opam-repo-ci does not use it, however its deployment dockerfiles do (fixed by Make the deployment Dockerfile resistant against changes in the base images opam-repo-ci#151)
  • ocaml-ci relies on it, as well as its deployment dockerfiles
  • ocluster uses a hash for both of its deployment dockerfiles
  • ocaml-docs-ci seems to be clear from it in both its dockerfile and internal actions (uses opam repo set-url default ...)
  • ocaml-multicore-ci uses it for both its dockerfiles and internal actions
  • ocurrent uses it for its dockerfile
  • mirage-ci seems to use it for at least its deployment dockerfile but I’m not sure about its internal actions
  • ocurrent-deployer’s deployment dockerfile uses a hash for its base image
  • current-bench: no idea
  • ...

@avsm
Copy link
Member

avsm commented Oct 13, 2022

I still can't form an opinion about this PR without knowing which users it will break (outside of the CI systems above). Perhaps posting on discuss.ocaml.org about the proposed change would be worthwhile first, @kit-ty-kate ?

@benmandrew
Copy link
Contributor

@kit-ty-kate any updates on this?

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.

3 participants