CI: Bump CI Lint base image from bionic to jammy #26581

robigan · 2022-11-26T08:44:26Z

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

DrahtBot · 2022-11-26T08:44:28Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#26716 (ci: Build python from source in "lint" task by hebasto)

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.

robigan · 2022-11-26T08:45:20Z

CC @hebasto & @MarcoFalke

hebasto · 2022-11-26T08:59:38Z

ci/lint/04_install.sh

-(
-  # Temporary workaround for https://github.com/bitcoin/bitcoin/pull/26130#issuecomment-1260499544
-  # Can be removed once the underlying image is bumped to something that includes git2.34 or later
-  sed -i -e 's/bionic/jammy/g' /etc/apt/sources.list
-  ${CI_RETRY_EXE} apt-get update
-  ${CI_RETRY_EXE} apt-get install -y --reinstall git
-)


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.

.cirrus.yml

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)

robigan · 2022-11-26T09:07:19Z

Just quickly amended my commit as I forgot pyenv complains about python versions being already present

hebasto · 2022-11-26T09:46:34Z

.cirrus.yml

+      - cat ./ci/lint_run_all.sh
+      - cat ./ci/lint/*.sh


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

hebasto · 2022-11-26T13:39:32Z

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?

hebasto · 2022-11-26T19:16:00Z

Here is an alternative implementation, which:

uses pyenv's 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
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:

robigan · 2022-11-27T00:59:28Z

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

robigan · 2022-11-27T11:11:05Z

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:

* 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.

robigan · 2022-11-27T11:14:12Z

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.

robigan · 2022-11-27T11:17:54Z

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

hebasto

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/

hebasto · 2022-11-27T13:49:15Z

ci/lint/04_install.sh

+${CI_RETRY_EXE} apt-get install -y make build-essential libssl-dev zlib1g-dev \
+libbz2-dev libreadline-dev libsqlite3-dev wget curl llvm \
+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

hebasto · 2022-11-27T14:04:23Z

Going to re-run the lint task manually, to check it with the cached Python.

robigan · 2022-11-27T15:09:33Z

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/

Then in that case I do strongly believe that making a separate task which only builds python is the best way

maflcko · 2022-12-13T16:28:34Z

Are you still working on this? See also #26637

hebasto · 2022-12-17T13:40:45Z

Are you still working on this? See also #26637

See #26716.

fanquake · 2022-12-20T16:04:23Z

No response here, so closing in favour of #26716.

robigan · 2023-01-02T01:04:49Z

Yeah no haven't been able to work on this given that I have family

123043e ci: Bump lint task image to Ubuntu Jammy (Hennadii Stepanov) 9b86114 ci: Use pyenv's `python-build` to install Python in lint task (Hennadii Stepanov) Pull request description: This PR: - is an alternative to #26581 and #26637 - closes #26548 Key advantages of this PR over others: - it uses pyenv's `python-build` [standalone](https://github.com/pyenv/pyenv/tree/master/plugins/python-build#using-python-build-standalone) - requires no additional computational resources Note for testing. The lint task must success regardless of whether the `python_cache` is populated or invalidated. ACKs for top commit: MarcoFalke: ACK 123043e fanquake: ACK 123043e Tree-SHA512: ba0fcdd4f2939a59692b173dcd1f5704444cfcfbb8111538c6f8160056d0536bba250e4f9b0f8c66f8b454e52034bf36ffe6afae76cdc0f7cc5b58b576d790ba

123043e ci: Bump lint task image to Ubuntu Jammy (Hennadii Stepanov) 9b86114 ci: Use pyenv's `python-build` to install Python in lint task (Hennadii Stepanov) Pull request description: This PR: - is an alternative to bitcoin#26581 and bitcoin#26637 - closes bitcoin#26548 Key advantages of this PR over others: - it uses pyenv's `python-build` [standalone](https://github.com/pyenv/pyenv/tree/master/plugins/python-build#using-python-build-standalone) - requires no additional computational resources Note for testing. The lint task must success regardless of whether the `python_cache` is populated or invalidated. ACKs for top commit: MarcoFalke: ACK 123043e fanquake: ACK 123043e Tree-SHA512: ba0fcdd4f2939a59692b173dcd1f5704444cfcfbb8111538c6f8160056d0536bba250e4f9b0f8c66f8b454e52034bf36ffe6afae76cdc0f7cc5b58b576d790ba

Bump lint task base image

b701c9a

Bump lint task base image from bionic to jammy

DrahtBot added the Tests label Nov 26, 2022

robigan force-pushed the ci-bump branch from 3db57e2 to d9ad290 Compare November 26, 2022 08:52

hebasto reviewed Nov 26, 2022

View reviewed changes

robigan force-pushed the ci-bump branch from d9ad290 to ced77c2 Compare November 26, 2022 09:03

hebasto reviewed Nov 26, 2022

View reviewed changes

DrahtBot mentioned this pull request Nov 26, 2022

build: Add CMake-based build system #25797

Closed

16 tasks

hebasto reviewed Nov 27, 2022

View reviewed changes

fanquake changed the title ~~CI: Bump CI Lint base image from bionic to jammy [Properly]~~ CI: Bump CI Lint base image from bionic to jammy Nov 28, 2022

fanquake mentioned this pull request Dec 5, 2022

ci: Use pyenv for the lint task #26637

Closed

hebasto mentioned this pull request Dec 17, 2022

ci: Build python from source in "lint" task #26716

Merged

fanquake closed this Dec 20, 2022

bitcoin locked and limited conversation to collaborators Jan 2, 2024

CI: Bump CI Lint base image from bionic to jammy #26581

CI: Bump CI Lint base image from bionic to jammy #26581

Uh oh!

Conversation

robigan commented Nov 26, 2022

Uh oh!

DrahtBot commented Nov 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

robigan commented Nov 26, 2022

Uh oh!

hebasto Nov 26, 2022

Choose a reason for hiding this comment

Uh oh!

robigan Nov 26, 2022

Choose a reason for hiding this comment

Uh oh!

hebasto Nov 26, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robigan commented Nov 26, 2022

Uh oh!

hebasto Nov 26, 2022

Choose a reason for hiding this comment

Uh oh!

robigan Nov 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hebasto Nov 26, 2022

Choose a reason for hiding this comment

Uh oh!

robigan Nov 27, 2022

Choose a reason for hiding this comment

Uh oh!

hebasto commented Nov 26, 2022

Uh oh!

hebasto commented Nov 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robigan commented Nov 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robigan commented Nov 27, 2022

Uh oh!

robigan commented Nov 27, 2022

Uh oh!

robigan commented Nov 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto left a comment

Choose a reason for hiding this comment

Uh oh!

hebasto Nov 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robigan Nov 27, 2022

Choose a reason for hiding this comment

Uh oh!

hebasto commented Nov 27, 2022

Uh oh!

robigan commented Nov 27, 2022

Uh oh!

maflcko commented Dec 13, 2022

Uh oh!

hebasto commented Dec 17, 2022

Uh oh!

fanquake commented Dec 20, 2022

Uh oh!

robigan commented Jan 2, 2023

Uh oh!

Uh oh!

DrahtBot commented Nov 26, 2022 •

edited

Loading

robigan Nov 26, 2022 •

edited

Loading

hebasto commented Nov 26, 2022 •

edited

Loading

robigan commented Nov 27, 2022 •

edited

Loading

robigan commented Nov 27, 2022 •

edited

Loading

hebasto Nov 27, 2022 •

edited

Loading