Skip to content

[NIXL][XPU] update install script of NIXL#28778

Merged
jikunshang merged 2 commits intovllm-project:mainfrom
zhenwei-intel:update_xpu_nixl
Nov 17, 2025
Merged

[NIXL][XPU] update install script of NIXL#28778
jikunshang merged 2 commits intovllm-project:mainfrom
zhenwei-intel:update_xpu_nixl

Conversation

@zhenwei-intel
Copy link
Contributor

@zhenwei-intel zhenwei-intel commented Nov 15, 2025

update install script of NIXL
pin to 0.7.0 in dockerfile

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the installation script for NIXL and pins its version in the Dockerfile. The changes aim to improve the build process by using rpath to locate shared libraries, which is a good practice. However, I've found a critical issue in how the rpath is being set in the installation script, which will likely cause runtime failures. My review includes a suggested fix for this issue.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Signed-off-by: zhenwei-intel <zhenwei.liu@intel.com>
libxext6 \
libgl1 \
lsb-release \
libaio-dev \
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's this for? if nixl have libaio dependency, I am not sure whether this is a good manner since version may matter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

which is needed by nixl v0.7.1

@jikunshang jikunshang enabled auto-merge (squash) November 17, 2025 02:29
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Nov 17, 2025
@jikunshang jikunshang merged commit d64429b into vllm-project:main Nov 17, 2025
14 checks passed
devpatelio pushed a commit to SumanthRH/vllm that referenced this pull request Nov 29, 2025
Signed-off-by: zhenwei-intel <zhenwei.liu@intel.com>
kitaekatt pushed a commit to kitaekatt/vllm that referenced this pull request Dec 1, 2025
Signed-off-by: zhenwei-intel <zhenwei.liu@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants