Bump python minimum version to 3.8 #27483

maflcko · 2023-04-18T11:06:02Z

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 ci: Use Cirrus CI dockerfile env #27340 (comment))
Compiling python 3.7 from source is also unsupported on at least macos, according to Bump Python dependency version #24017 (comment)
Recent versions of lief require 3.8, see lint: stop ignoring LIEF imports #27507 (comment)

Fix all maintenance issues by bumping the minimum.

DrahtBot · 2023-04-18T11:06:05Z

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	fanquake, RandyMcMillan, fjahr
Concept ACK	stickies-v, jamesob
Stale ACK	TheCharlatan

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#26222 (Introduce field element and group element classes to test_framework/key.py by sipa)
#25797 (build: Add CMake-based build system by hebasto)
#24005 (test: add python implementation of Elligator swift by stratospher)

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.

stickies-v

Concept ACK

I'm not sure why we're switching to focal instead of bullseye for the qt5 ci, but I don't really have a view either way. Just pointing it out.

doc/dependencies.md

maflcko · 2023-04-18T11:49:46Z

I'm not sure why we're switching to focal instead of bullseye

The reason is that bullseye does not have a single of the needed packages:

(Also, bullseye will be EOL a year earlier than focal)

fanquake · 2023-04-18T12:59:34Z

Concept ACK

maflcko · 2023-04-18T16:36:18Z

Unrelated: Looks like there are a few bugs with GCC libstdc++-9:

Bitcoin Core compiled with Ubuntu Focal gcc-8 immediately segfaults #27487 (comment)
clang-8: ./util/bitdeque.h:240:5: error: exception specification of explicitly defaulted move constructor does not match the calculated one (https://cirrus-ci.com/task/6721414045106176?logs=ci#L3390)
clang-8/9 libc++ DEBUG=1 in depends: Segfault (https://cirrus-ci.com/task/6091472936632320?logs=ci#L4372)

So I worked around them.

Also, switch ci_native_qt5 to clang-8 (from g++-8) to work around bugs. This should be fine, because the i686_centos task still checks for g++-8 compatibility. The bugs were that neither clang-8, nor g++-8, would play nicely with GCC libstdc++-9. See bitcoin#27483 (comment)

Also, switch ci_native_qt5 to clang (from g++-8) to work around bugs. This should be fine, because the i686_centos task still checks for g++-8 compatibility. See bitcoin#27483 (comment) for the list of bugs.

Also, switch ci_native_qt5 to g++-9 (from g++-8) to work around bugs. This should be fine, because the i686_centos task still checks for g++-8 compatibility. See bitcoin#27483 (comment) for the list of bugs.

This bump should not be needed, see discussion starting at bitcoin#27483 (comment)

RandyMcMillan · 2023-04-27T14:43:06Z

ACK fac395e

#27130 (comment)

fjahr · 2023-04-27T17:36:34Z

ACK fac395e

This is ok to merge but when I wrote that comment I meant to remove the modinv() function completely, e.g. fjahr@75b8ba5. You can pull this commit in here or we merge this now and I will open it as a follow-up. I think the comment in modinv() isn't accurate anymore after this change because python seems to use Exponentiation by Squaring and we don't really need a one-line function that and we also don't need to test native python code.

maflcko · 2023-04-28T09:03:28Z

Thanks. I think it is best for you to open a follow-up, unless people want me to push the commit here.

fanquake · 2023-04-28T09:09:52Z

I think it is best for you to open a follow-up,

Lets do that.

dc14ba0 test: remove modinv python util helper function (Fabian Jahr) Pull request description: Since #27483 was merged the `modinv()` body is just one line calling pythons own implementation of `pow()`. We can just remove the function as it doesn't seem to add any value. Additionally the comment in the function is now outdated and the test is only testing two ways of doing modular inverse but both using python's `pow()` function. ACKs for top commit: theStack: ACK dc14ba0 Tree-SHA512: e8b470c72dc3f9fd53699d0684650517b1ea35ad1d4c01cf9472c80d3e4474c0c72e429c0bd201eb99d204c87eee0d68285e6a388e4c506f30e14b2bff9c1c32

Also, switch ci_native_qt5 to g++-9 (from g++-8) to work around bugs. This should be fine, because the i686_centos task still checks for g++-8 compatibility. See bitcoin#27483 (comment) for the list of bugs.

This bump should not be needed, see discussion starting at bitcoin#27483 (comment)

Also, switch ci_native_qt5 to g++-9 (from g++-8) to work around bugs. This should be fine, because the i686_centos task still checks for g++-8 compatibility. See bitcoin/bitcoin#27483 (comment) for the list of bugs.

This bump should not be needed, see discussion starting at bitcoin/bitcoin#27483 (comment)

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

backport: bitcoin#23416,bitcoin#18997,bitcoin#18996, partial bitcoin#27483 (TODO cleanup)

Summary: And remove some compatibility code. note that Electrum ABC is currently packaged with 3.9.13 Supported Linux operating systems ([[bitcoin/bitcoin#27483 | core#27483]]): > FreeBSD 12/13 also ships with 3.9 > CentOS-like 8/9 also ships with 3.9 (and 3.11) > OpenSuse Leap also ships with 3.9 (and 3.11) Debian bullseye or newer Ubuntu 21.04 and newer Note that it is relatively easy to install python 3.9 on Ubuntu 20.04 with the package manager: https://tecadmin.net/how-to-install-python-3-9-on-ubuntu-20-04/ Test Plan: `ninja check-functional` Check the doc for `hmac.digest` and `concurrent.futures` to confirm the API is compatible with python 3.9. Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D15373

DrahtBot changed the title ~~Bump python minimum version to 3.8~~ Bump python minimum version to 3.8 Apr 18, 2023

maflcko added this to the 26.0 milestone Apr 18, 2023

stickies-v reviewed Apr 18, 2023

View reviewed changes

doc/dependencies.md Outdated Show resolved Hide resolved

maflcko force-pushed the 2304-py38- branch 4 times, most recently from fad6c8e to fa4d91b Compare April 18, 2023 15:27

DrahtBot mentioned this pull request Apr 18, 2023

Introduce secp256k1 module with field and group classes to test framework #26222

Merged

maflcko force-pushed the 2304-py38- branch from fa4d91b to fa1ddb5 Compare April 18, 2023 16:38

maflcko force-pushed the 2304-py38- branch from fa1ddb5 to fa96b17 Compare April 18, 2023 17:08

maflcko force-pushed the 2304-py38- branch from fa96b17 to fa92564 Compare April 18, 2023 17:15

maflcko force-pushed the 2304-py38- branch from fa92564 to fae0e9c Compare April 18, 2023 17:36

DrahtBot mentioned this pull request Apr 18, 2023

test: add python implementation of Elligator swift #24005

Merged

maflcko force-pushed the 2304-py38- branch from fae0e9c to fa33d2d Compare April 18, 2023 18:29

maflcko force-pushed the 2304-py38- branch from fa33d2d to fa1cc36 Compare April 18, 2023 18:37

DrahtBot added CI failed and removed CI failed labels Apr 19, 2023

fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Apr 21, 2023

ci: Bump ci/lint/Dockerfile

8f9e1d4

This bump should not be needed, see discussion starting at bitcoin#27483 (comment)

maflcko requested review from fjahr and stickies-v April 27, 2023 07:45

DrahtBot removed the request for review from fjahr April 27, 2023 17:36

fanquake merged commit d89aca1 into bitcoin:master Apr 28, 2023

maflcko deleted the 2304-py38- branch April 28, 2023 10:43

fjahr mentioned this pull request Apr 28, 2023

test: Remove modinv python util helper function #27538

Merged

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 28, 2023

Merge bitcoin#27483: Bump python minimum version to 3.8

6eba6b1

kwvg added a commit to kwvg/dash that referenced this pull request May 10, 2023

partial bitcoin#27483: Bump python minimum version to 3.8

c0061a3

kwvg added a commit to kwvg/dash that referenced this pull request May 10, 2023

partial bitcoin#27483: Bump python minimum version to 3.8

f559b49

kwvg added a commit to kwvg/dash that referenced this pull request May 10, 2023

partial bitcoin#27483: Bump python minimum version to 3.8

9089511

PastaPastaPasta pushed a commit to kwvg/dash that referenced this pull request May 11, 2023

partial bitcoin#27483: Bump python minimum version to 3.8

0b9500f

RandyMcMillan pushed a commit to RandyMcMillan/bitcoin that referenced this pull request May 27, 2023

ci: Bump ci/lint/Dockerfile

63f0e72

This bump should not be needed, see discussion starting at bitcoin#27483 (comment)

janus pushed a commit to BitgesellOfficial/bitgesell that referenced this pull request Sep 3, 2023

ci: Bump ci/lint/Dockerfile

32538dc

This bump should not be needed, see discussion starting at bitcoin/bitcoin#27483 (comment)

knst mentioned this pull request Oct 19, 2023

backport: bitcoin#23416,#18997,#18996, partial #27483 (TODO cleanup) dashpay/dash#5626

Merged

5 tasks

PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Oct 23, 2023

Merge pull request #5626 from knst/v20-todo-backports

dab44cd

backport: bitcoin#23416,bitcoin#18997,bitcoin#18996, partial bitcoin#27483 (TODO cleanup)

bitcoin locked and limited conversation to collaborators Apr 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Bump python minimum version to 3.8 #27483

Bump python minimum version to 3.8 #27483

Uh oh!

maflcko commented Apr 18, 2023 •

edited

Loading

Uh oh!

DrahtBot commented Apr 18, 2023 •

edited

Loading

Uh oh!

stickies-v left a comment

Uh oh!

Uh oh!

maflcko commented Apr 18, 2023 •

edited

Loading

Uh oh!

fanquake commented Apr 18, 2023

Uh oh!

maflcko commented Apr 18, 2023 •

edited

Loading

Uh oh!

RandyMcMillan commented Apr 27, 2023 •

edited

Loading

Uh oh!

fjahr commented Apr 27, 2023

Uh oh!

maflcko commented Apr 28, 2023

Uh oh!

fanquake commented Apr 28, 2023

Uh oh!

Uh oh!

Bump python minimum version to 3.8 #27483

Bump python minimum version to 3.8 #27483

Uh oh!

Conversation

maflcko commented Apr 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Apr 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

stickies-v left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maflcko commented Apr 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fanquake commented Apr 18, 2023

Uh oh!

maflcko commented Apr 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RandyMcMillan commented Apr 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fjahr commented Apr 27, 2023

Uh oh!

maflcko commented Apr 28, 2023

Uh oh!

fanquake commented Apr 28, 2023

Uh oh!

Uh oh!

maflcko commented Apr 18, 2023 •

edited

Loading

DrahtBot commented Apr 18, 2023 •

edited

Loading

maflcko commented Apr 18, 2023 •

edited

Loading

maflcko commented Apr 18, 2023 •

edited

Loading

RandyMcMillan commented Apr 27, 2023 •

edited

Loading