-
Notifications
You must be signed in to change notification settings - Fork 51
Wheel building & PyPI publishing #109
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
base: main
Are you sure you want to change the base?
Conversation
- Add build-and-publish.yml workflow for multi-platform wheel building using cibuildwheel - Add release.yml workflow for automatic GitHub release creation - Update pyproject.toml with cibuildwheel configuration - Add release.py script for version management and tagging - Add RELEASE.md documentation for the release process Features: - Builds wheels for Linux and macOS across Python 3.8-3.13 - Publishes to TestPyPI on main branch pushes - Publishes to PyPI on GitHub releases - Uses trusted publishing for secure authentication
pyproject.toml
Outdated
|
|
||
| [tool.cibuildwheel.macos] | ||
| before-all = ["brew install macfuse pkg-config"] | ||
| environment = { PKG_CONFIG_PATH = "/usr/local/lib/pkgconfig:/opt/homebrew/lib/pkgconfig" } |
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.
add an LF here?
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.
done.
RELEASE.md
Outdated
| ### 1. Build and Publish (`build-and-publish.yml`) | ||
|
|
||
| This workflow runs on: | ||
| - Push to `main` branch → Publishes to TestPyPI |
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.
does that also run when merging a PR?
if so, it would publish potentially a lot on TestPyPI - do they discard stuff automatically after a while?
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.
yes, i removed this so it will only release packages when you publish a release (pre-release goes to test pypi, official releases go to pypi)
|
I think we should offer wheels for all popular and supported/supportable platforms, including macOS. Yeah, the macOS runners are slow, guess they are overloaded and microsoft can't afford more hardware from apple. |
i just mean for your wallet, macos runners are more expensive that the linux ones. i'll leave them in for now and address your issues. |
|
@mlapaglia For FOSS repos, the runners are free. To minimize load on the macOS runners (which are overloaded and slow already), I configured test runs in some of my repos so that the macOS runs depend on the linux runs being successful. Not sure how it is best done for wheel building though. |
|
if you need any other changes lmk ❤️ |
|
Apologies for the long delay here. This is useful functionality and I have some time for this over the next 7 days, so hopefully we can get this sorted. Could you clarify if any AI coding agents were used to create this branch, and if so document which agent was used and to which extent you have reviewed and tested the code yourself? It'd be great if you could also rebase this onto main and resolve the conflicts. |
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 think this should be integrated into developer-notes/release_process.rst instead.
| - name: Build wheels | ||
| run: python -m cibuildwheel --output-dir wheelhouse | ||
| env: | ||
| CIBW_BUILD: cp38-* cp39-* cp310-* cp311-* cp312-* cp313-* |
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.
Please remove end-of-life distributions.
| env: | ||
| CIBW_BUILD: cp38-* cp39-* cp310-* cp311-* cp312-* cp313-* | ||
|
|
||
| CIBW_SKIP: "*-win32 *-manylinux_i686 *-musllinux*" |
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.
Why skip manylinux?
| name: pypi | ||
| url: https://pypi.org/p/pyfuse3 | ||
| permissions: | ||
| id-token: write |
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.
Could you clarify how the permissioning works? Do we need to get a token from PyPi and store it somewhere where Github can access it?
| # Extract changelog for this version from Changes.rst | ||
| if [ -f "Changes.rst" ]; then | ||
| # Try to extract the section for this version | ||
| awk '/^pyfuse3 '"${{ steps.version.outputs.version }}"'/{flag=1; next} /^pyfuse3 [0-9]/{flag=0} flag' Changes.rst > changelog.txt |
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.
Could you add some output to the PR that illustrates that this parsing has been tested?
Also, can we make the job fail if parsing fails?
| strict = true | ||
|
|
||
| [tool.cibuildwheel] | ||
| build = "cp38-* cp39-* cp310-* cp311-* cp312-* cp313-*" |
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.
This seems to repeat code from the Github workflow.
| import subprocess | ||
| import sys | ||
| from pathlib import Path | ||
|
|
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 think it'd be better to extract the version from the Git tags (since we're effectively treating them as the new source of truth now).
| with open(setup_py) as f: | ||
| content = f.read() | ||
|
|
||
| match = re.search(r"PYFUSE3_VERSION = ['\"]([^'\"]+)['\"]", content) |
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.
The PYFUSE3_VERSION variable should probably be removed, and instead we should use dynamic versioning (https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html)
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.
As far as I can tell, this script just reads and updates the version number in .py files and then creates a git tag. So if we instead use git tags as the version source, I think this script can be removed completely.
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.
This feels like an unrelated change that should go into a separate PR.
Add CI/CD workflows for automated wheel building and PyPI publishing
Notes: