spesmilo / electrum

Electrum Bitcoin Wallet
https://electrum.org
MIT License
7.4k stars 3.08k forks source link

bump required python version to 3.10 #9239

Open SomberNight opened 1 week ago

SomberNight commented 1 week ago

I would like to bump the min version of python we support from 3.8 to something higher, at least 3.9, but ideally even more, to e.g 3.10.

Unlike last time when we bumped to 3.8, this time there is no pressing/acute reason, mostly just maintainability concerns.

That last bump was 3 years ago, and since then, a new "major" release came out every year. 3.13 just got released yesterday, and btw 3.8 got end-of-lifed at the same time. I haven't checked if current master works on 3.13, but if not, that is of course to be considered a bug in our code that we should fix. So, right now, in theory we support 3.8-3.13 -- that is 6 versions! The CI runs the unit tests for each.

Ubuntu 22 LTS ships 3.10 Ubuntu 24 LTS ships 3.12 debian 12 (current stable) ships 3.11

The binaries we build (on master) use 3.10 for Android, and 3.11 for the rest.

Thoughts about bumping the minimum to 3.10? I could even be persuaded to bump to 3.11, but then Android binaries would need fixing (p4a rebase probably).

accumulator commented 1 week ago

The AppImage is based on Debian Buster, which is intentionally older for glibc reasons, so it runs on older distros. Debian Buster ships python 3.7 so we're already violating our own lower version bound.

If we upgrade this to bullseye, this would put the lower version bound to python 3.9

Alternatively, we could include glibc and other low level system libraries in the AppImage to allow running on older distros, but this is not trivial (AFAIK).

SomberNight commented 6 days ago

We build our own CPython from source as part of the AppImage build: https://github.com/spesmilo/electrum/blob/6006971ab7cfaa84defb17a3bdd6288445278ac3/contrib/build-linux/appimage/make_appimage.sh#L22

The AppImage is based on Debian Buster, which is intentionally older for glibc reasons, so it runs on older distros. Debian Buster ships python 3.7 so we're already violating our own lower version bound.

(in the context of the AppImage) It does not matter what version of python debian packages in apt, as we build our own python and bundle it in the appimage. This is in contrast with libc, because that is not getting bundled in the appimage, instead it is expected to be provided by the host system running the appimage.

(see https://github.com/spesmilo/electrum/blob/6006971ab7cfaa84defb17a3bdd6288445278ac3/contrib/build-linux/appimage/make_appimage.sh#L199 https://github.com/AppImageCommunity/pkg2appimage/blob/a9c85b7e61a3a883f4a35c41c5decb5af88b6b5d/functions.sh#L121 https://github.com/AppImageCommunity/pkg2appimage/blob/a9c85b7e61a3a883f4a35c41c5decb5af88b6b5d/excludelist#L14 ) (related https://github.com/spesmilo/electrum/issues/7997)

Does that clear up the potential confusion, or am I missing something? :)

SomberNight commented 6 days ago

The motivation for building the appimage on an old distro is so that users of older distros can still run the appimage. Whether users of these old distros are able to run Electrum from source using the distro-provided python is orthogonal.

In fact historically, we originally started distributing appimages when we bumped the min required version of python to 3.6 (which at the time was newer than what debian stable shipped). The main motivation was to provide an alternative way for users of distros with old python to still run Electrum easily. see https://github.com/spesmilo/electrum-docs/blob/6cdb612d8086aae2d93b47e06886913f1971a026/faq.rst#electrum-requires-recent-python-my-linux-distribution-does-not-yet-have-it-what-now

accumulator commented 6 days ago

Does that clear up the potential confusion, or am I missing something? :)

No, it was just me being stupid :) I kinda knew we build our own python, but I had the version pinning for reproducibility in my head.

ecdsa commented 6 days ago

The code in plugin.py now branches in two places depending on the python version, bumping to 3.10 would allow us to simplify that. Besides, the aionostr package fails our tests with python 3.8, but they pass with 3.9. (its setup.py has python_requires >= 3.8)