-
Notifications
You must be signed in to change notification settings - Fork 14
fix install build file lack #45
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
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.
Pull Request Overview
This PR fixes a packaging issue where files in the vllm_xpu_kernels folder were not being included in the package distribution. The fix involves switching from automatic package discovery to explicit package specification and adding auto-imports for compiled extensions.
- Switches from
setuptools.packages.findto explicitpackagesconfiguration - Adds automatic imports for compiled C extensions (
_C,_moe_C,_vllm_fa2_C,_xpu_C) in the package's__init__.py
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pyproject.toml | Changes package discovery from find configuration to explicit packages list |
| vllm_xpu_kernels/init.py | Adds imports for compiled C extension modules |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| where = ["."] | ||
| include = ["vllm_xpu_kernels*"] | ||
| [tool.setuptools] | ||
| packages = ["vllm_xpu_kernels"] |
Copilot
AI
Sep 24, 2025
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 explicit packages configuration may not include subdirectories or subpackages within vllm_xpu_kernels. Consider using find_packages() or explicitly listing all subpackages if they exist, or use packages = {"vllm_xpu_kernels": "vllm_xpu_kernels"} with include-package-data = true to ensure all necessary files are included.
| packages = ["vllm_xpu_kernels"] | |
| packages = "find:" | |
| include-package-data = true |
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.
LGTM
Signed-off-by: Kunshang Ji <[email protected]>
Signed-off-by: Kunshang Ji <[email protected]>
Signed-off-by: Kunshang Ji <[email protected]>
Signed-off-by: Kunshang Ji <[email protected]>
Signed-off-by: Kunshang Ji <[email protected]>
Signed-off-by: Kunshang Ji <[email protected]>
Signed-off-by: Kunshang Ji <[email protected]>
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.PLEASE FILL IN THE PR DESCRIPTION HERE ENSURING ALL CHECKLIST ITEMS ABOVE HAVE BEEN CONSIDERED.
Purpose
we will always use develop install in ci to avoid python local first issue which may have conflicts in system path and delveop path for run pytest
update doc about build steps: prefer using
pip install .instead ofpython setup.py installTest Plan
Test Result
(Optional) Documentation Update
BEFORE SUBMITTING, PLEASE READ https://docs.vllm.ai/en/latest/contributing (anything written below this line will be removed by GitHub Actions)