Bump minimum python version to 3.7

maflcko commented at 2:49 pm on October 2, 2022: member

While there is nothing that requires a bump, it may require less maintenance to drop python3.6 support. Python3.7 is available through the package manager on all currently supported operating systems.

kristapsk commented at 3:15 pm on October 2, 2022: contributor

Concept ACK, Python 3.6 had EOL 23 Dec 2021. https://endoflife.date/python

maflcko force-pushed on Oct 2, 2022

DrahtBot commented at 10:39 pm on October 2, 2022: contributor

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	hebasto, jamesob
Concept ACK	kristapsk, theStack, jarolrod, fanquake, fjahr
Stale ACK	1440000bytes, achow101

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:

#25152 (refactor: Split util/system into exception, shell, and fs-specific files by Empact)

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.

in doc/dependencies.md:13 in 7c5df6679b outdated

 9@@ -10,7 +10,7 @@ You can find installation instructions in the `build-*.md` file for your platfor
10 | [Automake](https://www.gnu.org/software/automake/) | [1.13](https://github.com/bitcoin/bitcoin/pull/18290) |
11 | [Clang](https://clang.llvm.org) | [8.0](https://github.com/bitcoin/bitcoin/pull/24164) |
12 | [GCC](https://gcc.gnu.org) | [8.1](https://github.com/bitcoin/bitcoin/pull/23060) |
13-| [Python](https://www.python.org) (tests) | [3.6](https://github.com/bitcoin/bitcoin/pull/19504) |
14+| [Python](https://www.python.org) (tests) | [3.7](https://github.com/bitcoin/bitcoin/pull/19504) |

theStack commented at 10:58 pm on October 2, 2022:

0| [Python](https://www.python.org) (tests) | [3.7](https://github.com/bitcoin/bitcoin/pull/26226) |

maflcko commented at 1:32 pm on October 3, 2022:

thx, done

theStack commented at 10:59 pm on October 2, 2022: contributor

Concept ACK

jarolrod commented at 6:19 am on October 3, 2022: member

Concept ACK modulo #26226 (review)

It would be useful to include a build system release note when dependencies like this are updated, rather than leaving it hidden among the change-log sea; but that’s aside this pr.

maflcko force-pushed on Oct 3, 2022

fanquake commented at 8:21 am on October 3, 2022: member

Concept ACK for 25.x

maflcko force-pushed on Oct 3, 2022

maflcko commented at 1:32 pm on October 3, 2022: member

Concept ACK, Python 3.6 had EOL 23 Dec 2021. https://endoflife.date/python

EOL of python shouldn’t affect us, as we only use it to run the tests. Though, operating systems that ship with it going EOL would affect us.

maflcko added the label Brainstorming on Oct 3, 2022

fjahr commented at 2:15 pm on October 3, 2022: contributor

I don’t get the argument to do this now when nothing has changed since #24017…

Sjors commented at 8:54 am on October 4, 2022: member

No strong opinions. Paging @luke-jr.

maflcko marked this as a draft on Oct 4, 2022

maflcko commented at 9:55 am on October 4, 2022: member

I don’t get the argument to do this now when nothing has changed since #24017…

Right, there is no rush, so I guess I better close this.

It just has to be done at some point in the future and I thought having a working patch couldn’t hurt.

luke-jr commented at 11:36 am on October 4, 2022: member

No strong opinions. Paging @luke-jr.

Thanks. I did take a look at this the other day. My notes are that 3.6 was for RHEL, but it appears RHEL has since moved on.

Personally, 3.6 is slightly more convenient to me since I have no easy way to get 3.7 anymore for testing (nor 3.6, but it’s already installed) - Gentoo’s oldest is 3.8. But I can adapt if need be.

DrahtBot added the label Needs rebase on Oct 4, 2022

maflcko force-pushed on Oct 5, 2022

DrahtBot removed the label Needs rebase on Oct 5, 2022

maflcko force-pushed on Oct 5, 2022

in test/lint/lint-assertions.py:15 in a4b17a8bff outdated

11@@ -12,7 +12,7 @@
12 
13 def git_grep(params: [], error_msg: ""):
14     try:
15-        output = subprocess.check_output(["git", "grep", *params], universal_newlines=True, encoding="utf8")
16+        output = subprocess.check_output(["git", "grep", *params], text=True, encoding="utf8")

jamesob commented at 10:17 am on October 5, 2022:

I think encoding is now unnecessary to specify in this line (and many others below) because text ensures stdout is a string and strings in Python 3 are by default unicode.

maflcko commented at 10:30 am on October 5, 2022:

Heh, no. This only changed in Python 3.7, see https://docs.python.org/3/library/os.html#utf8-mode

I guess text/universal_newlines is not needed when specifying encoding, so might as well remove them completely.

luke-jr commented at 3:44 pm on October 5, 2022:

Why wouldn’t they be needed? Windows still has non-standard newlines.

jamesob commented at 10:17 am on October 5, 2022: member

Concept ACK

maflcko commented at 1:22 pm on October 5, 2022: member

Closing for now until there is a more pressing reason

maflcko closed this on Oct 5, 2022

maflcko deleted the branch on Oct 5, 2022

jamesob commented at 8:49 pm on January 8, 2023: member

Can we reopen this?

There are some nice features in Python 3.7+ (like typed namedtuple features, dataclasses),
Python 3.6 is EOL, and
platforms like Debian stable ship with Python 3.9.

kristapsk commented at 8:59 pm on January 8, 2023: contributor

There are some nice features in Python 3.7+ (like typed namedtuple features, dataclasses),

Agree this could be reason to reopen and do this, if we see benefits from these features.

jamesob commented at 0:01 am on January 9, 2023: member

I think we should do anything we can to encourage people to add to and improve the functional test suite, and saddling test writers with an out of date version of python that makes it difficult to write modern idiomatic code because we’re too lazy to review a test-only PR (?) seems like the opposite direction.

maflcko commented at 12:05 pm on January 9, 2023: member

What I implemented here in the first commit is ugly, because it requires installing software-properties-common, which installs python3. Then it installs another python via the ppa, to end up with two different pythons in the CI container. It would be better to only have one.

we’re too lazy to review a test-only PR (?)

No one is holding anyone back from reviewing #26716, which decouples the python version from the python version(s) shipped by vanilla Ubuntu. ;)

I am happy to re-open this after 26716

maflcko restored the branch on Jan 17, 2023

maflcko reopened this on Jan 17, 2023

maflcko force-pushed on Jan 17, 2023

hebasto commented at 6:53 pm on January 17, 2023: member

Concept ACK.

maflcko commented at 7:05 pm on January 17, 2023: member

Rebased, but no thoughts/opinion whether this should be done, or when a good time might be.

jamesob commented at 7:47 pm on January 17, 2023: member

ACK

Why wait? What’s the risk? 3.7 unlocks some nice features: https://docs.python.org/3.7/whatsnew/3.7.html

Notable are dataclasses, some typing improvements, and that dicts now preserve order based on insertion.

I don’t understand who would be fretting about minimimum Python version… if it’s too contemporary for your local host, run the tests in a container.

fjahr commented at 7:48 pm on January 17, 2023: contributor

Concept ACK

maflcko commented at 8:07 pm on January 17, 2023: member

What’s the risk?

Mostly users on Ubuntu Bionic or Centos 8 yelling that their default python3 can’t run the tests

maflcko removed the label Brainstorming on Jan 17, 2023

DrahtBot renamed this:
~~Bump minimum python version to 3.7~~
Bump minimum python version to 3.7
on Jan 17, 2023

maflcko marked this as ready for review on Jan 17, 2023

unknown approved

unknown commented at 8:09 pm on January 17, 2023: none

ACK https://github.com/bitcoin/bitcoin/pull/26226/commits/f7459c8582dfd19a84aa8052d59f39babbe9d996

maflcko force-pushed on Jan 17, 2023

hebasto commented at 8:13 pm on January 17, 2023: member

Mostly users on Ubuntu Bionic or Centos 8 yelling that their default python3 can’t run the tests

FWIW, the current minimum version 3.6.15 is greater than Bionic’s python version which is 3.6.7.

maflcko commented at 8:16 pm on January 17, 2023: member

FWIW, the current minimum version 3.6.15 is greater than Bionic’s python version which is 3.6.7.

3.6 is still the minimum version on master (minor non-breaking releases don’t matter). And we check that on master by running the tests on Bionic.

(The same will remain true if this pull is merged: 3.7 is the minimum supported, and any minor non-breaking release can be used)

achow101 commented at 8:32 pm on January 17, 2023: member

Mostly users on Ubuntu Bionic or Centos 8 yelling that their default python3 can’t run the tests

Both of those also EOL this year.

maflcko force-pushed on Jan 17, 2023

jamesob commented at 8:36 pm on January 17, 2023: member

I hereby volunteer to provide the code necessary to generate a container sufficient to run the functional tests for anyone both (i) yelling and (ii) running an ancient version of Python. :)

maflcko commented at 8:38 pm on January 17, 2023: member

Right, Ubuntu Bionic will be EOL before the next release anyway. Centos Stream 8 runs until May 31st, 2024 , but they can install python38, I guess :man_shrugging:

maflcko commented at 8:39 pm on January 17, 2023: member

(Force pushed minimum to 3.7.16 and “maximum” to 3.12)

maflcko added this to the milestone 25.0 on Jan 17, 2023

achow101 commented at 9:14 pm on January 17, 2023: member

Since #26716 changes our CI to use pyenv’s python, I think the changes to the container and installed packages in fa33242a1ab340aad177021f6f12073081615f86 end up being irrelevant?

maflcko commented at 9:17 pm on January 17, 2023: member

The ci/lint system is separate from the ci/test system, so you should be able to get a test failure by reverting the image or packages in ci/test

achow101 commented at 0:25 am on January 18, 2023: member

ACK fa1dbe04b2580a7d426be1729a3c78c61964075e

Bump minimum python version to 3.7 dddd462137

Revert "contrib: Fix capture_output in getcoins.py"

This reverts commit be59bd17ec753af7cc763474f2432d12bfc88c2f
because the changes are no longer needed.

fa2a23548a

scripted-diff: Use new python 3.7 keywords

-BEGIN VERIFY SCRIPT-
 sed -i 's/universal_newlines/text/g' $(git grep -l universal_newlines)
-END VERIFY SCRIPT-

fa8fe5b696

in ci/test/04_install.sh:96 in fa1dbe04b2 outdated

91@@ -92,6 +92,9 @@ elif [ "$CI_USE_APT_INSTALL" != "no" ]; then
92     # TODO: drop this once we can use newer images in GCE
93     CI_EXEC_ROOT add-apt-repository ppa:hadret/bpfcc
94   fi
95+  if [[ -n "${APPEND_APT_SOURCES_LIST}" ]]; then
96+    CI_EXEC echo "${APPEND_APT_SOURCES_LIST}" >> /etc/apt/sources.list

hebasto commented at 9:47 am on January 18, 2023:

fa33242a1ab340aad177021f6f12073081615f86:

0    CI_EXEC_ROOT echo "${APPEND_APT_SOURCES_LIST}" >> /etc/apt/sources.list

maflcko force-pushed on Jan 18, 2023

hebasto approved

hebasto commented at 1:38 pm on January 18, 2023: member

ACK fa8fe5b69669b4d86e0d0970d68502abee8785f3

jamesob commented at 3:05 pm on January 18, 2023: member

ACK https://github.com/bitcoin/bitcoin/pull/26226/commits/fa8fe5b69669b4d86e0d0970d68502abee8785f3

Looks ready to go to me!

jamesob commented at 3:11 pm on January 18, 2023: member

As a follow-up I’m betting we can drop all the changes that use pyenv to build Python from source, since Python 3.7 comes with buster by default?

maflcko commented at 3:16 pm on January 18, 2023: member

No, we also need git2.34 or later, which is only shipped in Ubuntu Jammy

maflcko merged this on Jan 18, 2023

maflcko closed this on Jan 18, 2023

maflcko deleted the branch on Jan 18, 2023

fanquake referenced this in commit 902b3b1987 on Jan 18, 2023

sidhujag referenced this in commit 2a26193545 on Jan 19, 2023

willcl-ark commented at 2:20 pm on January 20, 2023: contributor

Post merge ACK.

FWIW my version of pyenv did not contain 3.7.16 in the list of versions available to install. Easily fixed though by running pyenv update to update both pyenv itself, and also the list of available versions for install.

RandyMcMillan commented at 6:14 pm on February 20, 2023: contributor

#27130 (comment)

To maintain consistency across macOS x86 and Arm64 maybe the minimum python3 version should be python@3.8

ref: #26226

kwvg referenced this in commit 7a02498195 on May 10, 2023

kwvg referenced this in commit e97ea86e49 on May 10, 2023

PastaPastaPasta referenced this in commit 7bd22afacb on May 11, 2023

bitcoin locked this on Feb 20, 2024

Bump minimum python version to 3.7 #26226

Reviews

Conflicts