-
Notifications
You must be signed in to change notification settings - Fork 38k
ci: Build ci_native_base image layer #33620
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
base: master
Are you sure you want to change the base?
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33620. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
(currently a draft, because this is based on some other pulls) |
Also, it won't work anyway, due to
|
I think if we want something like this to work it could be tricky without using a registry to host the base image which all other jobs pulled from (on a cache hit). It may be possible without one, but as each job is One possible approach I have in mind which could work:
cc @m3dwards in case you have any other ideas for approach here? |
a145313
to
5d12fe0
Compare
yeah, I think this makes sense. I just can't even figure out how to both support the gha cache backend and also the local storage. edit: Upstream bug: moby/buildkit#2343 We want to support:
Maybe all of this is too complicated and not worth it? |
So the approach we have used currently is to have CI-only config here: bitcoin/.github/actions/configure-docker/action.yml Lines 35 to 59 in 6c4fe40
which appends flags to the docker build only in CI. We always add If we are on Cirrus we also update the required URLs.
I agree this could be tricky to support all configurations you list though. |
This has a few benefits: * The raw python3 -c "..." command to calculate --cpuset-cpus lives in a Python context. * The shellcheck SC2086 warning is disabled for the whole command, but is only needed for the DOCKER_BUILD_CACHE_ARG env var. So in Python, only pass this one env var to shlex.split() for proper word splitting. The comments are moved, which can be checked via the git options: --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
5d12fe0
to
63a063f
Compare
63a063f
to
21cfc05
Compare
Hmm, then we should probably revert 6c4fe40 (or disable docker caching for GHA explicitly), because with the ~20 tasks here, and each task having a ccache size of 500MB, the GHA limit of 10GB should already be reached. With the 10GB limit, it seems better to use if for ccache+depends than to cache installed packages? |
Storage is cheap, so there was little value in extracting build layers in the CI images.
However, the GHA cache is limited to 10GB, so extracting a shared base layer could help to reduce the overall footprint. Possibly, it could even speed up image building, because installation is only done once.