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

Update trial old deps CI to use poetry 1.2.0 #13707

Merged
merged 45 commits into from
Sep 6, 2022
Merged
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
60bbd62
Update trial old deps CI to use poetry 1.2.0
erikjohnston Sep 2, 2022
eddf98e
Newsfile
erikjohnston Sep 2, 2022
96c7cdb
Merge branch 'develop' into erikj/old_deps
erikjohnston Sep 2, 2022
c74a7af
Fix?
erikjohnston Sep 2, 2022
20f90b1
Try rejigging old-deps
erikjohnston Sep 2, 2022
d5234f7
Fix?
erikjohnston Sep 2, 2022
3953ab0
fix
erikjohnston Sep 2, 2022
19279b4
Fix??
erikjohnston Sep 2, 2022
bff7fb4
FIX ONESEFLF!!!
erikjohnston Sep 2, 2022
22ea18b
JUST RUN
erikjohnston Sep 2, 2022
70f3ffd
:(
erikjohnston Sep 2, 2022
ca584b4
Merge branch 'develop' into erikj/old_deps
erikjohnston Sep 5, 2022
86248cf
Add some debugging
erikjohnston Sep 5, 2022
0359401
More debugging details
erikjohnston Sep 5, 2022
1c25715
Try py3.8
erikjohnston Sep 5, 2022
1818f14
Try shelling out
erikjohnston Sep 5, 2022
7c5d4ad
Activate virtual env
erikjohnston Sep 5, 2022
f62405a
Fix?
erikjohnston Sep 5, 2022
24360df
Try the shell again
erikjohnston Sep 5, 2022
e11f2ba
Try just activating the virtualenv
erikjohnston Sep 5, 2022
6225fd5
try uninstall tests
erikjohnston Sep 5, 2022
362ab60
More debugging
erikjohnston Sep 5, 2022
74b015e
Add some caching
erikjohnston Sep 5, 2022
eae0143
Add current directory to path
erikjohnston Sep 5, 2022
dbdb3b0
Try single job
erikjohnston Sep 5, 2022
435342d
Try
erikjohnston Sep 5, 2022
77eada8
Try a different tack
erikjohnston Sep 5, 2022
702818a
Add and populate cache
erikjohnston Sep 5, 2022
f5b21f3
Re-enable tests
erikjohnston Sep 5, 2022
96b8bb0
Update cache
erikjohnston Sep 5, 2022
54d4a10
Fix
erikjohnston Sep 5, 2022
bb2a6df
Run tests
erikjohnston Sep 5, 2022
c9acee4
Install build deps
erikjohnston Sep 5, 2022
9353e82
Update deps
erikjohnston Sep 5, 2022
e829e94
list bcrypt files
erikjohnston Sep 5, 2022
25cf13f
Try manually installing bcrypt
erikjohnston Sep 5, 2022
7f47134
Reinstall bcrypt
erikjohnston Sep 5, 2022
4fe4bc4
fix
erikjohnston Sep 5, 2022
5205d05
Install all deps
erikjohnston Sep 5, 2022
fb1364d
Install all deps
erikjohnston Sep 5, 2022
737a827
Install test deps too
erikjohnston Sep 5, 2022
f4b582d
Try not reinstalling bcrypt
erikjohnston Sep 5, 2022
4756c35
Make it ready for review
erikjohnston Sep 5, 2022
28f7eb3
Review comments
erikjohnston Sep 6, 2022
ac0d74a
Expand comment
erikjohnston Sep 6, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,8 @@
# - creates a venv with these old versions using poetry; and finally
# - invokes `trial` to run the tests with old deps.

# Prevent tzdata from asking for user input
export DEBIAN_FRONTEND=noninteractive

set -ex

apt-get update
apt-get install -y \
python3 python3-dev python3-pip python3-venv pipx \
libxml2-dev libxslt-dev xmlsec1 zlib1g-dev libjpeg-dev libwebp-dev

export LANG="C.UTF-8"

# Prevent virtualenv from auto-updating pip to an incompatible version
export VIRTUALENV_NO_DOWNLOAD=1

Expand Down Expand Up @@ -55,7 +45,7 @@ sed -i \
# toml file. This means we don't have to ensure compatibility between old deps and
# dev tools.

pip install --user toml
pip install toml wheel

REMOVE_DEV_DEPENDENCIES="
import toml
Expand All @@ -69,15 +59,12 @@ with open('pyproject.toml', 'w') as f:
"
python3 -c "$REMOVE_DEV_DEPENDENCIES"

pipx install poetry==1.1.14
~/.local/bin/poetry lock
pip install poetry==1.2.0
poetry lock
Comment on lines -72 to +57
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW it's possible that poetry's dependencies might conflict with those of toml and wheel here. It's probably not a problem but calling it out for paranoia's sake.

Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't poetry lock here be ignoring toml and wheels deps?

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right that the virtualenv managed by poetry will be independent of toml and wheel. What I mean: the depedencies of the poetry package itself, i.e. the stuff listed here

$ pipx runpip poetry show poetry
Name: poetry
Version: 1.2.0
Summary: Python dependency management and packaging made easy.
Home-page: https://python-poetry.org/
Author: Sébastien Eustace
Author-email: sebastien@eustace.io
License: MIT
Location: /home/dmr/.local/pipx/venvs/poetry/lib/python3.10/site-packages
Requires: cachecontrol, cachy, cleo, crashtest, dulwich, html5lib, jsonschema, keyring, packaging, pexpect, pkginfo, platformdirs, poetry-core, poetry-plugin-export, requests, requests-toolbelt, shellingham, tomlkit, urllib3, virtualenv
Required-by: poetry-plugin-export

might have conflicts with the toml and wheel versions we've just installed.

But a) it doesn't look like there are any conflicts, and b) I think pip would at least warn us about them.

TL;DR ignore this comment, it's all good


echo "::group::Patched pyproject.toml"
cat pyproject.toml
echo "::endgroup::"
echo "::group::Lockfile after patch"
cat poetry.lock
echo "::endgroup::"

~/.local/bin/poetry install -E "all test"
~/.local/bin/poetry run trial --jobs=2 tests
40 changes: 33 additions & 7 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,42 @@ jobs:
# Note: sqlite only; no postgres
if: ${{ !cancelled() && !failure() }} # Allow previous steps to be skipped, but not fail
needs: linting-done
runs-on: ubuntu-latest
runs-on: ubuntu-20.04
steps:
- uses: actions/checkout@v2
- name: Test with old deps
uses: docker://ubuntu:focal # For old python and sqlite
# Note: focal seems to be using 3.8, but the oldest is 3.7?
# See https://github.com/matrix-org/synapse/issues/12343

# There aren't wheels for some of the older deps, so we need to install
# their build dependencies
- run: |
sudo apt-get -qq install build-essential libffi-dev python-dev \
libxml2-dev libxslt-dev xmlsec1 zlib1g-dev libjpeg-dev libwebp-dev

- uses: actions/setup-python@v4
with:
python-version: '3.7'

# Calculating the old-deps actually takes a bunch of time, so we cache the
# pyproject.toml / poetry.lock.
- uses: actions/cache@v3
id: cache-poetry-old-deps
name: Cache poetry.lock
with:
workdir: /github/workspace
entrypoint: .ci/scripts/test_old_deps.sh
path: |
poetry.lock
pyproject.toml
Copy link
Contributor

Choose a reason for hiding this comment

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

I found this confusing; maybe worth a comment noting that the reason we cache pyproject.toml is because the step beneath patches it.

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding:

  • We compute a cache key based on the pyproject.toml in the checkout
  • If there's a hit, we retrieve the patched pyproject.toml and poetry.lock from the cache
  • Otherwise we patch pyproject.toml and regenerate the lockfile
  • During GHA cleanup we will write the patched pyproject.toml and poetry.lock to the cache for future runs

It does need a little bit of thought though: a quick comment SoundsGTM

key: poetry-old-deps2-${{ hashFiles('pyproject.toml') }}
- name: Prepare old deps
if: steps.cache-poetry-old-deps.outputs.cache-hit != 'true'
run: .ci/scripts/prepare_old_deps.sh

# We only now install poetry so that `setup-python-poetry` caches the
# right poetry.lock's dependencies.
- uses: matrix-org/setup-python-poetry@v1
with:
python-version: '3.7'
extras: "all test"

- run: poetry run trial -j 2 tests
- name: Dump logs
# Logs are most useful when the command fails, always include them.
if: ${{ always() }}
Expand Down
1 change: 1 addition & 0 deletions changelog.d/13707.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Update trial old deps CI to use poetry 1.2.0.