Skip to content

[Build] Skip spinloop extension on Python < 3.11#44783

Merged
Harry-Chen merged 5 commits into
vllm-project:mainfrom
Jasen2201:fix/spinloop-py310-build
Jun 11, 2026
Merged

[Build] Skip spinloop extension on Python < 3.11#44783
Harry-Chen merged 5 commits into
vllm-project:mainfrom
Jasen2201:fix/spinloop-py310-build

Conversation

@Jasen2201

@Jasen2201 Jasen2201 commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

The spinloop extension uses Py_buffer, PyBuffer_Release, and the "y*" format code in PyArg_ParseTupleAndKeywords — none of which are available in the Python Limited/Stable API. Combined with USE_SABI 3.11, this causes compilation to fail on Python 3.10 because the Limited API headers do not declare these symbols.

Guard the spinloop target in both CMakeLists.txt (Python_VERSION check) and setup.py (sys.version_info check) so the build skips it on older interpreters. This aligns build-time behavior with the existing runtime fallback (try/except import in vllm/_spinloop.py) and keeps vLLM buildable on Python 3.10, which is still within the declared requires-python range (>=3.10,<3.15).

Purpose

Test Plan

Test Result


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added the ci/build label Jun 7, 2026
@Jasen2201

Copy link
Copy Markdown
Contributor Author

This issue was introduced by #36517

@Harry-Chen

Copy link
Copy Markdown
Member

I think it is fixed by #43659?

@Jasen2201

Copy link
Copy Markdown
Contributor Author

I think it is fixed by #43659?

Not exactly. #43659 only fixed the runtime side (added try/except ImportError in Python). The build itself still fails on Python 3.10 because spinloop.cpp uses Py_buffer which is unavailable under Py_LIMITED_API. You never get a wheel, so the runtime fallback never gets a chance to run. This PR adds the missing build-time guard.

@Harry-Chen Harry-Chen added the ready ONLY add when PR is ready to merge/full CI is needed label Jun 8, 2026
@Harry-Chen

Copy link
Copy Markdown
Member

@Jasen2201 Please add sign-off to your commits

@Jasen2201 Jasen2201 force-pushed the fix/spinloop-py310-build branch from fff4f02 to a18cddd Compare June 8, 2026 03:13
The spinloop extension uses Py_buffer, PyBuffer_Release, and the "y*"
format code in PyArg_ParseTupleAndKeywords — none of which are available
in the Python Limited/Stable API. Combined with USE_SABI 3.11, this
causes compilation to fail on Python 3.10 because the Limited API
headers do not declare these symbols.

Guard the spinloop target in both CMakeLists.txt (Python_VERSION check)
and setup.py (sys.version_info check) so the build skips it on older
interpreters. This aligns build-time behavior with the existing runtime
fallback (try/except import in vllm/_spinloop.py) and keeps vLLM
buildable on Python 3.10, which is still within the declared
requires-python range (>=3.10,<3.15).

Signed-off-by: Jasen2201 <yajizhan@amd.com>
@Jasen2201 Jasen2201 force-pushed the fix/spinloop-py310-build branch from a18cddd to abaaf20 Compare June 8, 2026 03:14
@Jasen2201

Copy link
Copy Markdown
Contributor Author

@Jasen2201 Please add sign-off to your commits

Done, rebased onto the latest origin/main and added Signed-off-by.

@Jasen2201

Copy link
Copy Markdown
Contributor Author

The CI failure is unrelated to this PR @Harry-Chen

@Harry-Chen Harry-Chen enabled auto-merge (squash) June 9, 2026 08:11
@mergify

mergify Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Hi @zhyajie, the pre-commit checks have failed. Please run:

uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

@Jasen2201

Copy link
Copy Markdown
Contributor Author

Hi @zhyajie, the pre-commit checks have failed. Please run:

uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

https://github.com/vllm-project/vllm/actions/runs/27285824805/job/80592869532?pr=44783 The update-dockerfile-graph hook failure is unrelated to this PR. It's caused by a pre-existing mismatch between docker/Dockerfile and docs/assets/contributing/dockerfile-stages-dependency.png on main. This PR only modifies CMakeLists.txt — no Dockerfile changes involved.

@Harry-Chen Harry-Chen merged commit ef67071 into vllm-project:main Jun 11, 2026
188 checks passed
@pschlan-amd

Copy link
Copy Markdown
Contributor

Thanks for fixing this!

Saddss pushed a commit to Saddss/vllm that referenced this pull request Jun 14, 2026
divineearthly pushed a commit to divineearthly/vllm that referenced this pull request Jun 19, 2026
Signed-off-by: Jasen2201 <yajizhan@amd.com>
Signed-off-by: divineearthly <divineearthly@gmail.com>
tunglinwood pushed a commit to tunglinwood/vllm that referenced this pull request Jun 22, 2026
nkzhenhua pushed a commit to nkzhenhua/vllm that referenced this pull request Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/build ready ONLY add when PR is ready to merge/full CI is needed

3 participants