Skip to content

#11655 Update package metadata#11656

Merged
adiroiban merged 9 commits intotwisted:trunkfrom
ofek:modernize-metadata
Oct 24, 2022
Merged

#11655 Update package metadata#11656
adiroiban merged 9 commits intotwisted:trunkfrom
ofek:modernize-metadata

Conversation

@ofek
Copy link
Copy Markdown
Contributor

@ofek ofek commented Sep 11, 2022

Scope and purpose

Fixes #11655

Hello there! The Python packaging ecosystem has standardized on the interface for build backends (PEP 517/PEP 660) and the format for metadata declaration (PEP 621/PEP 631). As a result, the execution of setup.py files is now deprecated.

So, I'm spending my free time updating important projects so that they are modernized and set an example for others 😄

Summary of changes

This implements PEP 621, obviating the need for setup.py and MANIFEST.in. The build backend hatchling (of which I am a maintainer in the PyPA) is now used as that is the default in the official Python packaging tutorial. Hatchling is available on all the major distribution channels such as Debian, Fedora, Arch Linux, conda-forge, Nixpkgs, Alpine Linux, FreeBSD/OpenBSD, Gentoo Linux, MacPorts, OpenEmbedded, Spack, MSYS2, etc.

Notes

  • The source distributions on PyPI are erroneously shipping a build artifact src/*.egg-info from python setup.py develop; this is now fixed
  • I didn't know what to put for news so I just talked about how using underscores for extras names is now deprecated to comply with PEP 685
  • I think admin/twisted.spec is right but I'm not an expert
  • Self-referential extras are now used which pip has supported since 21.2

Future

  • Move config for tools out of setup.cfg
  • Update a few places in demo/example docs that talk about creating a setup.py

Contributor Checklist:

  • The title of the PR should describe the changes and starts with the associated issue number, like “#9782 Remove twisted.news. #1234 Brief description”.
  • A release notes news fragment file was create in src/twisted/newsfragments/ (see: Release notes fragments docs.)
  • The automated tests were updated.
  • Once all checks are green, request a review by leaving a comment that contains exactly the string please review.
    Our bot will trigger the review process, by applying the pending review label
    and requesting a review from the Twisted dev team.

@ofek ofek force-pushed the modernize-metadata branch from 9029bc7 to 0f6eba3 Compare September 11, 2022 02:53
@adiroiban
Copy link
Copy Markdown
Member

Thanks for helping with this.

I think admin/twisted.spec is right but I'm not an expert

What is that file. Do we need it?

Self-referential extras are now used which pip pypa/pip#11296 since 21.2

Thanks.

This needs and update... but maybe in another PR.
The dev should also install the test dependencies.


There is also this PR #1656

I guess that it would help to merge that first.


Don't forgot to ask for a review, when you think this is ready for review

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Sep 12, 2022

Oh thanks I forgot:

please review

@chevah-robot chevah-robot requested a review from a team September 12, 2022 15:58
@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Sep 12, 2022

I think admin/twisted.spec is right but I'm not an expert

What is that file. Do we need it?

No need I think https://src.fedoraproject.org/rpms/python-twisted/blob/rawhide/f/python-twisted.spec

@ofek ofek force-pushed the modernize-metadata branch from 95d4b73 to 0295a48 Compare October 5, 2022 04:56
@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 5, 2022

I rebased to fix the conflict.

@adiroiban
Copy link
Copy Markdown
Member

Thanks, @ofek for the updates and the changes.

The changes look good.

I don't have much experience with python packaging ...so I don't know what to say.

I understand that the changes don't bring many benefits...at least not in the short term.

This is more about getting the hatch library used by more projects.

This looks to me more like a political decision, rather than a technical one :)


Before merging this I would like to get some feedback from other team members.

@graingert you were at some point our packaging champion, Any comments against merging this.

@glyph sorry for the noise, maybe you care.

I have not mentioned Tom and Jean-Paul as I think they are already subscribed to this.

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 6, 2022

This looks to me more like a political decision, rather than a technical one

Not at all! As I mentioned:

  • the execution of setup.py files is deprecated and will, without exaggeration, stop working one day
  • names of extras need to be normalized per PEP 685 and all builders will do that

@glyph
Copy link
Copy Markdown
Member

glyph commented Oct 6, 2022

important projects

flattery will get you everywhere :)

* **all-non-platform** - installs **tls**, **conch**, **soap**, and **serial** options.

* **macos_platform** - **all_non_platform** options and `pyobjc`_ to work with Objective-C apis.
* **macos-platform** - **all-non-platform** options and `pyobjc`_ to work with Objective-C apis.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for updating the docs here.

Co-authored-by: Glyph <glyph@twistedmatrix.com>
Copy link
Copy Markdown
Member

@glyph glyph left a comment

Choose a reason for hiding this comment

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

So, @ofek, I'm inclined to approve this, I love the idea of moving into the future here, and I appreciate your thoroughness in mentioning platforms and distribution channels which support the necessary tooling.

I particularly love getting rid of MANIFEST.in / check-manifest, which involves a huge amount of obscure knowledge that nobody really understands, and centralizing around pyproject.toml which is much friendlier to user comprehension.

However, here's the question that I would really prefer to answer before saying "yes, definitely", which is: what platforms, and particularly what out-of-the-box experiences, are we breaking here?

For example: on macOS, you generally should install your own Python. However, there's a "default" workflow where, if you run python3 in a terminal, a dialog box pops up and installs command-line development tools, which is a convenient and frequently-used workflow for novice developers who may not have much experience of the CLI or of other platforms. So there's some motivation to continue to support Python 3.9.6, which is the current version of Python bundled into Xcode, and pip 21.2.4.

So, hooray, for that use-case, we're all good. But what versions of Linux and Windows do people actually run on their laptops and production servers? What audiences depend on old versions of things?

In general we're becoming less tolerant of people who are massively out of date and using unsupported platforms, but are there any still-supported platforms that are going to have difficulty with this?

That said, I think that blocking based on this concern would be bad. It may create a huge amount of tedious work to figure out, and then if we do figure out that some platform is broken we have to actually come up with some crisp criteria for which ones we support and why, which is work we haven't done yet. And meanwhile, as you say, new installs are gonna break one day when setup.py stops working.

So here's how I would suggest splitting the difference: send an email to twisted@python.org to increase the visibility of the upcoming change, explain as best you can what versions of things will be supported, ask people to test out this branch for a little while — not more than two weeks, probably less if we actually get any feedback — and then land it.

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 7, 2022

I sent an email as requested!

@glyph
Copy link
Copy Markdown
Member

glyph commented Oct 9, 2022

I sent an email as requested!

Hm. I don't even see any held messages on the mailing list admin interface. Did you subscribe to the list first?

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 9, 2022

Oops no, how?

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 9, 2022

Okay, I think it worked this time

edit: yes https://mail.python.org/archives/list/twisted@python.org/thread/CCDCU4R2VLJEPENPGBJLRJL3IMRXVEYT/

@hynek
Copy link
Copy Markdown
Member

hynek commented Oct 10, 2022

To put

what out-of-the-box experiences, are we breaking here?

into perspective: Black, the probably most used Python CLI tool has switched to Hatch.

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 12, 2022

I liked the setup.py as it allows you to do all kind of "crazy things"

Local build hook:

[...] like the GitHub Readme URL replacements

We're still doing that here thanks to @hynek's hatch-fancy-pypi-readme plugin

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 12, 2022

Tests are passing but uploading to Codecov is failing. I actually added that to Hatch over the weekend for a day then got rid of it pypa/hatch#545

@adiroiban
Copy link
Copy Markdown
Member

adiroiban commented Oct 18, 2022

Hi,

The coverage was fixed after a retry...but I see the documentation build failing when using tox.

https://github.com/twisted/twisted/actions/runs/3237401117/jobs/5304748238#step:5:17

It looks like sphinx is not automatically installed by tox -e narrativedocs
it uses

extras =
    dev_release

The separate Read the docs build works.

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 18, 2022

Woah, real bug. pip treats (pre-PEP 685) dev-release and dev_release as distinct, but it's normalized self-referentially so dev_release = ["twisted[dev-release]"] only uses dev_release which provides no other dependencies. I'll fix tonight.

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 19, 2022

Fixed, Hatchling made this quite easy 🙂

I liked the setup.py as it allows you to do all kind of "crazy things"

Now we're doing such a thing!

@ofek ofek force-pushed the modernize-metadata branch from 7349295 to bd6eeb2 Compare October 19, 2022 04:25
@abravalheri
Copy link
Copy Markdown

abravalheri commented Oct 19, 2022

the execution of setup.py files is deprecated and will, without exaggeration, stop working one day

One important piece of information:

Using setup.py to configure a Python build is not deprecated at all and is not going to stop working.

Executing the file, in the sense of running it as a CLI tool, is deprecated, so, from the setuptools point of view, the important change is to replace:

  • python setup.py install with pip install .
  • python setup.py develop with pip install -e ..
  • python setup.py sdist bdist_wheel with python -m build

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 23, 2022

CI is passing now btw

@adiroiban
Copy link
Copy Markdown
Member

Thanks for the updated.

I will get this up to date and enable automerge.

It looks like the other Twisted devs are busy or don't care that much about this change.

For me, this is also not very important and I am not very excited about this changes.

Also, this is pushing a new dependency.

But the tests are green, so I think we can merge this.

And if people are saying that this is the future, what can I say ? :)

If someone complains about this change we should be able to revert it.

@adiroiban adiroiban enabled auto-merge October 24, 2022 14:40
@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 24, 2022

Codecov flakes again

@adiroiban adiroiban merged commit 960e26b into twisted:trunk Oct 24, 2022
@ofek ofek deleted the modernize-metadata branch October 24, 2022 15:43
@alex
Copy link
Copy Markdown
Member

alex commented Oct 25, 2022

Hello, I'm one of the maintainers of pyca/cryptography. We run twisted's test suite in our CI.

It recently broke -- somewhere in 4b62da0...8d8f1b0. I think this PR is most likely

https://github.com/pyca/cryptography/actions/runs/3324679422/jobs/5496544314 is the failure we are seeing. Do you have a clue what caused this? Is this an issue on our side, or in twisted?

@adiroiban
Copy link
Copy Markdown
Member

Thanks, @alex for the report.

Most probably this is the reason for the failure.

I see that the zope-interface is recognized and installed

https://github.com/pyca/cryptography/actions/runs/3324679422/jobs/5496544314#step:5:102

I will wait a bit for @ofek to see if there is a quick fix.

But I think that this PR will need a revert.

Thanks again for the report

@glyph
Copy link
Copy Markdown
Member

glyph commented Oct 25, 2022

I'm going to pull this change immediately; no reason to leave trunk broken. If this doesn't fix the issue then we can put it back in, if @ofek can fix it quickly he can fix it in the revert-the-revert PR :)

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 25, 2022

Looking

@glyph
Copy link
Copy Markdown
Member

glyph commented Oct 25, 2022

@ofek you may have a little while as we figure out what to do with the buggy newsfragment check ;-)

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 25, 2022

jazzband/pip-tools#1576

finding link to setuptools issue now

❯ docker run --rm -it python:3.9 bash
root@0fd4d3c43311:/# git clone -q --depth=1 https://github.com/twisted/twisted
root@0fd4d3c43311:/# cd twisted
root@0fd4d3c43311:/twisted# git rev-parse HEAD
8d8f1b070d0baa06a5792969cdfb7ea5707aa622
root@0fd4d3c43311:/twisted# pip install -q ".[all_non_platform]"
WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
WARNING: You are using pip version 22.0.4; however, version 22.3 is available.
You should consider upgrading via the '/usr/local/bin/python -m pip install --upgrade pip' command.
root@0fd4d3c43311:/twisted# python -m pip show setuptools
Name: setuptools
Version: 58.1.0
Summary: Easily download, build, install, upgrade, and uninstall Python packages
Home-page: https://github.com/pypa/setuptools
Author: Python Packaging Authority
Author-email: distutils-sig@python.org
License: UNKNOWN
Location: /usr/local/lib/python3.9/site-packages
Requires:
Required-by: zope.interface
root@0fd4d3c43311:/twisted# python -m twisted.trial src/twisted
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/local/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/usr/local/lib/python3.9/site-packages/twisted/trial/__main__.py", line 9, in <module>
    sys.exit(load_entry_point("Twisted", "console_scripts", "trial")())
  File "/usr/local/lib/python3.9/site-packages/pkg_resources/__init__.py", line 474, in load_entry_point
    return get_distribution(dist).load_entry_point(group, name)
  File "/usr/local/lib/python3.9/site-packages/pkg_resources/__init__.py", line 2846, in load_entry_point
    return ep.load()
  File "/usr/local/lib/python3.9/site-packages/pkg_resources/__init__.py", line 2449, in load
    self.require(*args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/pkg_resources/__init__.py", line 2472, in require
    items = working_set.resolve(reqs, env, installer, extras=self.extras)
  File "/usr/local/lib/python3.9/site-packages/pkg_resources/__init__.py", line 772, in resolve
    raise DistributionNotFound(req, requirers)
pkg_resources.DistributionNotFound: The 'zope-interface>=4.4.2' distribution was not found and is required by the application
root@0fd4d3c43311:/twisted# python -m pip install --upgrade setuptools
Requirement already satisfied: setuptools in /usr/local/lib/python3.9/site-packages (58.1.0)
Collecting setuptools
  Downloading setuptools-65.5.0-py3-none-any.whl (1.2 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.2/1.2 MB 17.1 MB/s eta 0:00:00
Installing collected packages: setuptools
  Attempting uninstall: setuptools
    Found existing installation: setuptools 58.1.0
    Uninstalling setuptools-58.1.0:
      Successfully uninstalled setuptools-58.1.0
Successfully installed setuptools-65.5.0
WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
WARNING: You are using pip version 22.0.4; however, version 22.3 is available.
You should consider upgrading via the '/usr/local/bin/python -m pip install --upgrade pip' command.
root@0fd4d3c43311:/twisted# python -m twisted.trial src/twisted
/usr/local/lib/python3.9/site-packages/twisted/conch/ssh/transport.py:97: CryptographyDeprecationWarning: Blowfish has been deprecated
  b"blowfish-cbc": (algorithms.Blowfish, 16, modes.CBC),
/usr/local/lib/python3.9/site-packages/twisted/conch/ssh/transport.py:101: CryptographyDeprecationWarning: CAST5 has been deprecated
  b"cast128-cbc": (algorithms.CAST5, 16, modes.CBC),
/usr/local/lib/python3.9/site-packages/twisted/conch/ssh/transport.py:106: CryptographyDeprecationWarning: Blowfish has been deprecated
  b"blowfish-ctr": (algorithms.Blowfish, 16, modes.CTR),
/usr/local/lib/python3.9/site-packages/twisted/conch/ssh/transport.py:107: CryptographyDeprecationWarning: CAST5 has been deprecated
  b"cast128-ctr": (algorithms.CAST5, 16, modes.CTR),
twisted._threads.test.test_convenience
  QuitTests
    test_checkAfterSetRaises ...                                           [OK]
    test_checkDoesNothing ...                                              [OK]
    test_isInitiallySet ...                                                [OK]

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 25, 2022

pypa/setuptools#3153

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 25, 2022

https://github.com/twisted/twisted/blob/trunk/src/twisted/trial/__main__.py needs to not use the deprecated pkg_resources but rather https://docs.python.org/3/library/importlib.metadata.html

I'll make a PR

@glyph
Copy link
Copy Markdown
Member

glyph commented Oct 25, 2022

@ofek I've opened a revert-the-revert PR here: #11728

@DMRobertson
Copy link
Copy Markdown
Contributor

I suspect (haven't had chance to confirm) that this change caused us to fail to poetry add twisted from trunk in matrix-org/synapse#14296. Not sure if that was a metadata problem or a poetry problem.

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 26, 2022

Works for me:

❯ docker run --rm -it python:3.9 bash
root@91d9eeaa8351:/# pip install -qqq poetry
root@91d9eeaa8351:/# poetry new foo
Created package foo in foo
root@91d9eeaa8351:/# cd foo
root@91d9eeaa8351:/foo# poetry add -n --extras tls git+https://github.com/twisted/twisted.git#revert-11727-revert-11656-modernize-metadata
Creating virtualenv foo-b2TG5iYf-py3.9 in /root/.cache/pypoetry/virtualenvs

Updating dependencies
Resolving dependencies... (10.7s)

Writing lock file

Package operations: 17 installs, 0 updates, 0 removals

  • Installing pycparser (2.21)
  • Installing cffi (1.15.1)
  • Installing pyasn1 (0.4.8)
  • Installing attrs (22.1.0)
  • Installing cryptography (38.0.1)
  • Installing idna (3.4)
  • Installing pyasn1-modules (0.2.8)
  • Installing six (1.16.0)
  • Installing automat (20.2.0)
  • Installing constantly (15.1.0)
  • Installing hyperlink (21.0.0)
  • Installing incremental (22.10.0)
  • Installing pyopenssl (22.1.0)
  • Installing service-identity (21.1.0)
  • Installing typing-extensions (4.4.0)
  • Installing zope-interface (5.5.0)
  • Installing twisted (22.8.0.post0 8dddece)

@DMRobertson
Copy link
Copy Markdown
Contributor

Hmm, odd. Maybe there's some unusual interaction between Twisted's dependencies and poetry's dependencies which was breaking for us? Thanks for trying: if it pops up again I'll investigate more and file a proper report.

@ofek
Copy link
Copy Markdown
Contributor Author

ofek commented Oct 26, 2022

was a Poetry thing matrix-org/synapse#14296 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update package metadata

10 participants