As the title says, this PR aims to address #26548 by switching the lint CI job from ubuntu's bionic base image which has python 3.6.5 to ubuntu jammy and use pyenv to install the correct python version as dictated in the project's .python-version file.
DISCLAIMER: This PR is a copy of #26572 as there were issues with that PR and how merging and squashing from the master branch were done
Bump lint task base image from bionic to jammy
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process. A summary of reviews will appear here.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, 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.
CC @hebasto & @MarcoFalke
12 | - # Temporary workaround for https://github.com/bitcoin/bitcoin/pull/26130#issuecomment-1260499544 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 | -)
Shouldn't this removal be a part of the first commit?
No, as the first commit just addresses bumping the version. This addresses actually removing the sources for making way for pyenv
The removed lines were a workaround to deal with an old/broken git version in Ubuntu Bionic. After bumping to Bionic, it is no longer required. And it has nothing to do with pyenv. That's why I still believe this change belongs to the first commit.
74 | + cpu: 2 # Assign 2 cores and 4G of ram in case .python-version has changed and cirrus has to rebuild python 75 | + memory: 4G 76 | + python_cache: 77 | + folder: /tmp/python-build # Uses intermediary directory for python caching. See the install.sh script in ci/lint/ 78 | + fingerprint_script: 79 | + - echo $CIRRUS_OS
This task is the only user of python_cache. So why echo $CIRRUS_OS?
Portability/future-proofing, the Cirrus docs also say that it is preferable to use it when caching content that is OS specific, and the python builds are OS specific
This patch:
- Sets up a cache in .cirrus.yml for pyenv's building of python 3.6.15 at the location /tmp/python-build/
- Uses Operating System ($CIRRUS_OS), .python-version and checks the contents of the install scripts for calculating the hash of the cache
- Installs necessary build libraries for building python with pyenv
- Installs pyenv, runs init scripts and adds it to the path
- Checks if a python cache is available and copies it to pyenv's version directories. If not, install it
- And copies the python version in pyenv to the intermediary cache directory (It's up to Cirrus to decide if to cache it or not)
Just quickly amended my commit as I forgot pyenv complains about python versions being already present
78 | + folder: /tmp/python-build # Uses intermediary directory for python caching. See the install.sh script in ci/lint/ 79 | + fingerprint_script: 80 | + - echo $CIRRUS_OS 81 | + - cat .python-version 82 | + - cat ./ci/lint_run_all.sh 83 | + - cat ./ci/lint/*.sh
Why?
If you change the lint scripts (which might change how you interact with the cache), you'd want to be able to test that no? And if you do end up changing how caching works. You'd want a new cache uploaded wouldn't you?
Still trying to understand you, but we don't want to consume resources to rebuild python after every change in linter scripts. What kind of such changes could lead to different python binaries?
Consume? I wouldn't say so, 2 cores and 4G of RAM are small amounts nowadays. If you really want to be specific, then I could put the python build scripts into a separate file and cat only that. Or if you really really want to make sure you don't waist any resources, then a separate task which builds python in the Cirrus CI is needed
This:
/root/.pyenv/plugins/python-build/bin/python-build: line 1790: 17770 Segmentation fault (core dumped) "$PYTHON_BIN" -c "import $1" > /dev/null 2>&1
WARNING: The Python ctypes extension was not compiled. Missing the libffi lib?
/root/.pyenv/plugins/python-build/bin/python-build: line 2001: 17777 Segmentation fault (core dumped) "$PYTHON_BIN" -s -m ensurepip ${ensurepip_opts} > /dev/null 2>&1
does not look correct, does it?
Here is an alternative implementation, which:
python-build standalone; no other tools requiredNOTE: To test the lint CI task in a personal Cirrus account, the patch from #20728 is required. For example:
Both implementations are just as good. But I'll provide further comment in the morning, it's late and I am tipsy so I need some good sleep before I do anything
This:
/root/.pyenv/plugins/python-build/bin/python-build: line 1790: 17770 Segmentation fault (core dumped) "$PYTHON_BIN" -c "import $1" > /dev/null 2>&1 WARNING: The Python ctypes extension was not compiled. Missing the libffi lib? /root/.pyenv/plugins/python-build/bin/python-build: line 2001: 17777 Segmentation fault (core dumped) "$PYTHON_BIN" -s -m ensurepip ${ensurepip_opts} > /dev/null 2>&1does not look correct, does it?
Here is an alternative implementation, which:
* uses pyenv's `python-build` [standalone](https://github.com/pyenv/pyenv/tree/master/plugins/python-build#using-python-build-standalone); no other tools required * switches from gcc to clang to avoid issues like [Segmentation fault when installing Python 3.6.15 on Ubuntu 22.04 pyenv/pyenv#2359](https://github.com/pyenv/pyenv/issues/2359) * avoids copying of the cache * requires no additional computational resources@robigan Feel free to cherry-pick it into this PR.
NOTE: To test the lint CI task in a personal Cirrus account, the patch from #20728 is required. For example:
* https://cirrus-ci.com/task/6626175015976960 * https://cirrus-ci.com/task/6276286176296960
Yes I noticed this, I ignored it because it wasn't affecting the linting process in any way, but is worth addressing. Clang could be a good alternative. And we can skip using python-build from pyenv and set the flags right from the pyenv install script. Although it doesn't address the fact that it seems it has got to do with openssl instead of fixing it with clang, as pointed out by the issue you linked.
Here is an alternative implementation, which:
* uses pyenv's `python-build` [standalone](https://github.com/pyenv/pyenv/tree/master/plugins/python-build#using-python-build-standalone); no other tools required * switches from gcc to clang to avoid issues like [Segmentation fault when installing Python 3.6.15 on Ubuntu 22.04 pyenv/pyenv#2359](https://github.com/pyenv/pyenv/issues/2359) * avoids copying of the cache * requires no additional computational resources@robigan Feel free to cherry-pick it into this PR.
NOTE: To test the lint CI task in a personal Cirrus account, the patch from #20728 is required. For example:
* https://cirrus-ci.com/task/6626175015976960 * https://cirrus-ci.com/task/6276286176296960
Python-build can be used if we so wish, as I have said, using pyenv executable just makes maintaining this easier. And if we so wish to use clang, we can still use pyenv. And what do you mean by it doesn't use other tools? It's all the same thing.
Here is an alternative implementation, which:
* uses pyenv's `python-build` [standalone](https://github.com/pyenv/pyenv/tree/master/plugins/python-build#using-python-build-standalone); no other tools required * switches from gcc to clang to avoid issues like [Segmentation fault when installing Python 3.6.15 on Ubuntu 22.04 pyenv/pyenv#2359](https://github.com/pyenv/pyenv/issues/2359) * avoids copying of the cache * requires no additional computational resources@robigan Feel free to cherry-pick it into this PR.
NOTE: To test the lint CI task in a personal Cirrus account, the patch from #20728 is required. For example:
* https://cirrus-ci.com/task/6626175015976960 * https://cirrus-ci.com/task/6276286176296960
Copying of the cache is hardly an issue, copy takes less than a second. That's if you're coming from a resource waste point of view. If it's for simplicity's sake, then yes I agree. And for the record it was @MarcoFalke's idea to bump resources used by the linting task. If we want to do this the right way, then the building of python itself should be done in a separate Cirrus CI Task, and then by using caches, deploy the caches as needed to other tasks
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 make build-essential libssl-dev zlib1g-dev \ 19 | +libbz2-dev libreadline-dev libsqlite3-dev wget curl llvm \ 20 | +libncursesw5-dev xz-utils tk-dev libxml2-dev libxmlsec1-dev libffi-dev liblzma-dev
Assuming that those packages are required to build the Python, why are they installed every time, regardless of using of the cached Python?
The curl package was installed two lines above.
Why are llvm, xz-utils and libffi-dev packages required?
The packages listed there is an exact copy as detailed by the pyenv docs. As I have said, my concern is making it as maintainable as possible
As we have a self-compiled Python, the python3-pip package can be dropped now, right?
After the first commit, the lint task uses the system Python which is not v3.6.15. It is the reason to re-order commits.
Regarding resources, I'll try to clarify my statement. It is not about resources actually consumed by a task. Rather about the requested amount of resources, because they do affect the scheduling time when using Cirrus Community Cluster. See: https://cirrus-ci.org/guide/linux/
As we have a self-compiled Python, the
python3-pippackage can be dropped now, right?After the first commit, the lint task uses the system Python which is not v3.6.15. It is the reason to re-order commits.
Regarding resources, I'll try to clarify my statement. It is not about resources actually consumed by a task. Rather about the requested amount of resources, because they do affect the scheduling time when using Cirrus Community Cluster. See: https://cirrus-ci.org/guide/linux/
Then in that case I do strongly believe that making a separate task which only builds python is the best way
Are you still working on this? See also https://github.com/bitcoin/bitcoin/pull/26637
Yeah no haven't been able to work on this given that I have family