-
Notifications
You must be signed in to change notification settings - Fork 37.7k
ci: Use Cirrus CI dockerfile env #27340
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
The head ref may contain hidden characters: "2303-ci-env-\u{1F3FA}"
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsNo conflicts as of last run. |
ddf4cba
to
e31f87d
Compare
No idea why CI fails. Seems to be related to https://superuser.com/questions/1642858/git-on-debian-10-backports-throws-fatal-unable-to-access-https-github-com-us https://cirrus-ci.com/task/5391244998737920:
|
0e6363e
to
64c118f
Compare
This is needed to work around bitcoin#27340 (comment) The only change should be that python3.7 is bumped to 3.8, but this is fine because ci/test/00_setup_env_native_qt5.sh still checks for python3.7 compatibility.
This is needed to work around bitcoin#27340 (comment) The only change should be that python3.7 is bumped to 3.8, but this is fine because ci/test/00_setup_env_native_qt5.sh still checks for python3.7 compatibility.
This is needed to work around bitcoin#27340 (comment) The only change should be that python3.7 is bumped to 3.8, but this is fine because ci/test/00_setup_env_native_qt5.sh still checks for python3.7 compatibility.
fab6de4
to
fa90e47
Compare
This is needed to work around bitcoin#27340 (comment) The only change should be that python3.7 is bumped to 3.8, but this is fine because ci/test/00_setup_env_native_qt5.sh still checks for python3.7 compatibility.
This should give faster feedback about the CI result, while still keeping expenses reasonable.
This is needed to work around bitcoin#27340 (comment) The only change should be that python3.7 is bumped to 3.8, but this is fine because ci/test/00_setup_env_native_qt5.sh still checks for python3.7 compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code review ACK fa4a46d
read through the linked documentation from cirrus and this definitely makes sense. I tried to run the ci locally using cirrus run
but ran into some issues (unrelated to this PR). If you have any tips on how to test this, happy to give it a go, but the code looks good otherwise.
@@ -47,6 +47,7 @@ container_depends_template: &CONTAINER_DEPENDS_TEMPLATE | |||
cpu: 2 | |||
greedy: true | |||
memory: 8G # Set to 8GB to avoid OOM. https://cirrus-ci.org/guide/linux/#linux-containers | |||
dockerfile: ci/test_imagefile # https://cirrus-ci.org/guide/docker-builder-vm/#dockerfile-as-a-ci-environment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note to self/reviewers: this file was introduced in #26976
@@ -195,6 +197,7 @@ task: | |||
image: debian:bullseye | |||
cpu: 2 | |||
memory: 8G | |||
# docker_arguments: # Can use dockerfile after https://github.com/cirruslabs/cirrus-ci-docs/issues/1154 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I grep for "TODO"s a lot when looking for stuff to work on, might be nice to add a todo comment here to make it easier for others to find later on
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not really something to work on, other than maybe subscribing to the linked issue and follow-up when it is closed?
@@ -210,6 +210,7 @@ task: | |||
docker_arguments: | |||
CI_IMAGE_NAME_TAG: ubuntu:jammy | |||
FILE_ENV: "./ci/test/00_setup_env_win64.sh" | |||
<< : *CREDITS_TEMPLATE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is unrelated to switching to the docker file, right? would prefer this change in it's own PR because I have a few questions about cost (before / after, etc), but not a super big deal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to answer any questions. The cost will be displayed on every task in a badge.
I can help with running the CI locally, but not how to run Cirrus locally. I don't think testing this is possible, other than looking at the CI tasks for this pull? |
I installed the cirrus cli tool and ran |
Going to merge this now, so that |
fac395e ci: Bump ci/lint/Dockerfile (MarcoFalke) fa6eb65 test: Use python3.8 pow() (MarcoFalke) 88881cf Bump python minimum version to 3.8 (MarcoFalke) Pull request description: There is no pressing reason to drop support for 3.7, however there are several maintenance issues: * There is no supported operating system that ships 3.7 by default. (debian:buster is EOL and unmaintained to the extent that it doesn't run in the CI environment. See bitcoin/bitcoin#27340 (comment)) * Compiling python 3.7 from source is also unsupported on at least macos, according to bitcoin/bitcoin#24017 (comment) * Recent versions of lief require 3.8, see bitcoin/bitcoin#27507 (comment) Fix all maintenance issues by bumping the minimum. ACKs for top commit: RandyMcMillan: ACK fac395e fjahr: ACK fac395e fanquake: ACK fac395e Tree-SHA512: c198decdbbe29d186d73ea3f6549d8a38479383495d14a965a2f9211ce39637b43f13a4c2a5d3bf56e2d468be4bbe49b4ee8e8e19ec69936ff43ddf2b714c712
This is needed to work around bitcoin#27340 (comment) The only change should be that python3.7 is bumped to 3.8, but this is fine because ci/test/00_setup_env_native_qt5.sh still checks for python3.7 compatibility.
This is needed to work around bitcoin/bitcoin#27340 (comment) The only change should be that python3.7 is bumped to 3.8, but this is fine because ci/test/00_setup_env_native_qt5.sh still checks for python3.7 compatibility.
fac395e ci: Bump ci/lint/Dockerfile (MarcoFalke) fa6eb65 test: Use python3.8 pow() (MarcoFalke) 88881cf Bump python minimum version to 3.8 (MarcoFalke) Pull request description: There is no pressing reason to drop support for 3.7, however there are several maintenance issues: * There is no supported operating system that ships 3.7 by default. (debian:buster is EOL and unmaintained to the extent that it doesn't run in the CI environment. See bitcoin#27340 (comment)) * Compiling python 3.7 from source is also unsupported on at least macos, according to bitcoin#24017 (comment) * Recent versions of lief require 3.8, see bitcoin#27507 (comment) Fix all maintenance issues by bumping the minimum. ACKs for top commit: RandyMcMillan: ACK fac395e fjahr: ACK fac395e fanquake: ACK fac395e Tree-SHA512: c198decdbbe29d186d73ea3f6549d8a38479383495d14a965a2f9211ce39637b43f13a4c2a5d3bf56e2d468be4bbe49b4ee8e8e19ec69936ff43ddf2b714c712
fac395e ci: Bump ci/lint/Dockerfile (MarcoFalke) fa6eb65 test: Use python3.8 pow() (MarcoFalke) 88881cf Bump python minimum version to 3.8 (MarcoFalke) Pull request description: There is no pressing reason to drop support for 3.7, however there are several maintenance issues: * There is no supported operating system that ships 3.7 by default. (debian:buster is EOL and unmaintained to the extent that it doesn't run in the CI environment. See bitcoin#27340 (comment)) * Compiling python 3.7 from source is also unsupported on at least macos, according to bitcoin#24017 (comment) * Recent versions of lief require 3.8, see bitcoin#27507 (comment) Fix all maintenance issues by bumping the minimum. ACKs for top commit: RandyMcMillan: ACK fac395e fjahr: ACK fac395e fanquake: ACK fac395e Tree-SHA512: c198decdbbe29d186d73ea3f6549d8a38479383495d14a965a2f9211ce39637b43f13a4c2a5d3bf56e2d468be4bbe49b4ee8e8e19ec69936ff43ddf2b714c712
fac395e ci: Bump ci/lint/Dockerfile (MarcoFalke) fa6eb65 test: Use python3.8 pow() (MarcoFalke) 88881cf Bump python minimum version to 3.8 (MarcoFalke) Pull request description: There is no pressing reason to drop support for 3.7, however there are several maintenance issues: * There is no supported operating system that ships 3.7 by default. (debian:buster is EOL and unmaintained to the extent that it doesn't run in the CI environment. See bitcoin#27340 (comment)) * Compiling python 3.7 from source is also unsupported on at least macos, according to bitcoin#24017 (comment) * Recent versions of lief require 3.8, see bitcoin#27507 (comment) Fix all maintenance issues by bumping the minimum. ACKs for top commit: RandyMcMillan: ACK fac395e fjahr: ACK fac395e fanquake: ACK fac395e Tree-SHA512: c198decdbbe29d186d73ea3f6549d8a38479383495d14a965a2f9211ce39637b43f13a4c2a5d3bf56e2d468be4bbe49b4ee8e8e19ec69936ff43ddf2b714c712
This is needed to work around bitcoin/bitcoin#27340 (comment) The only change should be that python3.7 is bumped to 3.8, but this is fine because ci/test/00_setup_env_native_qt5.sh still checks for python3.7 compatibility.
Currently the CI env has many intermittent issues:
Fix all issues by using the Cirrus CI dockerfile env.