Don’t fix Python patch version #33051

pull Sjors wants to merge 2 commits into bitcoin:master from Sjors:2025/07/pyenv changing 4 files +15 −12
  1. Sjors commented at 12:18 pm on July 24, 2025: member

    .python-version always matches the minimum supported Python version. It’s main purpose is to catch accidental use of too modern syntax in scripts and functional tests.

    We (currently) don’t specify a minimum patch version, so it’s not necessary to do so here. The minor verion is enough.

    This also avoids requiring users to keep a potentially unsafe old patch version installed.

    The linter CI job used python_build (part of PyEnv) which requires specifying an exact Python version. The first commit changes it to use the higher level pyenv install which will pick the most recent patch version it knows about.

    The original stated reason for using the lower level python_build #26716 (comment) was that it allowed specifying CC=clang which needs fewer resources, but pyenv install can do that as well.

    See also https://github.com/bitcoin-core/HWI/pull/695

  2. DrahtBot commented at 12:18 pm on July 24, 2025: contributor

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

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33051.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Stale ACK maflcko

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

    Conflicts

    No conflicts as of last run.

  3. maflcko commented at 12:26 pm on July 24, 2025: member
    the lint CI failed
  4. fanquake commented at 2:15 pm on July 24, 2025: member

    https://cirrus-ci.com/task/4540580631412736?logs=build#L1735:

    0[12:20:39.690] 38.29 ++ cat /.python-version
    1[12:20:39.690] 38.29 + env CC=clang python-build 3.10 /python_build
    2[12:20:39.690] 38.31 python-build: definition not found: 3.10
    3[12:20:39.690] ------
    
  5. Sjors commented at 3:01 pm on July 24, 2025: member
    Ah I see. For some reason the linter is calling python-build directly instead of pyenv install, which just picks a patch version. Added a workaround.
  6. Sjors force-pushed on Jul 24, 2025
  7. fanquake commented at 3:09 pm on July 24, 2025: member

    E.g. if the user installed Python 3.10.14 and 3.10.16 through PyEnv, if HWI was locked to 3.10 it would default to 3.10.16. HWI then can’t find any of its dependencies.

    Then the user should reinstall the HWI dependencies, given they’ve changed Python version. They would have the same issue any time they update their Python, to a newer 3.10.x version.

    Regardless, can you solve your issue using something out of https://github.com/pyenv/pyenv?tab=readme-ov-file#understanding-python-version-selection. Us having to change our Python version selection, to accomodate the Python version selection of a different project, would seem to undermine the point of both projects using Python version management.

  8. Sjors commented at 3:16 pm on July 24, 2025: member

    It’s not really a problem for me, but I noticed HWI stopped using patch versions in https://github.com/bitcoin-core/HWI/pull/695 and I don’t think there’s a good reason for us to insist on one.

    Everywhere else in the project we only specify the Python minor version. And if there’s ever a security issue with the patch version we picked, we probably should go and update it (which we’ve never done).

  9. fanquake commented at 3:18 pm on July 24, 2025: member

    and I don’t think there’s a good reason for us to insist on one.

    Well at least one is that we don’t need to add more code/“workarounds”, for a problem we don’t have. (https://github.com/bitcoin/bitcoin/pull/33051#issuecomment-3113814862)

  10. Sjors commented at 3:19 pm on July 24, 2025: member
    But why are we cloning the PyEnv repo, but then not just using pyenv install 3.10? Then the workaround wouldn’t be needed.
  11. fanquake commented at 3:30 pm on July 24, 2025: member

    but then not just using pyenv install

    pyenv install is a wrapper around python-build. Looks like it was used to use slightly less resources.

  12. Sjors commented at 4:46 pm on July 24, 2025: member

    It’s defined here: https://github.com/pyenv/pyenv/blob/master/plugins/python-build/bin/pyenv-install

    Which internally calls pyenv-latest --known: https://github.com/pyenv/pyenv/blob/master/libexec/pyenv-latest

    Which it turn calls python-build --definitions as I did.

  13. maflcko commented at 9:17 am on July 25, 2025: member

    lgtm ACK e71990fabc53c53d1524ea00851e0d9a67a8f86f

    CI: https://cirrus-ci.com/task/5023354685489152?logs=build#L1722

     0[15:02:47.597] [#10](/bitcoin-bitcoin/10/) 36.72 ++ cat /.python-version
     1[15:02:47.813] [#10](/bitcoin-bitcoin/10/) 36.72 + PYTHON_VERSION=3.10
     2[15:02:47.813] [#10](/bitcoin-bitcoin/10/) 36.72 ++ python-build --definitions
     3[15:02:47.813] [#10](/bitcoin-bitcoin/10/) 36.72 ++ grep '^3.10'
     4[15:02:47.813] [#10](/bitcoin-bitcoin/10/) 36.72 ++ tail -1
     5[15:02:47.813] [#10](/bitcoin-bitcoin/10/) 36.73 + PYTHON_PATCH_VERSION=3.10.18
     6[15:02:47.813] [#10](/bitcoin-bitcoin/10/) 36.73 + env CC=clang python-build 3.10.18 /python_build
     7[15:02:47.813] [#10](/bitcoin-bitcoin/10/) 36.78 Downloading Python-3.10.18.tar.xz...
     8[15:02:47.813] [#10](/bitcoin-bitcoin/10/) 36.78 -> https://www.python.org/ftp/python/3.10.18/Python-3.10.18.tar.xz
     9[15:02:49.055] [#10](/bitcoin-bitcoin/10/) 38.17 Installing Python-3.10.18...
    10[15:04:10.084] [#10](/bitcoin-bitcoin/10/) 119.2 Installed Python-3.10.18 to /python_build
    11[15:04:10.275] [#10](/bitcoin-bitcoin/10/) 119.4 + export PATH=/python_build/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
    12[15:04:10.275] [#10](/bitcoin-bitcoin/10/) 119.4 + PATH=/python_build/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
    13[15:04:10.275] [#10](/bitcoin-bitcoin/10/) 119.4 + command -v python3
    14[15:04:10.275] [#10](/bitcoin-bitcoin/10/) 119.4 + python3 --version
    15[15:04:10.427] [#10](/bitcoin-bitcoin/10/) 119.4 /python_build/bin/python3
    16[15:04:10.427] [#10](/bitcoin-bitcoin/10/) 119.4 Python 3.10.18
    

    In theory this could increase CI non-determinism, but we are using Ubuntu as base image, so that goal is already lost.

  14. fanquake commented at 9:20 am on July 25, 2025: member
    The PR description and commit message will need re-writing, to explain the motivation, as the current motivaiton is just user error (updating python and not reinstalling dependencies).
  15. fanquake commented at 9:21 am on July 25, 2025: member

    But why are we cloning the PyEnv repo, but then not just using pyenv install 3.10? Then the workaround wouldn’t be needed.

    If we are going to change anything here, then we might as well swap to this. Given the original motivation is unclear, and it’d be less lines of code.

  16. Sjors commented at 11:44 am on July 25, 2025: member

    Original discussion: #26716 (comment)

    I reckon the lower memory usage is due to using clang instead of gcc.

    pyenv install also honors CC=clang (tested by setting CC=fkfkkf which fails)

    it’d be less lines of code.

    It removes a few lines for the installer, but adds some for pyenv global and pyenv init.

    Using PyEnv directly does make it easier to use multiple Python versions, in case any of our linters would benefit from a more recent version.

    I updated the commit message and PR description.

  17. Sjors force-pushed on Jul 25, 2025
  18. ci: install Python using pyenv for linter
    Using `pyenv install` avoids the need to specify a patch version,
    which will be dropped in the next commit.
    
    The original reason for using the lower level `python_build`, which
    is part of PyEnv, was that it allowed specifying CC=clang, which
    needs fewer resources. But `pyenv install` supports that too.
    b9a46ce26c
  19. Don't pin Python patch version
    .python-version always matches the minimum supported Python version.
    It's main purpose is to catch accidental use of too modern syntax
    in scripts and functional tests.
    
    We (currently) don't specify a minimum patch version, so it's not
    necessary to do so here. The minor verion is enough.
    
    This also avoids requiring users to keep a potentially unsafe old
    patch version installed.
    43b4d002e1
  20. Sjors force-pushed on Jul 25, 2025
  21. Sjors commented at 11:55 am on July 25, 2025: member
    (added rationale to the first commit message)

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-08-31 06:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me