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

feature: use ABI3 for cp36+ #2102

Merged
merged 2 commits into from
Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
85 changes: 0 additions & 85 deletions .ci/appveyor/install.ps1

This file was deleted.

49 changes: 46 additions & 3 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@
on: [push, pull_request]
name: build
jobs:
# Linux + macOS + Python 3
linux-macos-py3:
# Linux + macOS + Windows Python 3
py3:
name: ${{ matrix.os }}-py3
runs-on: ${{ matrix.os }}
timeout-minutes: 20
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-12]
os: [ubuntu-latest, macos-12, windows-2019]

steps:
- name: Cancel previous runs
Expand Down Expand Up @@ -60,6 +60,49 @@ jobs:
mv dist/psutil*.tar.gz wheelhouse/
python scripts/internal/print_hashes.py wheelhouse/

# Windows cp37+ tests
# psutil tests do not like running from a virtualenv with python>=3.7 so
# not using cibuildwheel for those. run them "manually" with this job.
windows-py3-test:
name: windows-py3-test-${{ matrix.python }}-${{ matrix.architecture }}
needs: py3
runs-on: windows-2019
timeout-minutes: 20
strategy:
fail-fast: false
matrix:
python: ["3.7", "3.8", "3.9", "3.10", "~3.11.0-0"]
architecture: ["x86", "x64"]

steps:
- name: Cancel previous runs
uses: styfle/cancel-workflow-action@0.9.1
with:
access_token: ${{ github.token }}
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
with:
python-version: "${{ matrix.python }}"
architecture: "${{ matrix.architecture }}"
cache: pip
cache-dependency-path: .github/workflows/build.yml
- name: Download wheels
uses: actions/download-artifact@v3
with:
name: wheels
path: wheelhouse
- name: Run tests
run: |
mkdir .tests
cd .tests
pip install $(find ../wheelhouse -name '*-cp36-abi3-${{ matrix.architecture == 'x86' && 'win32' || 'win_amd64'}}.whl')[test]
export PYTHONWARNINGS=always
export PYTHONUNBUFFERED=1
export PSUTIL_DEBUG=1
python ../psutil/tests/runner.py
python ../psutil/tests/test_memleaks.py
shell: bash
Copy link
Owner

@giampaolo giampaolo Oct 18, 2022

Choose a reason for hiding this comment

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

Before I was against removing appveyor, but with pyproject.toml, wheels and Py_LIMITED_API I now see why this is necessary. One question though: with this new addition GitHub will test setup.py using Py_LIMITED_API, while appveyor CI will test python 2.7 + setup.py without Py_LIMITED_API. Am I correct? Do you still see value in keep using appveyor or do you think we should abandon it?

Some background on why we're using appveyor:

  • historical reason: initially (years ago) GitHub did not support windows
  • to split work on 2 CI services and therefore complete the whole CI test/build sooner (but perhaps I'm wrong on this)

Copy link
Contributor

@ben9923 ben9923 Oct 19, 2022

Choose a reason for hiding this comment

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

I personally find using 2 CI services a bit confusing and harder to maintain, and GHA's better integration with the repo/PRs is a nice advantage.

I believe there's no speed benefit to this split. GHA's concurrency limits are (much?) higher than the current jobs amount.

Edit: Ugh, I forgot building Windows 2.7 wheels (on GHA, didn't know AppVeyor has the tools) requires adding a Visual C++ dependency...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Am I correct?

Yes you are.

Do you still see value in keep using appveyor or do you think we should abandon it?

If you want to keep Python 2.7 wheels on Windows, then I'd say it shall remain. AppVeyor still keeps old images around and that includes some build tools "required" for Python 2.7 that are not available for download anymore and not available on GHA.

Given AppVeyor has 1 concurrent job and GHA has higher concurrency limits than the current jobs amount, it will be faster with GHA. The current limiter on master to get a green CI is AppVeyor with an average of 30 minutes. This PR cuts that at least in half.


# Linux + macOS + Python 2
linux-macos-py2:
name: ${{ matrix.os }}-py2
Expand Down
45 changes: 0 additions & 45 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,62 +24,17 @@ environment:
PYTHON_VERSION: "2.7.x"
PYTHON_ARCH: "32"

- PYTHON: "C:\\Python36"
PYTHON_VERSION: "3.6.x"
PYTHON_ARCH: "32"

- PYTHON: "C:\\Python37"
PYTHON_VERSION: "3.7.x"
PYTHON_ARCH: "32"

- PYTHON: "C:\\Python38"
PYTHON_VERSION: "3.8.x"
PYTHON_ARCH: "32"

- PYTHON: "C:\\Python39"
PYTHON_VERSION: "3.9.x"
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
PYTHON_ARCH: "32"

- PYTHON: "C:\\Python310"
PYTHON_VERSION: "3.10.x"
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
PYTHON_ARCH: "32"

# 64 bits

- PYTHON: "C:\\Python27-x64"
PYTHON_VERSION: "2.7.x"
PYTHON_ARCH: "64"

- PYTHON: "C:\\Python36-x64"
PYTHON_VERSION: "3.6.x"
PYTHON_ARCH: "64"

- PYTHON: "C:\\Python37-x64"
PYTHON_VERSION: "3.7.x"
PYTHON_ARCH: "64"

- PYTHON: "C:\\Python38-x64"
PYTHON_VERSION: "3.8.x"
PYTHON_ARCH: "64"

- PYTHON: "C:\\Python39-x64"
PYTHON_VERSION: "3.9.x"
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
PYTHON_ARCH: "64"

- PYTHON: "C:\\Python310-x64"
PYTHON_VERSION: "3.10.x"
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
PYTHON_ARCH: "64"

init:
- "ECHO %PYTHON% %PYTHON_VERSION% %PYTHON_ARCH%"

install:
- "powershell .ci\\appveyor\\install.ps1"
# - ps: (new-object net.webclient).DownloadFile('https://raw.github.com/pypa/pip/master/contrib/get-pip.py', 'C:/get-pip.py')
- "%WITH_COMPILER% %PYTHON%/python.exe -m pip --version"
- "%WITH_COMPILER% %PYTHON%/python.exe -m pip install --upgrade --user setuptools pip"
- "%WITH_COMPILER% %PYTHON%/python.exe scripts/internal/winmake.py setup-dev-env"
Expand Down
32 changes: 13 additions & 19 deletions psutil/_psutil_linux.c
Original file line number Diff line number Diff line change
Expand Up @@ -296,52 +296,46 @@ psutil_proc_cpu_affinity_set(PyObject *self, PyObject *args) {
cpu_set_t cpu_set;
size_t len;
pid_t pid;
int i, seq_len;
Py_ssize_t i, seq_len;
PyObject *py_cpu_set;
PyObject *py_cpu_seq = NULL;

if (!PyArg_ParseTuple(args, _Py_PARSE_PID "O", &pid, &py_cpu_set))
return NULL;

if (!PySequence_Check(py_cpu_set)) {
PyErr_Format(PyExc_TypeError, "sequence argument expected, got %s",
Py_TYPE(py_cpu_set)->tp_name);
goto error;
return PyErr_Format(PyExc_TypeError, "sequence argument expected, got %R", Py_TYPE(py_cpu_set));
}

py_cpu_seq = PySequence_Fast(py_cpu_set, "expected a sequence or integer");
if (!py_cpu_seq)
goto error;
seq_len = PySequence_Fast_GET_SIZE(py_cpu_seq);
seq_len = PySequence_Size(py_cpu_set);
if (seq_len < 0) {
return NULL;
}
CPU_ZERO(&cpu_set);
for (i = 0; i < seq_len; i++) {
PyObject *item = PySequence_Fast_GET_ITEM(py_cpu_seq, i);
PyObject *item = PySequence_GetItem(py_cpu_set, i);
Copy link

Choose a reason for hiding this comment

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

Suggested change
PyObject *item = PySequence_GetItem(py_cpu_set, i);
PyObject *item = PySequence_GetItem(py_cpu_set, i);
if (!item) {
goto error;
}

(Previously, PySequence_Fast checked for errors while getting items.)

if (!item) {
return NULL;
}
#if PY_MAJOR_VERSION >= 3
long value = PyLong_AsLong(item);
#else
long value = PyInt_AsLong(item);
#endif
Py_XDECREF(item);
if ((value == -1) || PyErr_Occurred()) {
if (!PyErr_Occurred())
PyErr_SetString(PyExc_ValueError, "invalid CPU value");
goto error;
return NULL;
}
CPU_SET(value, &cpu_set);
}

len = sizeof(cpu_set);
if (sched_setaffinity(pid, len, &cpu_set)) {
PyErr_SetFromErrno(PyExc_OSError);
goto error;
return PyErr_SetFromErrno(PyExc_OSError);
}

Py_DECREF(py_cpu_seq);
Py_RETURN_NONE;

error:
if (py_cpu_seq != NULL)
Py_DECREF(py_cpu_seq);
return NULL;
}
#endif /* PSUTIL_HAVE_CPU_AFFINITY */

Expand Down
2 changes: 1 addition & 1 deletion psutil/arch/windows/process_info.c
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ psutil_get_cmdline(DWORD pid, int use_peb) {
wcslen(szArglist[i]));
if (py_unicode == NULL)
goto out;
PyList_SET_ITEM(py_retlist, i, py_unicode);
PyList_SetItem(py_retlist, i, py_unicode);
py_unicode = NULL;
}
ret = py_retlist;
Expand Down
20 changes: 17 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,11 @@ exclude_lines = [
]

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

[tool.cibuildwheel]
build = ["cp36-*", "cp37-*", "cp38-*", "cp39-*", "cp310-*"]
skip = ["*-musllinux*"]
skip = ["pp*", "*-musllinux*"]
test-extras = "test"
test-command = [
"PYTHONWARNINGS=always PYTHONUNBUFFERED=1 PSUTIL_DEBUG=1 python {project}/psutil/tests/runner.py",
Expand All @@ -52,3 +51,18 @@ test-command = [

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

[tool.cibuildwheel.windows]
# psutil tests do not like running from a virtualenv with python>=3.7
# restrict build & tests to cp36
# cp36-abi3 wheels will need to be tested outside cibuildwheel for python>=3.7
build = "cp36-*"
test-command = [
"python {project}/psutil/tests/runner.py",
"python {project}/psutil/tests/test_memleaks.py"
]

[tool.cibuildwheel.windows.environment]
PYTHONWARNINGS = "always"
PYTHONUNBUFFERED = "1"
PSUTIL_DEBUG = "1"