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.

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

chore: update cibuildwheel #2040

Merged
merged 3 commits into from Oct 18, 2022
Merged

Conversation

mayeut
Copy link
Contributor

@mayeut mayeut commented Dec 25, 2021

Summary

Description

This commit updates the build workflow to use the latest cibuildwheel as a GitHub Action.
cibuildwheel configuration is now in its own file (as there's no pyproject.toml yet)

Signed-off-by: mayeut mayeut@users.noreply.github.com

Diff of wheels produced by GHA:

+psutil-5.9.0-cp27-cp27m-macosx_10_9_x86_64.whl
 psutil-5.9.0-cp27-cp27m-manylinux2010_i686.whl
 psutil-5.9.0-cp27-cp27m-manylinux2010_x86_64.whl
 psutil-5.9.0-cp27-cp27mu-manylinux2010_i686.whl
 psutil-5.9.0-cp27-cp27mu-manylinux2010_x86_64.whl
 psutil-5.9.0-cp36-cp36m-macosx_10_9_x86_64.whl
 psutil-5.9.0-cp36-cp36m-manylinux_2_12_i686.manylinux2010_i686.manylinux_2_17_i686.manylinux2014_i686.whl
 psutil-5.9.0-cp36-cp36m-manylinux_2_12_x86_64.manylinux2010_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl
 psutil-5.9.0-cp37-cp37m-macosx_10_9_x86_64.whl
 psutil-5.9.0-cp37-cp37m-manylinux_2_12_i686.manylinux2010_i686.manylinux_2_17_i686.manylinux2014_i686.whl
 psutil-5.9.0-cp37-cp37m-manylinux_2_12_x86_64.manylinux2010_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl
+psutil-5.9.0-cp38-cp38-macosx_11_0_arm64.whl
 psutil-5.9.0-cp38-cp38-macosx_10_9_x86_64.whl
 psutil-5.9.0-cp38-cp38-manylinux_2_12_i686.manylinux2010_i686.manylinux_2_17_i686.manylinux2014_i686.whl
 psutil-5.9.0-cp38-cp38-manylinux_2_12_x86_64.manylinux2010_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl
+psutil-5.9.0-cp39-cp39-macosx_11_0_arm64.whl
 psutil-5.9.0-cp39-cp39-macosx_10_9_x86_64.whl
 psutil-5.9.0-cp39-cp39-manylinux_2_12_i686.manylinux2010_i686.manylinux_2_17_i686.manylinux2014_i686.whl
 psutil-5.9.0-cp39-cp39-manylinux_2_12_x86_64.manylinux2010_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl
+psutil-5.9.0-cp310-cp310-macosx_11_0_arm64.whl
 psutil-5.9.0-cp310-cp310-macosx_10_9_x86_64.whl
 psutil-5.9.0-cp310-cp310-manylinux_2_12_i686.manylinux2010_i686.manylinux_2_17_i686.manylinux2014_i686.whl
 psutil-5.9.0-cp310-cp310-manylinux_2_12_x86_64.manylinux2010_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl

It's worth reminding that the macOS arm64 wheels can't be tested on GitHub Actions

@mayeut mayeut force-pushed the cibuildwheel-update branch 2 times, most recently from bc3c582 to 4eb8276 Compare April 16, 2022 08:59
@mayeut mayeut force-pushed the cibuildwheel-update branch 4 times, most recently from eacb737 to 44b986c Compare June 4, 2022 23:15
@mayeut mayeut force-pushed the cibuildwheel-update branch 2 times, most recently from 8172e31 to 08722f2 Compare August 28, 2022 07:26
@mayeut mayeut force-pushed the cibuildwheel-update branch 2 times, most recently from 858b688 to 48bcc39 Compare September 6, 2022 21:35
@mayeut mayeut force-pushed the cibuildwheel-update branch 6 times, most recently from 581d2c7 to 1ec85ee Compare September 22, 2022 18:26
@mayeut mayeut force-pushed the cibuildwheel-update branch 2 times, most recently from 118b244 to cb7fdf4 Compare October 6, 2022 17:32
@mayeut
Copy link
Contributor Author

mayeut commented Oct 8, 2022

@giampaolo,

in #2103, you said:

The most pressing tickets at the moment appears to be about people unable to install psutil on macOS Big Sur, so I would welcome a PR for that only, if you can (I believe that is #2040?).

I've cleaned that up a bit & could verify that together with #2153 & #2146, all tests are passing when run on cirrus CI which offers macOS arm64 runners: https://cirrus-ci.com/build/5870591818334208

]

[tool.cibuildwheel.macos]
archs = ["x86_64", "arm64"]
Copy link
Owner

Choose a reason for hiding this comment

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

Can we do this without using an additional file? I like having the test runner defined in the main file.
If we can't avoid this, at the very least can we put this file under .github/workflows/ directory instead of the root directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we do this without using an additional file?

It could be done but I'd rather not.
cibuildwheel is more than just a CI tool & as such, I see its config file living at project root just like the ones from flake8/isort/coverage.
It's more & more common for cibuildwheel users to put there config in pyproject.toml and be able to just run cibuildwheel, whatever the CI provider, or run locally (which I use extensively) to reproduce the steps that would happen on CI.
Running locally, I can just do cibuildwheel --platform macos --config-file cibuildwheel.toml rather than remembering/typing all of the CI config from the GitHub workflow in my console.
This file is CI provider independent so if it were to be kept & moved, would you be okay to move it to .ci/ ?
Another option if you're open to it would be to add a pyproject.toml file with cibuildwheel config inside. It would simplify the workflow even more (& running locally) by removing the need to specify a config file:

[build-system]
requires = ["setuptools>=43"]
build-backend = "setuptools.build_meta"

[tool.cibuildwheel]
skip = ["pp*", "*-musllinux*"]
test-extras = "test"
test-command = [
    "PYTHONWARNINGS=always PYTHONUNBUFFERED=1 PSUTIL_DEBUG=1 python {project}/psutil/tests/runner.py",
    "PYTHONWARNINGS=always PYTHONUNBUFFERED=1 PSUTIL_DEBUG=1 python {project}/psutil/tests/test_memleaks.py"
]

[tool.cibuildwheel.macos]
archs = ["x86_64", "arm64"]

Copy link
Owner

Choose a reason for hiding this comment

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

Another option if you're open to it would be to add a pyproject.toml file with cibuildwheel config inside.

Interesting. I had no idea. I think this is the best course, also because it seems natural to move flake8 / autopep8, isort and coveragerc configs in there as well as a next step.

CIBW_TEST_COMMAND:
PYTHONWARNINGS=always PYTHONUNBUFFERED=1 PSUTIL_DEBUG=1 python {project}/psutil/tests/runner.py &&
PYTHONWARNINGS=always PYTHONUNBUFFERED=1 PSUTIL_DEBUG=1 python {project}/psutil/tests/test_memleaks.py
CIBW_TEST_EXTRAS: test
CIBW_BUILD: 'cp27-*'
CIBW_SKIP: '*-musllinux_*'
Copy link
Owner

Choose a reason for hiding this comment

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

Please let's keep this PR related to macOS only. There is another PR related to Musl Linux and I haven't decided yet on whether to provide wheels for that platform.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cibuildwheel 1.12.0 will never build musllinux & there are no images for musllinux cp27.
I reverted the change adding cp 3.5 on linux.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove cp 3.11 as well.

@mayeut mayeut force-pushed the cibuildwheel-update branch 2 times, most recently from 39b9ad5 to c00a732 Compare October 10, 2022 19:41
Copy link
Contributor

@ben9923 ben9923 left a comment

Choose a reason for hiding this comment

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

I like the pyproject.toml approach :)
Figured I'd comment as I've also looked at some CI/build improvements for psutil.

- name: Run tests
run: cibuildwheel .
uses: pypa/cibuildwheel@v2.10.2
Copy link
Contributor

Choose a reason for hiding this comment

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

You can also use this action (@v1.12.0) for the Python 2 job to make it cleaner :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forgot the action was already available in 1.12.0
Updated.

.github/workflows/build.yml Outdated Show resolved Hide resolved
This commit updates the build workflow to use the latest cibuildwheel as a GitHub Action.
cibuildwheel configuration is now in its own file (as there's no `pyproject.toml` yet)

Signed-off-by: mayeut <mayeut@users.noreply.github.com>
Signed-off-by: mayeut <mayeut@users.noreply.github.com>
Signed-off-by: mayeut <mayeut@users.noreply.github.com>
@sweh
Copy link

sweh commented Oct 18, 2022

Now that the pyproject.toml is here, what are the chances that it will be merged in the near future?

@giampaolo giampaolo merged commit 3f81c62 into giampaolo:master Oct 18, 2022
@giampaolo
Copy link
Owner

(finally) merged. Thanks.

giampaolo added a commit that referenced this pull request Oct 18, 2022
...since pyproject.toml was introduced in #2040. CC @mayeut

Signed-off-by: Giampaolo Rodola <g.rodola@gmail.com>
giampaolo added a commit that referenced this pull request Oct 18, 2022
…upporter)

Signed-off-by: Giampaolo Rodola <g.rodola@gmail.com>
@mayeut mayeut deleted the cibuildwheel-update branch October 20, 2022 23:18
ddelange added a commit to ddelange/psutil that referenced this pull request Oct 28, 2022
* 'master' of https://github.com/giampaolo/psutil:
  add windows test for free physical mem giampaolo#2074
  fix OSX tests broken by accident
  update HISTORY + give CREDITS for @arossert, @smoofra, @mayeut for giampaolo#2102, giampaolo#2156, giampaolo#2010
  build fix for Mac OS, Apple Silicon (giampaolo#2010)
  Linux: fix missing SPEED_UNKNOWN definition (giampaolo#2156)
  Use system-level values for Windows virtual memory (giampaolo#2077)
  feature: use ABI3 for cp36+ (giampaolo#2102)
  fix py2 compatibility
  improve API speed benchmark script giampaolo#2102
  fix: linter issues from giampaolo#2146 (giampaolo#2155)
  chore: skip test_cpu_freq on macOS arm64 (giampaolo#2146)
  pre-release + give CREDITS to @mayeut (PR giampaolo#2040) and @eallrich (new supporter)
  Fix a typo (giampaolo#2047)
  move isort and coverage config into pyproject.toml
  fix giampaolo#2021, fix giampaolo#1954, provide OSX arm64 bins + add pyproject.toml (giampaolo#2040)
  refactor git_log.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[M1 macOS BigSur] no suitable image found
4 participants