-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
setup.cfg: include llhttp LICENSE in license-files #11226
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
Conversation
cc7660d
to
705cea6
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #11226 +/- ##
=======================================
Coverage 98.76% 98.76%
=======================================
Files 129 129
Lines 43374 43374
Branches 2323 2323
=======================================
Hits 42837 42837
Misses 383 383
Partials 154 154
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
CodSpeed Performance ReportMerging #11226 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how to best handle this.
There's a way to build a pure-python wheel that doesn't link against llhttp and so that would be misleading.
At the Packaging Summit, one of the round table discussions tackled specifically what the semantics of pointing to various licenses is.
My understanding is that there's a plan to have a clarifying PEP post PEP 639.
cc @befeleme @CAM-Gerlach could you chime in on how to declare licences for occasionally vendored deps?
In supershort, within the boundaries of PEP 639, there are two questions:
|
I believe that sdist contains the source code of vendored
There's a request to ship an additional pure-python wheel that wouldn't contain the C-extensions and the vendored llhttp wouldn't be bundled either. We're planning to implement it as it's a small change to the CI. |
c347662
to
816f470
Compare
I've pushed a new version of the commit that should incorporate all suggestions. |
c786e21
to
57652d5
Compare
57652d5
to
fce1511
Compare
I've rebased the branch. Is there a preferred way to move forward with this? |
This is probably fine but let me try to ask options today at the packaging summit if I don't forget.. |
fce1511
to
58cd5f0
Compare
Make sure that the llhttp LICENSE file is included in the wheel by modifying setup.cfg's license-files field to be a list according to the Python Packaging User Guide. Also specify MIT in the 'license' field since llhttp is MIT and not Apache-2.0. Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]> Signed-off-by: Trevor Gamblin <[email protected]>
58cd5f0
to
7a04f39
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've shown this to Karolina at EuroPython the other day, which is where the SPDX point came from. Now that it's integrated, this is good to merge. We can try to deal with the pure-python wheel corner case separately.
@threexc I recommend you to always accept suggested changes via GH UI and then rewrite commits locally. This makes sure that you can credit additional authors correctly: https://hynek.me/til/easier-crediting-contributors-github/. |
Backport to 3.12: 💚 backport PR created✅ Backport PR branch: Backported as #11328 🤖 @patchback |
aiohttp vendors llhttp in its source distributions, and it also bundles it as a part for platform-specific wheels. Previously, this was not exposed in the core packaging metadata. With this patch, now it is. The change includes both the license file from the vendored project and adds it to the SPDX expression following PEP 639. This is configured through the `setup.cfg` config for the `setuptools` build backend. PR #11226 Co-Authored-By: Karolina Surma <[email protected]> Co-Authored-By: 🇺🇦 Sviatoslav Sydorenko <[email protected]> (cherry picked from commit 8afdc4d)
Backport to 3.13: 💚 backport PR created✅ Backport PR branch: Backported as #11329 🤖 @patchback |
aiohttp vendors llhttp in its source distributions, and it also bundles it as a part for platform-specific wheels. Previously, this was not exposed in the core packaging metadata. With this patch, now it is. The change includes both the license file from the vendored project and adds it to the SPDX expression following PEP 639. This is configured through the `setup.cfg` config for the `setuptools` build backend. PR #11226 Co-Authored-By: Karolina Surma <[email protected]> Co-Authored-By: 🇺🇦 Sviatoslav Sydorenko <[email protected]> (cherry picked from commit 8afdc4d)
Thanks, will do that from now on. |
…ckaging metadata Reflect llhttp license in core packaging metadata aiohttp vendors llhttp in its source distributions, and it also bundles it as a part for platform-specific wheels. Previously, this was not exposed in the core packaging metadata. With this patch, now it is. The change includes both the license file from the vendored project and adds it to the SPDX expression following PEP 639. This is configured through the `setup.cfg` config for the `setuptools` build backend. PR #11226 Co-Authored-By: Karolina Surma <[email protected]> Co-Authored-By: 🇺🇦 Sviatoslav Sydorenko <[email protected]>
…ckaging metadata Reflect llhttp license in core packaging metadata aiohttp vendors llhttp in its source distributions, and it also bundles it as a part for platform-specific wheels. Previously, this was not exposed in the core packaging metadata. With this patch, now it is. The change includes both the license file from the vendored project and adds it to the SPDX expression following PEP 639. This is configured through the `setup.cfg` config for the `setuptools` build backend. PR #11226 Co-Authored-By: Karolina Surma <[email protected]> Co-Authored-By: 🇺🇦 Sviatoslav Sydorenko <[email protected]>
What do these changes do?
Make sure that the
llhttp
LICENSE
file is included in the wheel by modifyingsetup.cfg
's license-files field to be a list according to the Python Packaging User Guide.Are there changes in behavior for the user?
No.
Is it a substantial burden for the maintainers to support this?
The main difference is that the maintainers will have to further update the
license-files
field insetup.cfg
(or possibly modifypyproject.toml
if that config content moves there) shouldllhttp
's LICENSE file change name, etc.Related issue number
Fixes: #11225
Checklist
CONTRIBUTORS.txt
CHANGES/
folder<issue_or_pr_num>.<type>.rst
(e.g.588.bugfix.rst
)number after creating the PR
.bugfix
: A bug fix for something the maintainers deemed animproper undesired behavior that got corrected to match
pre-agreed expectations.
.feature
: A new behavior, public APIs. That sort of stuff..deprecation
: A declaration of future API removals and breakingchanges in behavior.
.breaking
: When something public is removed in a breaking way.Could be deprecated in an earlier release.
.doc
: Notable updates to the documentation structure or buildprocess.
.packaging
: Notes for downstreams about unobvious side effectsand tooling. Changes in the test invocation considerations and
runtime assumptions.
.contrib
: Stuff that affects the contributor experience. e.g.Running tests, building the docs, setting up the development
environment.
.misc
: Changes that are hard to assign to any of the abovecategories.
for example:
Testing:
From master branch (building with
python3 -m build
in a venv):and from my patch branch: