Remove setuptools upper bound constraint (<80) #28337
Conversation
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run You ask your reviewers to trigger select CI tests on top of Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. 🚀 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the upper bound on the setuptools dependency to resolve conflicts with other packages. The change is applied to both pyproject.toml and requirements/build.txt. While this addresses the immediate issue, I've raised a concern about completely removing the upper bound, as it could lead to future build failures if setuptools releases a new major version with breaking changes. I've suggested re-introducing a looser upper bound (e.g., <81.0.0) to allow for current versions while protecting against future incompatibilities. This is a common practice to ensure long-term stability.
Fixes vllm-project#28336 The setuptools <80.0.0 upper bound constraint was causing dependency conflicts with packages that require setuptools 80+. Since setuptools 80.x is stable and widely adopted, and there are no known compatibility issues with vLLM's build system, this change removes the upper bound while maintaining the tested minimum version of 77.0.3. This update affects both pyproject.toml and requirements/build.txt to keep them in sync as indicated by the mirror comments. Signed-off-by: Cole Murray <colemurray.cs@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ColeMurray
force-pushed
the
fix-setuptools-constraint
branch
from
4aaa461 to
0d6252f
Compare
Fixes vllm-project#28336 This addresses AI reviewer feedback by: 1. Setting a conservative upper bound of <81.0.0 (instead of no upper bound) 2. Updating ALL requirement files with setuptools constraints for consistency Files updated: - pyproject.toml - requirements/build.txt - requirements/common.txt - requirements/cpu.txt - requirements/cpu-build.txt - requirements/rocm.txt - requirements/rocm-build.txt - requirements/xpu.txt This allows setuptools 80.x (resolving the dependency conflict) while providing protection against potential breaking changes in future major versions. Note: requirements/tpu.txt and requirements/test.txt use exact version pins (setuptools==78.1.0 and setuptools==77.0.3 respectively) and were left unchanged. Signed-off-by: Cole Murray <colemurray.cs@gmail.com>
github-actions
bot
added
the
ready
|
@ColeMurray using <80.0 on ROCm is required because starting from 80.0 setuptools would translate |
Signed-off-by: Cole Murray <colemurray.cs@gmail.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
Signed-off-by: Cole Murray <colemurray.cs@gmail.com>
3 participants
Fixes #28336
Summary
Removes the
<80.0.0upper bound constraint on setuptools to resolve dependency conflicts with packages that require setuptools 80+.Changes
pyproject.toml: Changedsetuptools>=77.0.3,<80.0.0tosetuptools>=77.0.3requirements/build.txt: Changedsetuptools>=77.0.3,<80.0.0tosetuptools>=77.0.3Rationale
Testing
This change has been tested locally to ensure the build system accepts the updated constraint. CI will verify compatibility with setuptools 80+.