Build Ubuntu RISCV64 Docker base images #1
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced May 1, 2024
mtelvers
force-pushed
the
build-riscv64-ubuntu-base
branch
from
fe83783 to
79314b2
Compare
Fix up for name collision
mtelvers
force-pushed
the
build-riscv64-ubuntu-base
branch
from
79314b2 to
af2f43e
Compare
shonfeder
reviewed
Jun 26, 2024
Comment on lines
+26
to
+27
| let base ?arch distro = | ||
| let repo = if arch = None then Conf.public_repo else Conf.staging_repo in |
There was a problem hiding this comment.
The default argument can be shortened to
Suggested change
| let base ?arch distro = | |
| let repo = if arch = None then Conf.public_repo else Conf.staging_repo in | |
| let base ?(arch=Conf.public_repo) distro = |
Comment on lines
+29
to
+30
| if distro = `Debian `Stable then "debian" | ||
| else Dockerfile_opam.Distro.tag_of_distro distro |
There was a problem hiding this comment.
Why can't we just use Dockerfile_opam.Distro.tag_of_distro without the conditional? Or, if we need just the distro name, could we use Dockerfile_opam.Distro.base_distro_tag?
| @@ -377,6 +408,7 @@ module Make (OCurrent : S.OCURRENT) = struct | |||
| let multiarch, pipelines = List.fold_left_map distro_pipeline Switch_map.empty distro_versions in | |||
| let pushes = | |||
| Switch_map.fold (fun switch images pushes -> | |||
| let distro_label = String.split_on_char '-' distro_label |> String.concat "-all-" in | |||
There was a problem hiding this comment.
Two questions here:
- Why add this new
-all-infix element? - It looks looks like windows labels can have multiple
-s, which would lead to wrong logic here, iiuc. I'm basing this on looking at https://github.com/shonfeder/ocaml-dockerfile/blob/b86eda9800d25f62a632b90f13ef44d9775a76e8/src-opam/distro.ml#L712-L719
| let build_distro_base ocluster arch distro = | ||
| let dist, release = Distro.base_distro_tag distro in | ||
| Current.component "%s %s" dist release |> | ||
| let> distro = Current.return distro in |
There was a problem hiding this comment.
Iiuc, this is a no-op, so can be removed
Suggested change
| let> distro = Current.return distro in |
Comment on lines
+142
to
+165
| let build_distro_base ocluster arch distro = | ||
| let dist, release = Distro.base_distro_tag distro in | ||
| Current.component "%s %s" dist release |> | ||
| let> distro = Current.return distro in | ||
| let dockerfile = | ||
| let open Dockerfile in | ||
| `Contents ( | ||
| string_of_t ( | ||
| from "ubuntu:jammy-20220801 AS base" @@ | ||
| workdir "/base" @@ | ||
| run "apt update && apt install -y debootstrap" @@ | ||
| run "ln -s gutsy /usr/share/debootstrap/scripts/noble" @@ | ||
| run "debootstrap %s /base" release @@ | ||
| from "scratch" @@ | ||
| copy ~from:"base" ~src:["/base"] ~dst:"/" () @@ | ||
| cmd "/bin/bash")) in | ||
| let options = { Cluster_api.Docker.Spec.defaults with | ||
| squash = squash distro; | ||
| buildkit = buildkit distro } in | ||
| let cache_hint = "ubuntu-" ^ release in | ||
| OCluster.Raw.build_and_push ocluster dockerfile | ||
| ~src:[] | ||
| ~cache_hint | ||
| ~push_target:(Cluster_api.Docker.Image_id.of_string Tag.(base ~arch distro) |> or_die) |
There was a problem hiding this comment.
ubuntu is hard coded throughout this function and we only call it for a single distro and architecture, so I'm not sure it makes sense for it to try to be general in other places, i.e., by using the newly defined Tag.base function and appearing as abstraction over arch and distro. Maybe we only really need the distro version at this point?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.