lint: modernise lint tooling #34547

pull willcl-ark wants to merge 6 commits into bitcoin:master from willcl-ark:modernise-linter changing 10 files +90 −86
  1. willcl-ark commented at 10:25 am on February 10, 2026: member

    Modernise our lint tooling by:

    - Replacing pyenv + pip with uv for better Python environment and dependency management - Replacing mypy with https://github.com/astral-sh/ty - Move uv ruff and ty to install via COPY --from multi-stage Docker image imports - Moving ruff lint rules from hardcoded Rust array (in lint_py.rs) into a top-level ruff.toml, and add ty.toml for the type checker - Extracting all remaining pip dependencies into dedicated ci/lint/requirements.txt

    Extra rationale:

    COPY --from pulls pre-built binaries from upstream images instead of compiling/downloading at runtime. Containerfile layer optimisations reduce rebuild frequency further.

    ty is significantly faster/more modern/maintained than mypy, and configured declaratively.

    Adding root-level [ty|ruff].toml config files means contributors can easily run ty check or ruff check locally without running the full linter, along with being accessible to other tooling (similarly for requriements.txt).

    Pinning tool versions in the dockerfile makes it more excplicit and easier to find.

    The tradeoff we make here is that there is no longer a single install script to install tooling on a local machine. However I think this is OK, as it currently only works for apt-based OSes anyway, and I don’t think running the linter outside of the container is such a valuable use-case as it is with some of the other CI jobs.

    Further work can drop individual rules from ty.toml fixing up the infringing code as necessary. Whilst we have ignore rules in ty.toml then certain things may go undetected (as they are ignored), and this may be a temporary regression against master using mypy, until the infracting code is fixed and the rule can be un-ignored.

  2. DrahtBot added the label Tests on Feb 10, 2026
  3. DrahtBot commented at 10:25 am on February 10, 2026: 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 maflcko, stickies-v
    Concept ACK fanquake

    If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

    Conflicts

    No conflicts as of last run.

  4. fanquake commented at 10:39 am on February 10, 2026: member

    Tried running this locally, but it currently fails on arm64, because there is no arm64 variant for becheran/mlc:

    0Error: choosing an image from manifest list docker://becheran/mlc:latest: no image found in image index for architecture "arm64", variant "v8", OS "linux"
    
  5. willcl-ark marked this as a draft on Feb 10, 2026
  6. in ci/lint_imagefile:15 in b63444e4d1 outdated
    11+ENV LC_ALL=C
    12+ENV PATH="/python_env/bin:${PATH}"
    13+ENV VIRTUAL_ENV="/python_env"
    14+
    15+COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/
    16+COPY --from=ghcr.io/astral-sh/ruff:0.14.4 /ruff /bin/
    


    maflcko commented at 10:58 am on February 10, 2026:
    Is there any value in having ruff separate? It would appear more consistent to install it next to the other ci/lint/requirements.txt (lief, etc)

    willcl-ark commented at 12:16 pm on February 10, 2026:

    The main advantage is that the COPY --from instructions are standalone layers and are not invalidated when things below change (they are just “replayed” over the top). The more tools we can install from them, the better (usually).

    No strong opinion here, as the end result is the same. But IMO it’s better practice to install from layers like I have it.


    maflcko commented at 12:53 pm on February 10, 2026:

    Sure, makes sense to use COPY –from to keep the layers in the cache.

    I guess the main question is whether we want to force all devs to use the docker image. I think it should be fine for devs to pick pip and then install ci/lint/requirements.txt and get all the stuff they want. But no strong opinion, I rely on the GHA-ci for the lint anyway.

  7. in ci/lint_imagefile:29 in b63444e4d1 outdated
    29+  uv python install && \
    30   echo 'alias lint="./ci/lint/06_script.sh"' >> ~/.bashrc && \
    31-  chmod 755 /entrypoint.sh && \
    32   rm -rf /var/lib/apt/lists/*
    33 
    34+COPY ./ci/lint/container-entrypoint.sh /entrypoint.sh
    


    maflcko commented at 10:59 am on February 10, 2026:
    This seems to conflict with #34427. Maybe that should be merged/closed first?

    willcl-ark commented at 12:59 pm on February 10, 2026:
    Agree. I’ve acked that PR already, and would be happy to rebase this on it (or see it closed) – no strong opinion either way really.
  8. willcl-ark force-pushed on Feb 10, 2026
  9. willcl-ark commented at 12:17 pm on February 10, 2026: member

    Tried running this locally, but it currently fails on arm64, because there is no arm64 variant for becheran/mlc:

    0Error: choosing an image from manifest list docker://becheran/mlc:latest: no image found in image index for architecture "arm64", variant "v8", OS "linux"
    

    This should be fixed in the latest push. We currently (and still here) use an x86_64 MLC binary, but this works in an arm64 container (e.g on Mac) because the Rosetta emulation is passed in/used inside the container.

    I keep this behaviour for now, but ideally MLC woudl start publishing multi-platform docker images.

  10. willcl-ark marked this as ready for review on Feb 10, 2026
  11. DrahtBot added the label Needs rebase on Feb 12, 2026
  12. willcl-ark force-pushed on Feb 12, 2026
  13. DrahtBot removed the label Needs rebase on Feb 12, 2026
  14. DrahtBot added the label Needs rebase on Mar 10, 2026
  15. willcl-ark force-pushed on Mar 12, 2026
  16. DrahtBot removed the label Needs rebase on Mar 12, 2026
  17. willcl-ark force-pushed on Mar 12, 2026
  18. willcl-ark force-pushed on Mar 12, 2026
  19. DrahtBot added the label CI failed on Mar 12, 2026
  20. willcl-ark commented at 11:59 am on March 12, 2026: member
    bumped to latest ruff (0.15.5) and ty (0.0.21) and added newly-needed rule ignores.
  21. DrahtBot removed the label CI failed on Mar 12, 2026
  22. in ci/lint/01_install.sh:33 in 10332d0129 outdated
    44+export PATH="/python_env/bin:${PATH}"
    45 command -v python3
    46 python3 --version
    47 
    48-${CI_RETRY_EXE} pip3 install \
    49+uv pip install --python /python_env \
    


    maflcko commented at 3:32 pm on March 13, 2026:
    nit in 10332d01290366c4e6f0b29e5e94aabe744452cc: Why drop the retry? Is this something that happens in uv? Why not just leave this line as-is?

    willcl-ark commented at 8:01 pm on March 13, 2026:
  23. in ci/lint_imagefile:11 in 87c0592779 outdated
     7@@ -8,6 +8,7 @@ FROM mirror.gcr.io/ubuntu:24.04
     8 
     9 COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/
    10 COPY --from=ghcr.io/astral-sh/ruff:0.15.5 /ruff /bin/
    11+COPY --from=docker.io/koalaman/shellcheck:v0.11.0 /bin/shellcheck /usr/bin/
    


    maflcko commented at 3:37 pm on March 13, 2026:

    nit: IIRC docker.io doesn’t support ipv6? Also, they are heavily rate-limiting some ipv4 (or even blocking them). This was the reason to use gcr.

    My preference would be to just leave this as-is :sweat_smile:


    willcl-ark commented at 8:02 pm on March 13, 2026:

    Oh yeah :'(

    Will fix that up shortly then.

  24. in ruff.toml:3 in 73eae6f76e outdated
    0@@ -0,0 +1,42 @@
    1+[lint]
    2+select = [
    3+    "B006", # mutable-argument-default
    


    maflcko commented at 3:38 pm on March 13, 2026:
    nit in 73eae6f76e5c008ca1933dd2edfb592796940289: Would be nice to squash this with a4200d48bf6e591e59a6eba4c5771251956b2409. Otherwise, it is harder to review.

    willcl-ark commented at 8:13 pm on March 13, 2026:
    Done in 7d936ac2111
  25. maflcko commented at 3:39 pm on March 13, 2026: member

    left some comments, but feel free to ignore them

    c7e4a67086626b8832372cf461cee30c22c62ea0~8 🥝

    Signature:

    0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
    1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
    2trusted comment: c7e4a67086626b8832372cf461cee30c22c62ea0~8 🥝
    3IVhYiz19o3yQe68zl8z74QmB+pkeGtEnNKMDk40qNOxOuO20cBwlEWe2l+juySUkL1C+N5ejQa0svHEqvOa2BQ==
    
  26. willcl-ark force-pushed on Mar 13, 2026
  27. in ci/lint_imagefile:11 in e613229f1a outdated
     7@@ -8,6 +8,7 @@ FROM mirror.gcr.io/ubuntu:24.04
     8 
     9 COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/
    10 COPY --from=ghcr.io/astral-sh/ruff:0.15.5 /ruff /bin/
    11+COPY --from=ghcr.io/astral-sh/ty:0.0.21 /ty /bin/ 8399357d746 (lint: install ty typechecker from docker and use)
    


    maflcko commented at 7:52 am on March 14, 2026:

    Nit in e613229f1a3734f8a8a12211415cc3b68471df35: :eyes:

    Also, could squash 5b756860bd7d84665cdfa55873db410e90532158 into this one?


    willcl-ark commented at 10:51 am on March 14, 2026:
    squashed in 28945207085
  28. in ci/lint_imagefile:12 in 0b7919a504 outdated
     5@@ -6,15 +6,32 @@
     6 
     7 FROM mirror.gcr.io/ubuntu:24.04
     8 
     9+ENV LC_ALL=C
    10+ENV PATH="/python_env/bin:${PATH}"
    11+ENV VIRTUAL_ENV="/python_env"
    12+ENV MLC_VERSION=v1
    


    maflcko commented at 7:56 am on March 14, 2026:
    0b7919a5044836b34df02cfc11cd3b951b904132: should be 1.2?

    maflcko commented at 8:03 am on March 14, 2026:

    Also, you are removing the comments as to which apt package is needed for what? It may be good to keep them. I think this is possible via syntax like:

    0$ echo apt install bla `# bla` \
    1>  foo `# foo` \
    2>  bar `# bar`
    

    (Haven’t tried this in docker)


    maflcko commented at 8:04 am on March 14, 2026:

    Also, the comment about the python version file is gone.

    If this is not possible to put the comments in the dockerfile, my preference would be to keep the shell script as a snippet.


    willcl-ark commented at 10:32 am on March 14, 2026:
    It seems its not possible to use multi-line and comments, so will revert to shell script
  29. in test/lint/lint-format.py:96 in 2464142c1d outdated
    91+        print(
    92+            "These files were ruff-formatted on the base branch but are no longer formatted:"
    93+        )
    94+        for f in failures:
    95+            print(f"  {f}")
    96+        print("\nRun 'ruff format' on these files to fix.")
    


    maflcko commented at 8:10 am on March 14, 2026:

    I think it is one thing to use ruff in the lint CI, but possibly requiring it for all devs (is this a requirement?) seems spicy. What happened to yapf, yapf-diff, and black? How do they interact?

    Maybe a separate pull for this commit?


    willcl-ark commented at 10:49 am on March 14, 2026:

    Agreed, I’ll drop this commit from this PR (but I would like it still eventually)

    I don’t think requiring ruff to format python code is a high bar. As you mention, we have previously “required” yapf, and do require things like clang-format.

    The idea behind this commit is that the “bar” for ruff usage starts extremely low:

    • if the file is not (yet) formatted, it does not need to be formatted in the PR.
    • if the file is formatted, then you cannot break the formatting in your PR.

    Only once we start formatting files (which we should) does ruff start to become required to avoid regressions.

    What happened to yapf, yapf-diff, and black? How do they interact?

    IIUC yapf/[-diff] and black is not enforced anywhere, so I don’t think anything changes with them practically 😄

    The idea behind the “astral stack” (uv, ruff, ty) is that they take over all of those roles (and more) much more comprehensively. Regarding formatting ruff format is 100% black-compatible formatting, and inherits also it’s “non-configurable” ethos (well, you can configure some parts e.g. line-length, but not much).

    Ruff can’t format-diff, but we won’t need that with this commit; you either format the entire file, or nothing at all. No diff-wise formatting introduced piecemeal; it’s messy, error-prone etc..

    IMO the ideal scenario for us to reach is that you can write code and either configure your editor to save+format, use a format bind in your editor, a pre-commit hook, or after you’re done editing just run:

    0ruff format <file>
    1# or eventually
    2ruff format
    

    and it’s all done. No thinking, nop opinions, no drama :)

    My intuition of how to phase it in is:

    • format less-often-touched files all in one go, taking a bit of an up-front hit
    • try to encourage contributors to add a format commit to their PR if they touch a python file and go one-at-a-time until we approach 100% formatting

    maflcko commented at 7:04 am on March 16, 2026:

    I don’t think requiring ruff to format python code is a high bar. As you mention, we have previously “required” yapf, and do require things like clang-format.

    I don’t think we do “require” them. Just now there was a clang-format unclean diff merged:

    0diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
    1index 3724c3f2b3..333c733ce5 100644
    2--- a/src/wallet/wallet.cpp
    3+++ b/src/wallet/wallet.cpp
    4@@ -586 +586 @@ static bool EncryptMasterKey(const SecureString& wallet_passphrase, const CKeyin
    5-    for (int i = 0; i < 2; i++){
    6+    for (int i = 0; i < 2; i++) {
    

    Regarding formatting ruff format is 100% black-compatible formatting, and inherits also it’s “non-configurable” ethos (well, you can configure some parts e.g. line-length, but not much).

    Ok, that sounds fair. So devs are free to use either black or ruff, and aren’t forced to use a specific one. There could still be the issue where the auto-format doesn’t look nice and a dev prefers to manually format, but maybe this is an acceptable price to pay. (I don’t mind, but it could make sense to ask other devs as well)

    Also, I think this script should print the diff that is expected. So that devs who want to refuse to use any tool at all can just copy-paste the diff from CI (like the iwyu diff).

    Finally, the commit should probably remove .style.yapf, because yapf is not 100% compatible with black, so I guess it won’t be compatible with ruff either.


    willcl-ark commented at 9:55 am on March 16, 2026:

    Thanks, will bear these in mind if I re-open the lint-format.py in a future followup :)

    So that devs who want to refuse to use any tool at all can just copy-paste the diff from CI (like the iwyu diff)

    Sure I guess we can accommodate that 🥲

    There could still be the issue where the auto-format doesn’t look nice and a dev prefers to manually format…

    this commit shows (partially) how to disable formatting on sections where manual layour is preferred, basically as simple as:

    0# fmt: off
    1<unformatted code>
    2# fmt: on 
    

    Those were pretty much the only places I (personally) thought it would be beneficial when I looked 9 months ago; looks like 12 sites to me.

  30. maflcko commented at 8:11 am on March 14, 2026: member

    seems fine, but I left some comments about the comments

    2464142c1d31db482bd6a24fc3d9af14c8519847~1 🏉

    Signature:

    0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
    1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
    2trusted comment: 2464142c1d31db482bd6a24fc3d9af14c8519847~1 🏉
    3YM0UGboaJZYuNZ4u9P2+S/U0wzmQDkKBRpY/jdBJQthxEqAsYg7gntYun4QLbrOhHuDVA1gccnYaM+Uptq9CBQ==
    
  31. willcl-ark force-pushed on Mar 14, 2026
  32. willcl-ark force-pushed on Mar 14, 2026
  33. DrahtBot added the label CI failed on Mar 14, 2026
  34. DrahtBot removed the label CI failed on Mar 14, 2026
  35. DrahtBot added the label Needs rebase on Mar 16, 2026
  36. in ci/lint/01_install.sh:33 in cc0cb5b202 outdated
    29@@ -30,10 +30,7 @@ export PATH="/python_env/bin:${PATH}"
    30 command -v python3
    31 python3 --version
    32 
    33-uv pip install --python /python_env \
    34-  lief==0.16.6 \
    35-  pyzmq==27.1.0 \
    36-  vulture==2.14
    37+uv pip install --python /python_env -r /ci/lint/requirements.txt
    


    maflcko commented at 6:54 am on March 16, 2026:
    nano nit in cc0cb5b202d523e19f0f8fbdcd7b594e6faa887a: In scripts I prefer the long option, for better self-documentation: -r -> --requirement

    willcl-ark commented at 9:55 am on March 16, 2026:
    taken in 4ab42247c87
  37. maflcko approved
  38. maflcko commented at 7:09 am on March 16, 2026: member

    Looks like a nice change to reduce some complexity:

    • Use uv over manual python installation (less stuff to maintain for us)
    • This also allows to drop the patch version (less stuff to maintain for us)
    • It moves some stuff, which all beneficial (using standard files like lint/requirements.txt or ruff.toml)

    I have not reviewed the ty change or the ty.toml, but it looks harmless.

  39. in test/lint/README.md:60 in 94676f96e1
    57 | [`lint-shell.py`](/test/lint/lint-shell.py) | [ShellCheck](https://github.com/koalaman/shellcheck)
    58 | `py_lint` | [ruff](https://github.com/astral-sh/ruff)
    59 | markdown link check | [mlc](https://github.com/becheran/mlc)
    60 
    61-In use versions and install instructions are available in the [CI setup](../../ci/lint/01_install.sh).
    62+In use versions and install instructions are available in the [CI setup](../../ci/lint/01_install.sh) and the [lint_imagefile](../../ci/lint_imagefile) (for tools where an OCI imagefile exists).
    


    maflcko commented at 7:10 am on March 16, 2026:
    llm nit: [“In use versions” is awkward; “Versions in use” (or “In-use versions”) is clearer and avoids misreading.]

    willcl-ark commented at 9:55 am on March 16, 2026:
    Tried to clarify more in 720fa75f0c4
  40. willcl-ark force-pushed on Mar 16, 2026
  41. maflcko commented at 9:48 am on March 16, 2026: member

    Wrong dropped hunk in rebase:

     03:  2894520708 ! 3:  b831bc9f08 lint: switch to ty for python typechecking
     1    @@ Commit message
     2         ty is faster and more maintained than mypy. Comes with better editor
     3         integrations, a language server and is written in rust (tm).
     4     
     5    - ## ci/lint/01_install.sh ##
     6    -@@ ci/lint/01_install.sh: python3 --version
     7    - 
     8    - uv pip install --python /python_env \
     9    -   lief==0.16.6 \
    10    --  mypy==1.19.1 \
    11    -   pyzmq==27.1.0 \
    12    -   vulture==2.14
    13    - 
    14    -
    

    re-review ACK 720fa75f0c4ea39aeffe52c23fee121d2316fe21 🛑

    Signature:

    0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
    1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
    2trusted comment: re-review ACK 720fa75f0c4ea39aeffe52c23fee121d2316fe21 🛑
    36AAWiWc6jsszVbCY/bf3qZsTvNaLWMvqNSo/wSVbptKhyyEp8ke7Ji8B605qfBjvF1Om79MWFCEit1jKxq39DQ==
    
  42. willcl-ark force-pushed on Mar 16, 2026
  43. DrahtBot added the label CI failed on Mar 16, 2026
  44. willcl-ark force-pushed on Mar 16, 2026
  45. willcl-ark commented at 10:01 am on March 16, 2026: member

    Wrong dropped hunk in rebase:

    Whoops, fixed now

  46. maflcko commented at 10:10 am on March 16, 2026: member

    re-ACK a7bcd34c172a8d91f3f2118c6e43b6672113b652 📟

    Signature:

    0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
    1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
    2trusted comment: re-ACK a7bcd34c172a8d91f3f2118c6e43b6672113b652  📟
    3ebip6gE7EEveMNp94Tpa7FjH8NdH8dSLiEIXgfn987tFiwlhfUGMDPE9Wu8olVojfD2PaTtf0nAyOZ4jWHsFBQ==
    
  47. DrahtBot removed the label CI failed on Mar 16, 2026
  48. fanquake commented at 10:45 am on March 16, 2026: member
    Concept ACK
  49. DrahtBot removed the label Needs rebase on Mar 16, 2026
  50. maflcko commented at 11:43 am on March 16, 2026: member
    cc @Sjors (as this includes your commit)
  51. fanquake referenced this in commit dc104cc333 on Mar 17, 2026
  52. Sjors commented at 3:34 pm on March 19, 2026: member

    27f89f92a11f7f37a2f650cd7a90e358ea158ae4 lgtm

    I haven’t used uv myself, but local development with PyEnv is not impacted by the CI changes in this PR.

  53. DrahtBot added the label Needs rebase on Mar 20, 2026
  54. willcl-ark force-pushed on Mar 20, 2026
  55. maflcko commented at 8:55 am on March 20, 2026: member

    re-lgtm, with my initial review comment: #34547#pullrequestreview-3951783614

    re-ACK b497401dd5ee1d84dbaa9217e9ebb983d590067b 🖌

    Signature:

    0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
    1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
    2trusted comment: re-ACK b497401dd5ee1d84dbaa9217e9ebb983d590067b 🖌
    3UAIzmRiMXOsI88wMwAzZstJBqd1l8a8yLuoZomV/Oai1H9sYRzCh8vN4r+KXJW3i6e5n5woFW3iM7NsQhj05Bg==
    
  56. DrahtBot requested review from fanquake on Mar 20, 2026
  57. DrahtBot removed the label Needs rebase on Mar 20, 2026
  58. in ci/lint_imagefile:12 in b497401dd5
     5@@ -6,8 +6,13 @@
     6 
     7 FROM mirror.gcr.io/ubuntu:24.04
     8 
     9+COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/
    10+COPY --from=ghcr.io/astral-sh/ruff:0.15.5 /ruff /bin/
    11+COPY --from=ghcr.io/astral-sh/ty:0.0.21 /ty /bin/
    12+
    


    stickies-v commented at 10:59 am on March 20, 2026:

    ty interface changes can happen between any versions, so pinning patch version makes sense.

    Both uv and ruff use minor for breaking changes, patch is guaranteed to be non-breaking. So, I suggest we pin both uv and ruff to minor version?


    willcl-ark commented at 11:47 am on March 20, 2026:
    Thank’s for review these pins. I took your suggestion in the latest push.
  59. in ci/lint/01_install.sh:26 in b497401dd5
    36-    clang
    37-  env CC=clang python-build "$(cat "/.python-version")" "${PYTHON_PATH}"
    38-fi
    39-export PATH="${PYTHON_PATH}/bin:${PATH}"
    40+# Install Python and create venv using uv (reads version from .python-version)
    41+uv python install
    


    stickies-v commented at 11:14 am on March 20, 2026:
    nit: I don’t think this line is necessary? uv venv automatically installs python

    willcl-ark commented at 11:28 am on March 20, 2026:
    This does seem to be correct.

    willcl-ark commented at 11:48 am on March 20, 2026:
    done in 152a04857e0

    maflcko commented at 12:26 pm on March 20, 2026:

    Ah interesting. I read the upstream docs on this and wasn’t sure if the .python-version does apply when it is dropped, but looking at the ci log, it seems to be correct/fine to drop:

    0...
    1[#20](/bitcoin-bitcoin/20/) 11.50 Running hooks in /etc/ca-certificates/update.d...
    2[#20](/bitcoin-bitcoin/20/) 11.50 done.
    3[#20](/bitcoin-bitcoin/20/) 11.53 + uv venv /python_env
    4[#20](/bitcoin-bitcoin/20/) 11.60 Downloading cpython-3.10.20-linux-x86_64-gnu (download) (28.5MiB)
    5[#20](/bitcoin-bitcoin/20/) 12.17  Downloaded cpython-3.10.20-linux-x86_64-gnu (download)
    6[#20](/bitcoin-bitcoin/20/) 12.29 Using CPython 3.10.20
    7[#20](/bitcoin-bitcoin/20/) 12.29 Creating virtual environment at: /python_env
    8...
    
  60. in ci/lint/01_install.sh:29 in b497401dd5 outdated
    39-export PATH="${PYTHON_PATH}/bin:${PATH}"
    40+# Install Python and create venv using uv (reads version from .python-version)
    41+uv python install
    42+uv venv /python_env
    43+
    44+export PATH="/python_env/bin:${PATH}"
    


    stickies-v commented at 11:16 am on March 20, 2026:
    What’s the point of installing into a venv and then adding the venv to PATH? At that point, doesn’t it make more sense to just uv pip install --system? It seems like that was true before this PR too, though.

    willcl-ark commented at 11:31 am on March 20, 2026:

    Is the system python the correct version though? I’m not sure how that works, and pretty sure --system will just print a few warnings about being dangerous each run.

    For me:

    • download python (if needed)
    • create venv, and activate it (or add to front of PATH)
    • install deps

    …is what python users have always done and would probably expect to do, and is easy to follow.

    Regarding adding to $PATH, without that the rust test_runner would have to activate the venv (or we’d have to use your system install)


    stickies-v commented at 11:40 am on March 20, 2026:

    is what python users have always done and would probably expect to do,

    Yeah venv is definitely the right approach for most contexts, just feels a bit silly on an ephemeral CI environment if we always want to use the venv anyway. But I haven’t properly thought through implications/interactions etc, so this might very well be a terrible idea. Let’s not do it in this PR, or at all.


    willcl-ark commented at 11:48 am on March 20, 2026:
    kept it using a venv + PATH for now, as it ain’t broke :)
  61. stickies-v approved
  62. stickies-v commented at 11:18 am on March 20, 2026: contributor

    Light code review ACK b497401dd5ee1d84dbaa9217e9ebb983d590067b

    I’ve been happily using uv and ruff for my own projects, and recently also switched to ty. No issues with it so far, and generally I think this is the right direction. Well maintained, documented, performant, …

  63. lint: switch to uv for python management in linter
    https://docs.astral.sh/uv/
    
    Install python in the linter using uv and a venv.
    This is faster and more simple than building pyenv.
    152a04857e
  64. lint: switch to ruff for formatting and linting
    - use dedicated ruff.toml for configuration
    - download via docker image layer at build time
    b4cbcbc4b0
  65. lint: switch to ty for python typechecking
    Add ty typechecker configuration file.
    
    Includes all needed rules to have `ty` pass on selected files in current
    codebase. This is pretty much all useful rules, but we can remove
    them systematically in follow-ups.
    
    https://docs.astral.sh/ty/
    https://astral.sh/blog/ty
    
    ty is faster and more maintained than mypy. Comes with better editor
    integrations, a language server and is written in rust (tm).
    b45433f761
  66. lint: use requirements.txt ab61f69612
  67. 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.
    ac4c3d5921
  68. lint: doc: detail lint tool install methods
    Installing tools in the dockerfile using `COPY --from` is better , but
    not all tools we use publish an OCI image to a non-docker.io registry.
    
    As we are frequently rate-limited from docker.io, only install tools
    which publish to another registry, e.g. ghcr.io.
    0809f765a0
  69. willcl-ark force-pushed on Mar 20, 2026
  70. in ci/lint_imagefile:9 in 0809f765a0
    5@@ -6,8 +6,14 @@
    6 
    7 FROM mirror.gcr.io/ubuntu:24.04
    8 
    9+# Pin uv and ruff to minor version and ty to a patch version to avoid breaking changes
    


    maflcko commented at 12:34 pm on March 20, 2026:

    I think it could make sense to link to the upstream docs on breaking changes. Otherwise, the doc link has to be manually researched every time this changes, or comes up.

    I am confident it will change for ty 0.1.0 (if and once it exists)?

  71. maflcko commented at 12:34 pm on March 20, 2026: member

    Haven’t read the upstream docs on versioning, but lgtm.

    review ACK 0809f765a080faf51cdabb2bb4382aad0c06780f 🦃

    Signature:

    0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
    1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
    2trusted comment: review ACK 0809f765a080faf51cdabb2bb4382aad0c06780f 🦃
    3fPIwXhbOOb7U10ERDsxCSSrZ7d9fVM4vQKi3HDoQGuetw8DWu76Rvcnp/TkTed8qxJOKlQs/qgMxZ3NJEPGwDg==
    
  72. DrahtBot requested review from stickies-v on Mar 20, 2026
  73. stickies-v commented at 2:19 pm on March 20, 2026: contributor
    re-lightcodereview ACK 0809f765a080faf51cdabb2bb4382aad0c06780f
  74. in ty.toml:8 in 0809f765a0
    0@@ -0,0 +1,22 @@
    1+[src]
    2+include = ["test/functional/**", "contrib/devtools/**"]
    3+
    4+[environment]
    5+root = ["test/functional"]
    6+
    7+[rules]
    8+deprecated = "ignore"
    


    fanquake commented at 1:30 am on March 23, 2026:

    deprecated = “ignore”

    This can be removed.

  75. DrahtBot requested review from fanquake on Mar 23, 2026
  76. fanquake commented at 2:18 am on March 23, 2026: member

    I tried testing this by introducing typing errors that are picked up in master (559df68240b9c74184a7da005f9ac2bbc52248a5):

     0diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py
     1index 90f597de17..1b3f2a26cd 100755
     2--- a/test/functional/feature_assumeutxo.py
     3+++ b/test/functional/feature_assumeutxo.py
     4@@ -558,7 +558,7 @@ class AssumeutxoTest(BitcoinTestFramework):
     5         txout_result = n1.gettxout(coinbase_tx['txid'], 0)
     6         assert_equal(txout_result['scriptPubKey']['desc'], coinbase_output_descriptor)
     7 
     8-        def check_tx_counts(final: bool) -> None:
     9+        def check_tx_counts(final: bool) -> bool:
    10             """Check nTx and nChainTx intermediate values right after loading
    11             the snapshot, and final values after the snapshot is validated."""
    12             for height, block in blocks.items():
    13diff --git a/test/functional/p2p_addrfetch.py b/test/functional/p2p_addrfetch.py
    14index 49c131f105..18e676c989 100755
    15--- a/test/functional/p2p_addrfetch.py
    16+++ b/test/functional/p2p_addrfetch.py
    17@@ -23,7 +23,7 @@ from test_framework.util import assert_equal
    18 ADDR = CAddress()
    19 ADDR.time = int(time.time())
    20 ADDR.nServices = P2P_SERVICES
    21-ADDR.ip = "192.0.0.8"
    22+ADDR.ip: bool = "192.0.0.8"
    23 ADDR.port = 18444
    
    0test/functional/p2p_addrfetch.py:26: error: Type cannot be declared in assignment to non-self attribute  [misc]
    1test/functional/feature_assumeutxo.py:561: error: Missing return statement  [return]
    2Found 2 errors in 2 files (checked 311 source files)
    3^---- ⚠️ Failure generated from lint-python.py
    4^^^
    5
    6^---- ⚠️ Failure generated from lint check 'all_python_linters' (Run all linters of the form: test/lint/lint-*.py)!
    

    However this branch (rebased) passes fine?

  77. willcl-ark commented at 1:02 pm on March 23, 2026: member

    I tried testing this by introducing typing errors that are picked up in master (559df68): However this branch (rebased) passes fine?

    Yes, this is expected and documented (poorly, apologies) in PR description here:

    Further work can drop individual rules from ty.toml fixing up the infringing code as necessary.

    My bad; I should have fleshed out what this means more concretely in the description: whilst we have ignore rules in ty.toml then certain things will go undetected (ignored), and this may be a temporary regression against master until the infracting code is fixed and the rule is un-ignored.

    If this is undesirable, then we should also bundle the fixes and rule-removals into this PR. I did not do this originally as some of the rules required quite invasive changes, so the plan was to add these incrementally in reviewable-chunks, if this change made it in.

    I will update the PR description to outline this more clearly.

    However to avoid the most egregious regressions I propose that we include fixes for the 5 least-invasive rules in this PR, which I will add in the next push to this branch. This is ~ 10 files changed, 28 insertions(+), 23 deletions(-). This will leave us with 11 ignore rules on master to work through.

  78. willcl-ark marked this as a draft on Mar 23, 2026

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: 2026-04-05 12:13 UTC

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