joinmarket-clientserver icon indicating copy to clipboard operation
joinmarket-clientserver copied to clipboard

Fix python3.13 compatibility

Open 3nprob opened this issue 3 months ago • 9 comments

  • Bump to twisted 24.11.0 to fix python3.13 compatibility
    • This is a conservative bump to just get the desired fix - it should probably be fine to upgrade twisted to latest 25.5 but keeping the scope small in this PR.
    • https://github.com/twisted/twisted/blob/twisted-24.11.0/NEWS.rst
  • Indicate python3.13 support in pyproject.toml
  • CI(unittests): Add python3.13 to python-versions
  • CI(unittests): Make ubuntu version explicit
    • During periods of migration it can be inconsistent and/or unpredictable when the transition of ubuntu-latest will happen for a given repo. Making it explicit removes it as source of confusion.
    • Run on both ubuntu-22.04 and ubuntu-24.04 to make sure there are no regressions in the former as long as it is supported.

Fixes:

  • #1794
  • #1787

3nprob avatar Sep 27 '25 01:09 3nprob

NACK You're rewriting the whole bencoder project & code and creating a whole new release of it in this repo!
This is too complex, convoluted and hard to audit.
Please submit your changes upstream to bencoder.pyx itself & get everyone to review it there.

seamo1 avatar Sep 27 '25 12:09 seamo1

You're rewriting the whole bencoder project & code and creating a whole new release of it in this repo! This is too complex, convoluted and hard to audit.

Hm?

This can be reviewed and audited commit-by-commit and is far from "rewriting the whole project".

  • Here upstream is copied/vendored as-is: 92e73e1e3c6f61bf6e90dc040382661135ed0a8a
  • Here is the one-line change: 1bd27afd4bafdfbbfe9b0a5c43bcbbd4a5dc1305
  • The rest is all to make CI actually run and test with improved coverage
    • Happy to break out or tweak this if preferred by maintainers

Please submit your changes upstream to bencoder.pyx itself & get everyone to review it there.

That would be my preferred default approach as well but unfortunately, upstream is unmaintained for > 2 years now.

An alternative would be to replace the module completely with an alternative implementation (which?) but to me it seems preferred to first do this minimal change, and then do that replacement as a follow-up (given the current state and timeline).

3nprob avatar Sep 27 '25 19:09 3nprob

I guess it's appropriate to add an entry to the bencoder.pyx changelog. Done now.

There is an upstream PR with the same change but again the package doesn't appear maintained: https://github.com/whtsky/bencoder.pyx/pull/145

3nprob avatar Sep 27 '25 19:09 3nprob

NACK switching to a maintained library seem more appropriate to me.

roshii avatar Sep 29 '25 21:09 roshii

NACK switching to a maintained library seem more appropriate to me.

Could this (vendoring) be a stepping stone? Unrelated development and fixes appear blocked by the broken CI (#1741, #1782, etc). Unbreaking that will also help validate a replacement.

3nprob avatar Sep 29 '25 21:09 3nprob

#1809 seems nicer solution.

kristapsk avatar Oct 19 '25 18:10 kristapsk

Bump to twisted 24.11.0 to fix python3.13 compatibility

  • This is a conservative bump to just get the desired fix - it should probably be fine to upgrade twisted to latest 25.5 but keeping the scope small in this PR.
  • https://github.com/twisted/twisted/blob/twisted-24.11.0/NEWS.rst

Could you split off this one in a separate PR?

kristapsk avatar Oct 19 '25 18:10 kristapsk

Bump to twisted 24.11.0 to fix python3.13 compatibility

  • This is a conservative bump to just get the desired fix - it should probably be fine to upgrade twisted to latest 25.5 but keeping the scope small in this PR.
  • https://github.com/twisted/twisted/blob/twisted-24.11.0/NEWS.rst

Could you split off this one in a separate PR?

Sure! For now holding off waiting for #1809 or equivalent to be merged first so CI tests can run cleanly. I wouldn't feel comfortable merging changes without CI suites running on a project of this nature.

3nprob avatar Oct 26 '25 07:10 3nprob

Bump to twisted 24.11.0 to fix python3.13 compatibility

  • This is a conservative bump to just get the desired fix - it should probably be fine to upgrade twisted to latest 25.5 but keeping the scope small in this PR.
  • https://github.com/twisted/twisted/blob/twisted-24.11.0/NEWS.rst

Could you split off this one in a separate PR?

I have updated this PR with the reduced scope (twisted update + python3.13).

3nprob avatar Oct 26 '25 23:10 3nprob