[WIP] Use Poetry to manage Synapse's dependencies by DMRobertson · Pull Request #12337 · matrix-org/synapse

DMRobertson · 2022-03-30T16:53:17Z

Reviewable commit-by-commit. Hopefully. Ignoring the lockfile, the diffstats for each individual commit should hopefully be managable.

I'd appreciate eyes on the output from CI. I'd like to ensure that it's still reasonably easy to follow, and clear that we're using the poetry locked environment rather than pip-installing the newest version of everything.

This set of changes:

uses poetry rather than setuptools to declare Synapse's project metadata and dependencies;
updates the docker image and debian package to use dependency versions managed in the poetry lockfile;
runs CI in the locked poetry environment; and
updates docs to strongly recommend using poetry for development purposes.

Included at the start are a couple of DEBUG commits which will ensure that the CI run for
5f8db3e will build all debian packages and run the twisted CI job. Before this is merged, they'll need to be undone.

Closes #11537.

TODO:

bump version number to appropriate version before merging,
compare the wheels and sdists generated on develop and on this branch.
CI that pip installs the latest released version of everything, to anticipate problems from people installing from wheels. (We'll no longer get notifications of this via our CI). This is probably best done in a separate PR.

- Nuke python_dependencies and setup.py - Allow commiting poetry.lock - Initial lockfile pulling in the latest version of deps at the time of locking

Missed in #12088. We got away with it because we were indirectly importing it in other places. Without this, we encounter pain in `poetry export`; and besides, we should be explicit about our direct dependencies.

See comment in the toml file: > The duplication here is awful. I hate hate hate hate hate it. I don't think that we're going to be adding many extras any time soon, and I think this is the least bad option.

After #12107 it's much easier for black, isort and flake8 to find the scripts we want them to lint.

Signed-off-by: Sean Quah <seanq@element.io>

In particular the documentaion for contributors should only advocate poetry. I am not thrilled at the propsect of now having N+1 installation methods---but at the very least we ought to mention poetry here.

debian/changelog

docker/Dockerfile-dhvirtualenv

pyproject.toml

.github/workflows/tests.yml

- Stop always running twisted trunk on this branch - Stop building all devs on this branch - Run olddeps after linting

squahtx

skimmed the changes - would appreciate more eyes on this!

.ci/scripts/test_old_deps.sh

.github/workflows/tests.yml

README.rst

babolivier

Thanks a lot for working on this! Really looking forward to seeing this PR land.
Not sure if the failing CI job is a flake or not...

docker/Dockerfile

pyproject.toml

synapse/__init__.py

README.rst

docs/code_style.md

docs/development/contributing_guide.md

docs/upgrade.md

.ci/scripts/test_old_deps.sh

Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> Co-authored-by: Brendan Abolivier <babolivier@matrix.org>

.ci/scripts/test_old_deps.sh

Co-authored-by: Brendan Abolivier <babolivier@matrix.org>

callahad · 2022-04-07T05:42:17Z

Note: CI is currently passing across the board except for Complement, which will be resolved by:

Move complement setup stuff into the Synapse repo #12404

Signed-off-by: Dan Callahan <danc@element.io>

instead, configure the tools to find sensible targets. Pulled out from #12337; part (but not all) of e53e99e. Related: #12107.

Signed-off-by: Dan Callahan <danc@element.io>

Olddeps and twisted trunk tests are handled in separate PRs. The PyPy config is a best-effort only; it's completely untested. Pulled out from #12337.

* Run "main" trial tests under poetry Olddeps and twisted trunk tests are handled in separate PRs. The PyPy config is a best-effort only; it's completely untested. Pulled out from #12337. * Changelog

I think these may have been retained in merge confusion.

we did need it to `install` though, (I think?)

DMRobertson · 2022-04-14T16:06:18Z

I've ripped out what I need to from this. Many thanks all for you eyes and words.

David Robertson and others added 15 commits March 30, 2022 14:07

DEBUG: always run twisted trunk; don't raise bugs

8dc02f9

DEBUG: build all debs

211becb

DEPS: Specify deps in pyproject.toml

7d57414

- Nuke python_dependencies and setup.py - Allow commiting poetry.lock - Initial lockfile pulling in the latest version of deps at the time of locking

DEPS: Add missing dependency on importlib_metadata

53e804f

Missed in #12088. We got away with it because we were indirectly importing it in other places. Without this, we encounter pain in `poetry export`; and besides, we should be explicit about our direct dependencies.

DEPS: Dirty duplication to make all work

c50da48

See comment in the toml file: > The duplication here is awful. I hate hate hate hate hate it. I don't think that we're going to be adding many extras any time soon, and I think this is the least bad option.

BUILDS: poetry in Synapse docker image

87ecb33

BUILDS: poetry in deb virtualenvs

d820ff2

CI: Use common linting workflow

e53e99e

After #12107 it's much easier for black, isort and flake8 to find the scripts we want them to lint.

CI: Fix the "main" trial test runs

3de49f4

CI: Fix the portdb job to run under poetry

03e5c93

CI: Fix the export-data job to run under poetry

6ab6f61

CI: Fix the olddeps job to run under poetry

0d8a54a

CI: Fix Twisted trunk to run under poetry

d3386af

Signed-off-by: Sean Quah <seanq@element.io>

Changelog

b03ffaa

DOCS: recommend poetry where appropriate

5f8db3e

In particular the documentaion for contributors should only advocate poetry. I am not thrilled at the propsect of now having N+1 installation methods---but at the very least we ought to mention poetry here.

DMRobertson commented Mar 30, 2022

View reviewed changes

debian/changelog Show resolved Hide resolved

docker/Dockerfile-dhvirtualenv Outdated Show resolved Hide resolved

pyproject.toml Outdated Show resolved Hide resolved

DMRobertson commented Mar 30, 2022

View reviewed changes

.github/workflows/tests.yml Outdated Show resolved Hide resolved

Undebug various changes

680dbc8

- Stop always running twisted trunk on this branch - Stop building all devs on this branch - Run olddeps after linting

DMRobertson marked this pull request as ready for review March 30, 2022 17:28

DMRobertson requested a review from a team as a code owner March 30, 2022 17:28

babolivier self-assigned this Mar 31, 2022

DMRobertson mentioned this pull request Mar 31, 2022

Nuke the tox packaging job #12334

Merged

squahtx reviewed Mar 31, 2022

View reviewed changes

.ci/scripts/test_old_deps.sh Outdated Show resolved Hide resolved

.github/workflows/tests.yml Outdated Show resolved Hide resolved

README.rst Outdated Show resolved Hide resolved

babolivier suggested changes Mar 31, 2022

View reviewed changes

DMRobertson commented Mar 31, 2022

View reviewed changes

.ci/scripts/test_old_deps.sh Outdated Show resolved Hide resolved

Fix typos spotted by Sean

a3fb2b8

Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> Co-authored-by: Brendan Abolivier <babolivier@matrix.org>

DMRobertson commented Mar 31, 2022

View reviewed changes

.ci/scripts/test_old_deps.sh Show resolved Hide resolved

DMRobertson mentioned this pull request Mar 31, 2022

update release script to be poetry-aware #12342

Closed

David Robertson and others added 2 commits March 31, 2022 16:39

Batch of suggestsion from Brendan, part 1

4c780ab

Co-authored-by: Brendan Abolivier <babolivier@matrix.org>

Batch of suggestsion from Brendan, part 2

8469bd8

Co-authored-by: Brendan Abolivier <babolivier@matrix.org>

callahad added 3 commits April 7, 2022 11:02

Clarify a CI workflow step for Twisted trunk

232a270

Signed-off-by: Dan Callahan <danc@element.io>

Inline CI steps for installing Twisted trunk

9225764

Signed-off-by: Dan Callahan <danc@element.io>

Merge branch 'develop' into dmr/pyproject-poetry

99892e7

Signed-off-by: Dan Callahan <danc@element.io>

DMRobertson mentioned this pull request Apr 7, 2022

CI: Get the portdb job to run under poetry #12412

Merged

Minor tweaks to comments in response to reviews

872dfb8

Signed-off-by: Dan Callahan <danc@element.io>

DMRobertson mentioned this pull request Apr 8, 2022

CI: Fix the export-data job to run under poetry #12418

Merged

DMRobertson pushed a commit that referenced this pull request Apr 8, 2022

Stop maintaining a list of lint targets

ad6da64

instead, configure the tools to find sensible targets. Pulled out from #12337; part (but not all) of e53e99e. Related: #12107.

DMRobertson mentioned this pull request Apr 8, 2022

Stop maintaining a list of lint targets #12420

Merged

DMRobertson marked this pull request as draft April 8, 2022 12:45

Merge branch 'develop' into dmr/pyproject-poetry

379944c

Signed-off-by: Dan Callahan <danc@element.io>

DMRobertson mentioned this pull request Apr 8, 2022

Use poetry lockfile in twisted trunk CI job #12425

Merged

Merge branch 'develop' into dmr/pyproject-poetry

fb611f1

DMRobertson mentioned this pull request Apr 11, 2022

Run lints under poetry in CI; remove lint tox jobs #12434

Merged

DMRobertson pushed a commit that referenced this pull request Apr 11, 2022

Add suggestions missed when cherry-picking from #12337

695eb3c

DMRobertson pushed a commit that referenced this pull request Apr 11, 2022

Run "main" trial tests under poetry

21fabc4

Olddeps and twisted trunk tests are handled in separate PRs. The PyPy config is a best-effort only; it's completely untested. Pulled out from #12337.

This was referenced Apr 11, 2022

Run "main" trial tests under poetry #12438

Merged

Use poetry to manage the virtualenv in debian packages #12449

Merged

Merge remote-tracking branch 'origin/develop' into dmr/pyproject-poetry

0cf4705

DMRobertson unassigned babolivier Apr 12, 2022

squahtx removed their request for review April 13, 2022 13:37

David Robertson added 3 commits April 14, 2022 10:42

Include some dockerfile changes from develop

988ff61

I think these may have been retained in merge confusion.

We don't need README to export

badd5cf

we did need it to `install` though, (I think?)

Merge remote-tracking branch 'origin/develop' into dmr/pyproject-poetry

443b94f

DMRobertson changed the title ~~Use Poetry to manage Synapse's dependencies~~ [WIP] Use Poetry to manage Synapse's dependencies Apr 14, 2022

Merge remote-tracking branch 'origin/develop' into dmr/pyproject-poetry

cbdbcb0

DMRobertson mentioned this pull request Apr 14, 2022

Dump setuptools; correct pyproject version number #12478

Merged

DMRobertson closed this Apr 14, 2022

Uh oh!

Conversation

DMRobertson commented Mar 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

squahtx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

babolivier left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

callahad commented Apr 7, 2022

Uh oh!

DMRobertson commented Apr 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

DMRobertson commented Mar 30, 2022 •

edited

Loading

babolivier left a comment •

edited

Loading