Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Make tox actions work on Debian 10 #7703

Merged
merged 1 commit into from
Jun 25, 2020

Conversation

ilmari
Copy link
Contributor

@ilmari ilmari commented Jun 15, 2020

  • Debian 10 ships Python 3.7, so make that basepython for all envs
  • Tox 3.7.0 doesn't seem to like trailing comments on dep lines

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
  • Pull request includes a sign off
  • Code style is correct (run the linters)

@clokep clokep requested a review from a team June 15, 2020 21:37
@clokep
Copy link
Member

clokep commented Jun 17, 2020

@ilmari I don't see anything wrong with this PR, but I'm not 100% sure what it is fixing. Is tox unable to run on Debian 10 without this change?

@ilmari
Copy link
Contributor Author

ilmari commented Jun 17, 2020

@clokep Yes, without this, running tox -e check_codestyle,check-newsfragment fails, because /usr/bin/python3 is Python 3.7.

I just realised a better option might be to make them reference {[base]basepython} so it only has to be changed in one place and is kept in sync across the different commands, or to leave out the basepython completely, so that it'll accept whatever version of Python 3 is in $PATH.

ilmari@ilmari-thinkpad:~/src/synapse (develop $)$ tox -e check_codestyle,check-newsfragment
check_codestyle recreate: /home/ilmari/src/synapse/.tox/check_codestyle
ERROR: InterpreterNotFound: python3.6
check-newsfragment create: /home/ilmari/src/synapse/.tox/check-newsfragment
ERROR: InterpreterNotFound: python3.6
_______________________________________________________ summary _______________________________________________________
ERROR:  check_codestyle: InterpreterNotFound: python3.6
ERROR:  check-newsfragment: InterpreterNotFound: python3.6

@erikjohnston
Copy link
Member

I think the latter makes sense to me, my worry here is that this will break on systems that don't have a python3.7? I haven't had a chance to really think about how this all works, but if we can make it not depend on a particular python then I think that would save me from having to think about this (I'm assuming that different versions of python won't result in different results for these linters).

- Remove the requirement for a specific version of Python
- Move dep comment to a separate line, Tox 3.7.0 like trailing ones

Signed-off-by: Dagfinn Ilmari Mannsåker <[email protected]>
Copy link
Member

@erikjohnston erikjohnston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@erikjohnston erikjohnston merged commit b099ef0 into matrix-org:develop Jun 25, 2020
@ilmari ilmari deleted the fix-tox-debian branch June 25, 2020 17:11
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit 'dc80a0762':
  1.16.0rc1
  Back out MSC2625 implementation (#7761)
  Additional configuration options for auto-join rooms (#7763)
  Add some metrics for inbound and outbound federation processing times (#7755)
  Explain the purpose of the "tests" conditional dependency requirement (#7751)
  Add another yield point to state res v2 (#7746)
  Move flake8 to end. Don't exit script on failure (#7738)
  Make tox actions work on Debian 10 (#7703)
  Yield during large v2 state res. (#7735)
  add org.matrix.login.jwt so that m.login.jwt can be deprecated (#7675)
  Set Content-Length for Metrics requests (#7730)
  Sync ignored table names in synapse_port_db to current database schema (#7717)
  Allow local media to be marked as safe from being quarantined. (#7718)
  Convert directory handler to async/await (#7727)
  Speed up state res v2 across large state differences. (#7725)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants