Travis: enforce Python 3.4 support through linter #14884

pull Sjors wants to merge 2 commits into bitcoin:master from Sjors:2018/12/python-3-4 changing 4 files +12 −3
  1. Sjors commented at 2:50 PM on December 6, 2018: member

    The minimum supported version of Python is 3.4 according to dependencies.md. This PR makes the Travis linter use this version in order to catch accidental use of modern syntax.

  2. laanwj added the label Tests on Dec 6, 2018
  3. practicalswift commented at 2:58 PM on December 6, 2018: contributor

    Concept ACK

    Good idea!

    ~What about bumping the requirement to 3.5 to allow for Xenial?~

  4. laanwj commented at 2:59 PM on December 6, 2018: member

    utACK

  5. hebasto commented at 3:07 PM on December 6, 2018: member

    Concept ACK

  6. in .travis.yml:108 in 400a46c629 outdated 
     103 | @@ -104,6 +104,8 @@ jobs:
 104 |          BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 CPPFLAGS=-DDEBUG_LOCKORDER --with-sanitizers=thread --disable-hardening --disable-asm CC=clang CXX=clang++"
 105 |  # x86_64 Linux (no depends, only system libs, sanitizers: address/leak (ASan + LSan) + undefined (UBSan) + integer)
 106 |      - stage: test
 107 | +      language: python
 108 | +      python: '3.4' # Minimum supported version according to dependencies.md
    MarcoFalke commented at 4:28 PM on December 6, 2018:

    No idea how this is supposed to work. We run the tests in a docker, which is completely separate from the travis environment?

    Sjors commented at 4:41 PM on December 6, 2018:

    In that case this probably won't work indeed... I thought we only used Docker on the Xenial host? I'm still waiting for Travis to build my test branch.

    Sjors commented at 4:48 PM on December 6, 2018:

    I see, trying a different approach...

  7. MarcoFalke commented at 4:47 PM on December 6, 2018: member

    I mean of course strong Concept ACK if you figure out a way to install older versions (specific versions) of packages in ubuntu.

  8. Sjors force-pushed on Dec 6, 2018
  9. Sjors force-pushed on Dec 6, 2018
  10. Sjors renamed this:
    [WIP] Travis: use Python 3.4 on one instance to check support
    [WIP] bump Python min version to 3.5 and enforce through Travis linter
    on Dec 6, 2018
  11. Sjors force-pushed on Dec 6, 2018
  12. Sjors commented at 6:56 PM on December 6, 2018: member

    The only way to install older or newer versions of Python is through pyenv. I'd rather not add another dependency.

    ~Instead, a more practical solution is to bump the minimum Python version to 3.5, because our linters use that. It's available on Ubuntu Xenial, but we would lose support for (almost EOL) Trusty (Python 3.4).~

    In fact, check-doc.py, optimize-pngs.py , gitian-build.py, and verify-commits.py rely on Python 3.6 via check_output(encoding='utf8'). However only a small subset of developers need those scripts. I made trivial workaround for check-doc.py so Travis can use it with Python ~3.5~ 3.4, but this workaround is not safe for the other scripts, see #14128

    ~Alternatively we could modify the linter to work with Python 3.4 syntax. I personally can't be bothered, but I'll happily cherry-pick a solution :-)~

    I also added .python-version for pyenv which will cause tests with too modern syntax to fail on a developer machine before Travis. When using a script that does require Python >=3.5, you'll have to do pyenv local 3.7.1.

  13. Sjors force-pushed on Dec 6, 2018
  14. Sjors commented at 6:59 PM on December 6, 2018: member
  15. Sjors renamed this:
    [WIP] bump Python min version to 3.5 and enforce through Travis linter
    Bbump Python min version to 3.5 and enforce through Travis linter
    on Dec 6, 2018
  16. MarcoFalke commented at 7:02 PM on December 6, 2018: member

    The python 3.4 requirement is only for the tests, which might be run by users that compile from source. Personally I don't care about the python version of developer tools and linters and I don't mind not supporting 3.4 for those.

  17. Sjors force-pushed on Dec 6, 2018
  18. Sjors force-pushed on Dec 6, 2018
  19. Sjors commented at 7:16 PM on December 6, 2018: member

    @MarcoFalke I initially thought the linter itself needed Python 3.5, but that's not true anymore. So in that case we don't have to bump the version, nice!

    Travis build failure demo when you use too modern syntax: https://travis-ci.org/bitcoin/bitcoin/jobs/464581145#L609

  20. Sjors renamed this:
    Bbump Python min version to 3.5 and enforce through Travis linter
    Travis: enforce Python 3.4 support through linter
    on Dec 6, 2018
  21. in .travis.yml:46 in bef76361ac outdated 
      42 | @@ -43,7 +43,7 @@ jobs:
  43 |        env:
  44 |        cache: false
  45 |        language: python
  46 | -      python: '3.6'
  47 | +      python: '3.4' # Oldest supported version according to doc/dependencies.md
    MarcoFalke commented at 7:28 PM on December 6, 2018:

    Hmm, that would set the minimum version of the linters to 3.4, but not of the functional test suite. (Note that only parsing code according to 3.4 syntax does not mean it can run 3.4)

    Sjors commented at 7:33 PM on December 6, 2018:

    The linter uses the available Python executable to lint the functional tests. It will throw an error if sees a syntax that it doesn't recognise, see above demo. So it's not necessary to run the functional tests themselves in Python 3.4, which would require adding pyenv as a dependency inside the Docker container and slow things down.

    Sjors commented at 7:35 PM on December 6, 2018:

    Also, though hopefully not needed, any developer who uses pyenv on their machine will detect any mistake that slips through the cracks once they run the functional test suite on master.

  22. laanwj commented at 1:59 PM on December 7, 2018: member

    I would prefer having all tools work with the supported version of Python (3.4), for consistency for developers, but agree it's not urgent—having the tests pass is most important.

  23. practicalswift commented at 2:42 PM on December 7, 2018: contributor

    @laanwj That's a good point. Agree.

  24. Sjors commented at 4:05 PM on December 7, 2018: member

    I agree with that too, but it's non-trivial because #14128 had to introduce Python 3.6 syntax in those tools to deal with some sort of encoding hell.

  25. laanwj commented at 4:34 PM on December 7, 2018: member

    I agree with that too, but it's non-trivial because #14128 had to introduce Python 3.6 syntax in those tools to deal with some sort of encoding hell.

    Yes, that particular functionality looks hard to replace. I didn't know that was Python 3.6 specific. (edit: whoops accidental close)

    Edit: One way to handle this would be to wrap the appropriate subprocess calls and do the encoding magic only for 3.6, fall back to the system encoding in 3.4 so that it'd at least work. But I don't know it's worth it. At some point in the future we'll be able to bump the minimum to 3.6 and any work there will be a waste of time.

  26. laanwj closed this on Dec 7, 2018
  27. laanwj reopened this on Dec 7, 2018
  28. [test] Travis: enforce Python 3.4 support in functional tests
    Make lint/check-doc.py Python 3.4 compatible.

Also add .python-version for pyenv which will cause tests with too
modern syntax to fail on developer machine rather than on Travis.
    74ce326831
  29. [test] functional framework: add CScript hex() for Python 3.4
    test/functional/wallet_importmulti.py failed with:
AttributeError: 'CScript' object has no attribute 'hex'
    31926ee8cf
  30. Sjors force-pushed on Dec 12, 2018
  31. Sjors commented at 10:13 AM on December 12, 2018: member

    Rebased and added a workaround for calling .hex() on a CScript array in #14886 (cc @jnewbery), which doesn't work before Python 3.5.

  32. laanwj commented at 12:16 PM on December 13, 2018: member

    utACK 31926ee8cfc73501524dfa0fef2ccbaa786d6a00

    Going to merge this immediately to avoid any non-Python3.4isms from sneaking in.

  33. laanwj merged this on Dec 13, 2018
  34. laanwj closed this on Dec 13, 2018
  35. laanwj referenced this in commit 84dc252a02 on Dec 13, 2018
  36. in .python-version:1 in 31926ee8cf 
       0 | @@ -0,0 +1 @@
   1 | +3.4.9
    jnewbery commented at 3:30 PM on December 13, 2018:

    Does pyenv allow you to comment this file? It's not immediately obvious that this is for pyenv, how it's supposed to work, or when we should update this.

    Sjors commented at 5:52 PM on December 13, 2018:

    I'll look into that. At least you can find out via git-blame :-P

    Sjors commented at 6:59 PM on January 15, 2019:

    pyenv doesn't like it when you add a comment to .python-version, so I just updated the code style guidelines with an explanation in #15173.

  37. jnewbery commented at 3:31 PM on December 13, 2018: member

    utACK 31926ee8cfc73501524dfa0fef2ccbaa786d6a00. One request inline.

    Thanks for catching my hex() mistake!

  38. Sjors deleted the branch on Dec 13, 2018
  39. MarcoFalke referenced this in commit 391a27376b on Jan 15, 2019
  40. Sjors commented at 6:12 PM on January 29, 2019: member

    I missed a spot, see #15285

  41. MarcoFalke referenced this in commit 33480c6366 on Feb 14, 2019
  42. PastaPastaPasta referenced this in commit c0ec92e9e2 on Jun 26, 2021
  43. PastaPastaPasta referenced this in commit 2ee07e0da2 on Jun 27, 2021
  44. PastaPastaPasta referenced this in commit 89972eeca7 on Jun 28, 2021
  45. PastaPastaPasta referenced this in commit 97e53da21d on Jun 28, 2021
  46. PastaPastaPasta referenced this in commit 589bbc9166 on Jun 29, 2021
  47. linuxsh2 referenced this in commit f273da1c2a on Jul 29, 2021
  48. linuxsh2 referenced this in commit d70d401c72 on Aug 3, 2021
  49. DrahtBot locked this on Dec 16, 2021
Contributors
SjorspracticalswiftlaanwjhebastoMarcoFalkejnewbery
Labels
Tests
Linked (view graph)
#15173 [doc] explain what .python-version does#15285 build: Prefer Python 3.4 even if newer versions are present on the system
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-14 09:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me