Storing cached stuff in host system folders may lead to unexpected issues when the ci-built stuff is used for a non-ci build or a ci task leaks into another ci task.
ci: Cache stuff in volumes, not host folders #27033
pull maflcko wants to merge 2 commits into bitcoin:master from maflcko:2302-ci-depends-vol-🏆 changing 3 files +11 −8-
maflcko commented at 10:28 AM on February 3, 2023: member
-
DrahtBot commented at 10:28 AM on February 3, 2023: contributor
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--021abf342d371248e50ceaed478a90ca-->
Reviews
See the guideline for information on the review process.
Type Reviewers ACK john-moffett If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
- DrahtBot added the label Tests on Feb 3, 2023
- maflcko force-pushed on Feb 3, 2023
- maflcko renamed this:
ci: Store depends in volume, not host folder
ci: Cache stuff in volumes, not host folders
on Feb 8, 2023 - DrahtBot renamed this:
ci: Cache stuff in volumes, not host folders
ci: Cache stuff in volumes, not host folders
on Feb 8, 2023 -
ci: Cache stuff in volumes, not host folders 5fffff54e9
- maflcko force-pushed on Feb 8, 2023
-
maflcko commented at 10:49 AM on February 9, 2023: member
Sanity check that ccache still works on the one task that runs this code path: https://cirrus-ci.com/task/5496539091566592?logs=ci#L2401
-
john-moffett commented at 7:14 PM on February 9, 2023: contributor
Approach ACK. Will test shortly.
- john-moffett approved
-
john-moffett commented at 3:34 PM on February 10, 2023: contributor
tACK https://github.com/bitcoin/bitcoin/pull/27033/commits/5fffff54e9fcf154c722dc421025a567fa0c5c97
Ran on Ubuntu arm64 host and performed as expected.
I'm torn over whether this should be updated:
-
doc: Update ci docs fa8e92c022
-
john-moffett commented at 4:08 PM on February 10, 2023: contributor
ACK fa8e92c022057adcb8b98647bde626ed9c054df2
-
maflcko commented at 4:09 PM on February 10, 2023: member
Yes, thx, updated. I think there are some follow-ups possible:
- Run the
rsync -a /ro_base/ ...unconditionally, to a static folder name. Maybe/ci_scratch/? - Replace
BASE_ROOT_DIRby either this folder or the host folder to avoid confusion.
Also,
- This should allow to remove the second user account in the docker container, as no files are propagated back
- Maybe this cleanup can be done after #27028 to avoid conflicts
Finally, running
config.guessin https://github.com/bitcoin/bitcoin/blob/5fffff54e9fcf154c722dc421025a567fa0c5c97/ci/test/00_setup_env.sh#L33 may violate the statment that only the packagesbash docker.ioare required to run the CI system. Not sure how to fix this and if it is worth it. - Run the
- maflcko merged this on Feb 13, 2023
- maflcko closed this on Feb 13, 2023
- maflcko deleted the branch on Feb 13, 2023
- sidhujag referenced this in commit d4999cf6f1 on Feb 13, 2023
- bitcoin locked this on Feb 13, 2024
Contributors
Labels