Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use poetry to lock dependencies #18

Closed
wants to merge 4 commits into from
Closed

Use poetry to lock dependencies #18

wants to merge 4 commits into from

Conversation

DMRobertson
Copy link
Contributor

@DMRobertson DMRobertson commented Jan 20, 2022

This set of changes

  • ditches setup.py in favour of poetry configuration in pyproject.toml
  • .gitignores poetry's lockfile. (signedjson is a library---there is no fixed set of dependencies it ships with and we should not allow ourselves the luxury of thinking so!)
  • notes that setup.py build can be replaced with poetry build.

From the bigger picture, this is a practice run to let us

  • experience poetry in use
  • (in subsequent PRs) work out how to use poetry in CI

How to try this out

  1. Install poetry.
  • They recommend using a script to install a self-managed version of poetry in ~/.poetry.
  • I naively used pip install --user poetry.
  • I've seen others recommend pipx install poetry.
  • Check it's on your path: which poetry.
  1. checkout and cd to this repository.
  2. poetry install. This will create a virtualenv behind the scenes and populate it with the right packages.
  3. To actually use the virtualenv:
  • poetry env info shows you which interpreter you're running, and the virtualenv location
  • for one-off commands: poetry run echo hello
  • to activate the virtualenv in a new shell: poetry shell. exit or Ctrl-D effectively quits the venv, instead of deactivate.
  • See poetry env --help for details of how you can switch between different environments.

direnv

I found it really pleasant to use direnv to automatically switch to/from the env as I navigated into/out of the directory. Needed some extra config to support poetry though, see here (which eventually made its way into the wiki). By the looks of it, this packages queries poetry to find out where the current venv is on disk, and appends its bin directory to your path. This doesn't seem to detect use of poetry env to switch to another virtual environment within the project.

What do the built packages look like?

We want to convince ourselves that poetry generates and equivalent sdist to setuptools. One can test as follows:

  1. poetry build. A wheel and sdist should be generated in dist.
  2. pushd dist; tar -xzf signedjson-1.1.1.tar.gz; popd;
  3. curl https://files.pythonhosted.org/packages/dc/f2/2b3c5574ab77e086597cdc85781fa1d2ef1b4e55bd53d47308bbd2ad7596/signedjson-1.1.1.tar.gz > /tmp/signedjson-pypi.tar.gz
  4. pushd /tmp; tar -zxf signedjson-pypi.tar.gz; popd
  5. diff -ru /tmp/signedjson-1.1.1/ dist/signedjson-1.1.1 > diff.txt

Here's my diff.txt.

It's probably most instructive to see which files have been added and removed as follows:

$ tree /tmp/signedjson-1.1.1/ > pypi.txt
$ tree dist/signedjson-1.1.1/ > poetry.txt
$ diff -u pypi.txt poetry.txt 

This yields

--- pypi.txt	2022-01-21 18:45:03.976920722 +0000
+++ poetry.txt	2022-01-21 18:45:11.595932982 +0000
@@ -1,24 +1,19 @@
-/tmp/signedjson-1.1.1/
+dist/signedjson-1.1.1/
 ├── changelog.d
+│   ├── 11.misc
+│   ├── 18.misc
+│   └── 9.bugfix
 ├── CHANGELOG.md
 ├── LICENSE
-├── MANIFEST.in
 ├── PKG-INFO
 ├── pyproject.toml
 ├── README.rst
-├── setup.cfg
 ├── setup.py
 ├── signedjson
 │   ├── __init__.py
 │   ├── key.py
 │   ├── sign.py
 │   └── types.py
-├── signedjson.egg-info
-│   ├── dependency_links.txt
-│   ├── PKG-INFO
-│   ├── requires.txt
-│   ├── SOURCES.txt
-│   └── top_level.txt
 ├── tests
 │   ├── __init__.py
 │   ├── test_key.py
@@ -26,4 +21,4 @@
 │   └── test_sign.py
 └── tox.ini
 
-4 directories, 22 files
+3 directories, 18 files

@DMRobertson DMRobertson requested a review from a team as a code owner January 20, 2022 16:27
@callahad
Copy link
Contributor

Would you mind throwing in a recursive diff of the two sdists (poetry and current)?

.gitignore Show resolved Hide resolved
@clokep clokep requested a review from a team January 20, 2022 16:30
@DMRobertson
Copy link
Contributor Author

Would you mind throwing in a recursive diff of the two sdists (poetry and current)?

Description updated with a diff between tree on the two sdists.

@DMRobertson
Copy link
Contributor Author

Description updated with a diff between tree on the two sdists.

Note that

  • poetry seems to generate its own setup.py in the sdist (a compatibility aid?). It does not generate a setup.cfg
  • setuptools generated a very boring setup.cfg:
    [egg_info]
    tag_build = 
    tag_date = 0
    

Additionally, the contents of the (new) wheel are minimal:

 tree -a wheel
wheel
├── signedjson
│   ├── __init__.py
│   ├── key.py
│   ├── sign.py
│   └── types.py
├── signedjson-1.1.1.dist-info
│   ├── LICENSE
│   ├── METADATA
│   ├── RECORD
│   └── WHEEL
└── signedjson-1.1.1-py3-none-any.whl

2 directories, 9 files

@DMRobertson
Copy link
Contributor Author

For cross referencing, matrix-org/synapse#11537

@callahad
Copy link
Contributor

  • poetry seems to generate its own setup.py in the sdist (a compatibility aid?). It does not generate a setup.cfg

Yep. Poetry 1.2 will stop generating setuptools stubs in sdists by default, now that support for PEP 517 builds is widespread. There's a generate-setup-file flag which can be used to skip generating the file in older Poetry versions, but it's fine to leave it as-is.

@DMRobertson
Copy link
Contributor Author

Would you mind throwing in a recursive diff of the two sdists (poetry and current)?

Description updated with a diff between tree on the two sdists.

Oh, I've just learned that diff -r is a thing. Description updated.

@DMRobertson
Copy link
Contributor Author

I'm going to force push a simpler commit history. Apologies all, but I think it's more helpful than to see my works in progress.

David Robertson added 3 commits January 26, 2022 13:46
The manifest file has been replaced with `include` config. To check this
was sensible, I compared the sdist on pypi against the sdist generated
by `poetry build`. I adjusted the TOML config until the newer sdist
matched the existing one.

We don't commit the lockfile, because this is a library.
@DMRobertson
Copy link
Contributor Author

This was a good exercise, but I think sydent (being an application, not a library) makes for a better test bed.

@DMRobertson DMRobertson closed this Feb 2, 2022
@clokep clokep deleted the poetry branch September 23, 2022 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants