Move metadata to pyproject.toml

[cdce8p]

What do these changes do?

The metadata changes:

 ...
-Maintainer: aiohttp team <[email protected]>
-Maintainer-email: [email protected]
+Maintainer-email: aiohttp team <[email protected]>
 License: Apache-2.0
-Home-page: https://github.com/aio-libs/aiohttp
+Project-URL: Homepage, https://github.com/aio-libs/aiohttp
 ...

Open questions

• Should this PR be backported? It's possible, might just require some manual work due to merge conflicts.

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 98.74%. Comparing base (e260b15) to head (a544158). :warning: Report is 3 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #9951   +/-   ##
=======================================
  Coverage   98.74%   98.74%           
=======================================
  Files         127      127           
  Lines       43448    43448           
  Branches     2326     2326           
=======================================
  Hits        42903    42903           
  Misses        389      389           
  Partials      156      156           

Flag	Coverage Δ
CI-GHA	98.63% <ø> (ø)
OS-Linux	98.36% <ø> (ø)
OS-Windows	96.69% <ø> (-0.01%)	:arrow_down:
OS-macOS	97.58% <ø> (ø)
Py-3.10.11	97.14% <ø> (ø)
Py-3.10.18	97.64% <ø> (ø)
Py-3.11.13	97.84% <ø> (+<0.01%)	:arrow_up:
Py-3.11.9	97.35% <ø> (ø)
Py-3.12.10	97.44% <ø> (-0.01%)	:arrow_down:
Py-3.12.11	97.94% <ø> (-0.01%)	:arrow_down:
Py-3.13.7	98.20% <ø> (-0.01%)	:arrow_down:
Py-3.14.0	98.15% <ø> (-0.01%)	:arrow_down:
Py-3.14.0t	97.22% <ø> (+<0.01%)	:arrow_up:
Py-pypy3.10.16-7.3.19	89.46% <ø> (-6.92%)	:arrow_down:
VM-macos	97.58% <ø> (ø)
VM-ubuntu	98.36% <ø> (ø)
VM-windows	96.69% <ø> (-0.01%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[codspeed-hq[bot]]

CodSpeed Performance Report

Merging #9951 will not alter performance

_{Comparing cdce8p:move-metadata (a544158) with master (62debeb)[^unexpected-base]} [^unexpected-base]: No successful run was found on master (e260b15) during the generation of this report, so 62debeb was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Summary

✅ 59 untouched

[webknjaz]

Let's not rush this. Keeping a dedicated config is a conscious choice. Also, PRs must not reformat half a project along with changing the packaging configs.

FWIW I'd rather keep it as is for now. Especially, since I was going to sync the configs across the projects and this is going to cause a lot of headache since it conflicts with how things are now.

I'm 👎 on this migration right now. It'd be interesting to gather the current dependency limitations in the downstreams to make an informed choice. Additionally, the move would have to happen in the dependencies first.

[cdce8p]

Let's not rush this. Keeping a dedicated config is a conscious choice. Also, PRs must not reformat half a project along with changing the packaging configs.

That's actually a bug with the current black config. Since it can't detect the lowest supported version, it assumes py33 and doesn't use the correct style. In https://github.com/aio-libs/aiohttp/pull/9951/commits/f718fa14a30d9c8837eeee341344a0cd04ef0038 I've pinned the lowest version which doesn't cause any reformatting.

FWIW I'd rather keep it as is for now. Especially, since I was going to sync the configs across the projects and this is going to cause a lot of headache since it conflicts with how things are now.

I'm 👎 on this migration right now. It'd be interesting to gather the current dependency limitations in the downstreams to make an informed choice. Additionally, the move would have to happen in the dependencies first.

I'm probably missing something here. This PR doesn't affect any downstream projects. The project metadata change is minimal and spec compliant.

Tbh I just thought it might be a good time to move the static metadata after I learned that it can work fine with setup.py. It will probably still take a while (couple of years) but I expect setuptools to eventually deprecate the setup.cfg config.

There is no strict need to do that now, so if you're opposed feel free to close the PR.

[asvetlov]

That's actually a bug with the current black config.

@cdce8p could you please extract a fix for this particular problem into a separate PR? A small patch has a great chance to be reviewed and approved quickly.

[asvetlov]

Ooh, sorry. I've pushed the wrong button and closed the PR. Please accept my apologizes.

[cdce8p]

That's actually a bug with the current black config.

@cdce8p could you please extract a fix for this particular problem into a separate PR? A small patch has a great chance to be reviewed and approved quickly.

@asvetlov See #9962. It looks bigger than it actually is.

[asvetlov]

Thanks, @cdce8p The PR is merged and back ported

[cdce8p]

Rebased the PR. Also moved the dependencies and optional-dependencies back to setup.cfg to purely focus on the static metadata. Please let me know if you're interested in moving this forward. If not or not at this time, feel free to close it.

[webknjaz]

Rebased the PR. Also moved the dependencies and optional-dependencies back to setup.cfg to purely focus on the static metadata. Please let me know if you're interested in moving this forward. If not or not at this time, feel free to close it.

Not sure if this would work well with setuptools. I think when it detects PEP 621 mode, it would only source the spec-compliant bits from pyproject.toml. There might be some backend configs that would be sourced from other places, but anything that's not listed as dynamic explicitly is likely to be disregarded or otherwise cause breakage. So with this move we'll need to migrate things atomically.

[webknjaz]

I'm 👎 on this migration right now. It'd be interesting to gather the current dependency limitations in the downstreams to make an informed choice. Additionally, the move would have to happen in the dependencies first.

I'm probably missing something here. This PR doesn't affect any downstream projects. The project metadata change is minimal and spec compliant.

Tbh I just thought it might be a good time to move the static metadata after I learned that it can work fine with setup.py. It will probably still take a while (couple of years) but I expect setuptools to eventually deprecate the setup.cfg config.

What I meant back then was that I'd like the move to first happen in projects that are direct deps of aiohttp (like yarl). This is because I've implemented some advanced packaging techniques there (like an in-tree build backend). I'm going to eventually migrate aiohttp to use the same model. So packaging updates would flow from those places where improvements were first implemented.

Upon reflection, thought, I think that this PR doesn't block that so you're fine. Talking about the downstreams, they've been historically behind some upstream packaging standards, but AFAIK they've mostly implemented their own PEP 517 helpers by now. Still, there might be problems where the setuptools version they ship is older than our spec (it'd conflict, forcing them to patch it out on their side). Also, if they patch it out, there's a possibility that it's older than PEP 621 effectively not producing most of the metadata. So we should at least be careful about https://github.com/aio-libs/aiohttp/pull/9951/files#r2066217591.

[cdce8p]

That's for the review @webknjaz! I'll address all comments in one to keep it readable. Please let me know if you have any followup questions.

Move dependencies to pyproject.toml

Sounds good. I'd just recommend to do that in a followup PR. Otherwise the backports PRs will likely not apply cleanly. In the end it's just a revert of https://github.com/aio-libs/aiohttp/pull/9951/commits/ec9022547a07f710a83a5c3e172884ccc82632bb

With dependencies and optional-dependencies marked as dynamic, they are still read from setup.cfg as well. Checked the generated METADATA file again just to be sure.

[tool.setuptools.packages.find]
include = ["aiohttp*"]

This will probably happen automatically.

Unfortunately not. Setuptools will throw an error Multiple top-level packages discovered in a flat-layout: ['vendor', 'aiohttp', 'CHANGES']. We can get rid of exclude = ["examples"] though.

zip_safe = False

This is no longer necessary?

Yes. From the setuptools docs

Obsolete - only relevant for pkg_resources, easy_install and setup.py install in the context of eggs (deprecated).

https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html#setuptools-specific-configuration

package-data and exclude-package-data

I'm not entirely sure if these are still needed. Might need some manual checks.

package-data can indeed be removed. The default for pyproject.toml configs is include-package-data = true. We still need exclude-package-data though. Otherwise the wheels would include *.c and *.h files. I plan to do a followup to optimize the sdist / wheel sizes a bit by extending the list further. E.g. we still include the raw cython *.pyx files in the wheels which probably aren't necessary.

Edit: Pushed one more commit to do directly: https://github.com/aio-libs/aiohttp/pull/9951/commits/f06239c7ab59d1f774f729030c6f130b7659d189. Mainly just to ignore the generated *.hash files. The global-include aiohttp *.pyi instruction was redundant as everything inside aiohttp and .pyi files are included by setuptools automatically.

What's the specific feature that requires at least v70?

Double checked that. It's actually >= 66.1 that's required, so I'd suggest >= 67.0. Previous versions relied on pkgutil.ImpImporter which isn't available on Python 3.12+.

https://setuptools.pypa.io/en/latest/history.html#v66-1-0

Still, there might be problems where the setuptools version they ship is older than our spec

Yes, I've seen a few reports in the light of the PEP 639 adoption. Support for it was only added in v77. I believe most (?) distributors still use an older version with no build isolation so that caused some confusion. That's one of the reasons I haven't updated the metadata here yet. v67.0 should be fine.

[cdce8p]

Pushed a new commit to incorporate the license metadata changes from #11226.

@webknjaz Would you mind taking a look at this again? I believe I've addressed all your comments in https://github.com/aio-libs/aiohttp/pull/9951#issuecomment-2841603641. Please let me know if you have any more questions.

[webknjaz]

I believe I've addressed all your comments in #9951 (comment). Please let me know if you have any more questions.

All other threads need to be addressed as well. There's a number of those with zero response.

[cdce8p]

I believe I've addressed all your comments in #9951 (comment). Please let me know if you have any more questions.

All other threads need to be addressed as well. There's a number of those with zero response.

I've addressed them in https://github.com/aio-libs/aiohttp/pull/9951#issuecomment-2841603641 as I personally find the Github code comments hard to follow when it's a discussion about one or more related topics.

[webknjaz]

I believe I've addressed all your comments in #9951 (comment).

That's why I missed it... Inline comments are attached to context they discuss. Dumping a bunch of text in an unrelated location make it more difficult to match with the actual diff. It's also something that makes it impossible to review on mobile. I'll have to find time to open on the computer and then I'll be able to actually read it in context with an extra window on screen.

[webknjaz]

@cdce8p short of a few cosmetic requests, this seems almost ready.

[cdce8p]

@webknjaz Pushed another update to address the style changes and also responded to your questions. Please let me know if I missed anything.

[cdce8p]

@webknjaz I just resolved the conflicts. Would love get get this over the finish line, especially as there isn't much left to do here.

[patchback[bot]]

Backport to 3.13: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 6b5d8e7f6b92fe944930b12e16d4884d01d14323 on top of patchback/backports/3.13/6b5d8e7f6b92fe944930b12e16d4884d01d14323/pr-9951

Backporting merged PR #9951 into master

1. Ensure you have a local repo clone of your fork. Unless you cloned it from the upstream, this would be your origin remote.
2. Make sure you have an upstream repo added as a remote too. In these instructions you'll refer to it by the name upstream. If you don't have it, here's how you can add it:

   $ git remote add upstream https://github.com/aio-libs/aiohttp.git
   

3. Ensure you have the latest copy of upstream and prepare a branch that will hold the backported code:

   $ git fetch upstream
   $ git checkout -b patchback/backports/3.13/6b5d8e7f6b92fe944930b12e16d4884d01d14323/pr-9951 upstream/3.13
   

4. Now, cherry-pick PR #9951 contents into that branch:

   $ git cherry-pick -x 6b5d8e7f6b92fe944930b12e16d4884d01d14323
   

   If it'll yell at you with something like fatal: Commit 6b5d8e7f6b92fe944930b12e16d4884d01d14323 is a merge but no -m option was given., add -m 1 as follows instead:

   $ git cherry-pick -m1 -x 6b5d8e7f6b92fe944930b12e16d4884d01d14323
   

5. At this point, you'll probably encounter some merge conflicts. You must resolve them in to preserve the patch from PR #9951 as close to the original as possible.
6. Push this branch to your fork on GitHub:

   $ git push origin patchback/backports/3.13/6b5d8e7f6b92fe944930b12e16d4884d01d14323/pr-9951
   

7. Create a PR, ensure that the CI is green. If it's not — update it so that the tests and any other checks pass. This is it! Now relax and wait for the maintainers to process your pull request when they have some cycles to do reviews. Don't worry — they'll tell you if any improvements are necessary when the time comes!

🤖 @patchback I'm built with octomachinery and my source is open — https://github.com/sanitizers/patchback-github-app.

[patchback[bot]]

Backport to 3.14: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 6b5d8e7f6b92fe944930b12e16d4884d01d14323 on top of patchback/backports/3.14/6b5d8e7f6b92fe944930b12e16d4884d01d14323/pr-9951

Backporting merged PR #9951 into master

1. Ensure you have a local repo clone of your fork. Unless you cloned it from the upstream, this would be your origin remote.
2. Make sure you have an upstream repo added as a remote too. In these instructions you'll refer to it by the name upstream. If you don't have it, here's how you can add it:

   $ git remote add upstream https://github.com/aio-libs/aiohttp.git
   

3. Ensure you have the latest copy of upstream and prepare a branch that will hold the backported code:

   $ git fetch upstream
   $ git checkout -b patchback/backports/3.14/6b5d8e7f6b92fe944930b12e16d4884d01d14323/pr-9951 upstream/3.14
   

4. Now, cherry-pick PR #9951 contents into that branch:

   $ git cherry-pick -x 6b5d8e7f6b92fe944930b12e16d4884d01d14323
   

   If it'll yell at you with something like fatal: Commit 6b5d8e7f6b92fe944930b12e16d4884d01d14323 is a merge but no -m option was given., add -m 1 as follows instead:

   $ git cherry-pick -m1 -x 6b5d8e7f6b92fe944930b12e16d4884d01d14323
   

5. At this point, you'll probably encounter some merge conflicts. You must resolve them in to preserve the patch from PR #9951 as close to the original as possible.
6. Push this branch to your fork on GitHub:

   $ git push origin patchback/backports/3.14/6b5d8e7f6b92fe944930b12e16d4884d01d14323/pr-9951
   

7. Create a PR, ensure that the CI is green. If it's not — update it so that the tests and any other checks pass. This is it! Now relax and wait for the maintainers to process your pull request when they have some cycles to do reviews. Don't worry — they'll tell you if any improvements are necessary when the time comes!

🤖 @patchback I'm built with octomachinery and my source is open — https://github.com/sanitizers/patchback-github-app.

[cdce8p]

Thanks @webknjaz! I'll work on the backports and open a separate PR to move the dependencies as we talked about.

[webknjaz]

Thank you!