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.

Sign up for GitHub

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

Jump to bottom

Enable universal wheel build on CI #883

Closed
wants to merge 4 commits into from
Closed

Enable universal wheel build on CI #883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d2eb136
Fix issue in MULTIDICT_NO_EXTENSIONS handling
jku Sep 14, 2023
47d6345
ci-cd: Enable building a pure Python wheel
jku Sep 14, 2023
8226019
CHANGES.rst: Update URL
jku Sep 14, 2023
8e244e2
Add NEWS entry for 881
jku Sep 14, 2023
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
6 changes: 5 additions & 1 deletion .github/workflows/ci-cd.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,11 @@ jobs:
matrix:
os: [ubuntu, windows, macos]
qemu: ['']
noextensions: ['0']
include:
# Split ubuntu job for the sake of speed-up
# Add a pure python build and qemu builds
- os: ubuntu
noextensions: '1'
Copy link
Member

Choose a reason for hiding this comment

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

This is not the right places to add it. It would essentially run the same wheel creation under each Python version + waste a CI job. It can be done together with the sdist. Although, that should also be updated as it still invokes setup.py.

I'm doing a round of maintenance updates across the repos, including packaging and CI changes. frozenlist and yarl are updated, multidict is next on my list. So I'll take care of this in a standardized way.

- os: ubuntu
qemu: aarch64
- os: ubuntu
Expand Down Expand Up @@ -202,6 +205,7 @@ jobs:
uses: pypa/cibuildwheel@v2.16.1
env:
CIBW_ARCHS_MACOS: x86_64 arm64 universal2
CIBW_ENVIRONMENT: MULTIDICT_NO_EXTENSIONS=${{ matrix.noextensions }}
- uses: actions/upload-artifact@v3
with:
name: dist
Expand Down
2 changes: 1 addition & 1 deletion CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Changelog
file is managed by towncrier. You *may* edit previous change logs to
fix problems like typo corrections or such.
To add a new change log entry, please see
https://pip.pypa.io/en/latest/development/#adding-a-news-entry
https://pip.pypa.io/en/latest/development/contributing/#news-entries
Copy link
Member

Choose a reason for hiding this comment

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

This update could be useful as a separate PR.

we named the news folder "changes".

WARNING: Don't drop the next directive!
Expand Down
1 change: 1 addition & 0 deletions CHANGES/881.feature.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Enable publishing a universal (pure Python) wheel on PyPI
Copy link
Member

Choose a reason for hiding this comment

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

The change notes should use the past tense with the end-users as the target audience in mind. They should include full sentences with periods et al.
Also, "universal" is an incorrect term — conventionally, wheels tagged as py2.py3 used to be called universal. But we should only have py3 in the tag.

2 changes: 1 addition & 1 deletion multidict/_compat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import os
import platform

NO_EXTENSIONS = bool(os.environ.get("MULTIDICT_NO_EXTENSIONS"))
NO_EXTENSIONS = os.environ.get("MULTIDICT_NO_EXTENSIONS", "0") == "1"
Copy link
Member

Choose a reason for hiding this comment

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

This is a semantic change that isn't even mentioned in the changelog. It's undesired at this point. But if we were to change it, this would mean a feature PR, made with more care.


PYPY = platform.python_implementation() == "PyPy"

Expand Down
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from setuptools import Extension, setup

NO_EXTENSIONS = bool(os.environ.get("MULTIDICT_NO_EXTENSIONS"))
NO_EXTENSIONS = os.environ.get("MULTIDICT_NO_EXTENSIONS", "0") == "1"
Copy link
Member

Choose a reason for hiding this comment

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

Same as the previous comment. Although, it doesn't matter much now, since I'll align the packaging with frozenlist and yarl soon.


if sys.implementation.name != "cpython":
NO_EXTENSIONS = True
Expand Down
Loading