This PR:
Key advantages of this PR over others:
python-build standaloneNote for testing. The lint task must success regardless of whether the python_cache is populated or invalidated.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | fanquake, MarcoFalke |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
No conflicts as of last run.
it uses pyenv’s python-build standalone; no other tools required
As far as I can tell, pyenv install is just a wrapper around python-build. You still need the same tools (compiler / linker / all the python build dependencies) to compile Python regardless of how you compile it. I guess the difference here is just not fully installing pyenv?
requires no additional computational resources
GIven that pyenv install and python-build are the same thing, why is there a difference in computational resources required?
22+ (
23+ git clone https://github.com/pyenv/pyenv.git
24+ cd pyenv/plugins/python-build || exit 1
25+ ./install.sh
26+ )
27+ ${CI_RETRY_EXE} apt-get install -y build-essential clang libbz2-dev liblzma-dev libncursesw5-dev libreadline-dev libssl-dev libsqlite3-dev zlib1g-dev
it uses pyenv’s python-build standalone; no other tools required
As far as I can tell,
pyenv installis just a wrapper aroundpython-build. You still need the same tools (compiler / linker / all the python build dependencies) to compile Python regardless of how you compile it. I guess the difference here is just not fully installing pyenv?
Correct. Dropped “no other tools required” from the PR description.
requires no additional computational resources
GIven that
pyenv installandpython-buildare the same thing, why is there a difference in computational resources required?
I reckon the lower memory usage is due to using clang instead of gcc.
75 # For faster CI feedback, immediately schedule the linters
76 << : *CREDITS_TEMPLATE
77+ python_cache:
78+ folder: "/tmp/python"
79+ fingerprint_script: cat .python-version /etc/os-release
80+ populate_script: rm -rf /tmp/python
Updated 3f602a628bdb8007c2ebebe58c10f42db787679f -> f99260fdd02453649222bfe4d38d0655ebdbc42c (pr26716.02 -> pr26716.03, diff):
23+ git clone https://github.com/pyenv/pyenv.git
24+ cd pyenv/plugins/python-build || exit 1
25+ ./install.sh
26+ )
27+ # For dependencies see https://github.com/pyenv/pyenv/wiki#suggested-build-environment
28+ ${CI_RETRY_EXE} apt-get install -y build-essential clang libbz2-dev liblzma-dev libncursesw5-dev libreadline-dev libssl-dev libsqlite3-dev zlib1g-dev
Updated f99260fdd02453649222bfe4d38d0655ebdbc42c -> d418803a6a60bfb0d51ed5deb658ffd7639f31d5 (pr26716.03 -> pr26716.05, diff):
CI tasks with the python cache being:
17@@ -18,22 +18,15 @@ if [ ! -d "${PYTHON_PATH}/bin" ]; then
18 )
19 # For dependencies see https://github.com/pyenv/pyenv/wiki#suggested-build-environment
20 ${CI_RETRY_EXE} apt-get install -y build-essential libssl-dev zlib1g-dev \
21- libbz2-dev libreadline-dev libsqlite3-dev curl llvm \
22+ libbz2-dev libreadline-dev libsqlite3-dev curl clang llvm \
23 libncursesw5-dev xz-utils tk-dev libxml2-dev libxmlsec1-dev libffi-dev liblzma-dev
24- python-build "$(cat "${BASE_ROOT_DIR}/.python-version")" "${PYTHON_PATH}"
25+ # Using clang to workaround https://github.com/pyenv/pyenv/issues/2359.
How/why does using Clang solve the segmentation fault issue?
I have no answers, unfortunately. FWIW, #26418 (review).
Also seems unecessary to put this in the second commit, instead of the first.
There were no problems with gcc on bionic.
I have no answers, unfortunately.
Well linking to this thread, that doesn’t explain anything, and just has a comment with a link to a comment in a different thread, where someone claimed that using Clang magically solved their problem, is more confusing than useful.
Presumably a gcc compiler bug?
Perhaps. But do we really want to dig into gcc bugtracker for the sake of building an old python?
Does this comment look good
0# Using clang for less memory footprint and to workaround https://github.com/pyenv/pyenv/issues/2359.
?
Also seems unecessary to put this in the second commit, instead of the first.
Done.
I don’t think there is any value in linking to pyenv/pyenv#2359.
Dropped.
Updated d418803a6a60bfb0d51ed5deb658ffd7639f31d5 -> 123043e99cf3aab9eef7e381b133477b518ac4d0 (pr26716.05 -> pr26716.06, diff):
13- # Can be removed once the underlying image is bumped to something that includes git2.34 or later
14- sed -i -e 's/bionic/jammy/g' /etc/apt/sources.list
15- ${CI_RETRY_EXE} apt-get update
16- ${CI_RETRY_EXE} apt-get install -y --reinstall git
17-)
18+${CI_RETRY_EXE} apt-get install -y curl git gawk jq xz-utils
xz-utils here when it is only needed when compiling python?
This change points to the wastefulness of our current CI setup as alluded to in the Cirrus docs:
On every CI run, we’re doing the same apt installs and Python setup. We should probably just specify a Dockerfile, the resulting image of which can be cached on the Cirrus side. I think that would also simplify CI on our end and make it easier to run locally.
If there’s interest I can create an issue for this.
Yeah, I did this because I want to be able to run the stuff easily locally: https://github.com/bitcoin/bitcoin/tree/master/ci#running-a-stage-locally
I don’t know enough about docker to figure out if there is a more efficient but equally simply way to do this.
Overall I am thinking that it shouldn’t matter if we download the packages or download a pod layer with the packages installed. (Modulo a slow mirror, which should ideally be solved by using a fast mirror, but there doesn’t seem to be an easy way to do this? Also modulo the time it takes to run the install script)