Skip to content

Conversation

@tlrmchlsmth
Copy link
Member

@tlrmchlsmth tlrmchlsmth commented Oct 2, 2025

Revert back to 20.04 to avoid breaking the wheel for users on OSes such as UBI 9 that have an earlier glibc version than 20.04.

Also use uv to install python 3.12 instead of deadsnakes.

@mergify mergify bot added documentation Improvements or additions to documentation ci/build labels Oct 2, 2025
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 aims to revert the build image to Ubuntu 20.04 for better glibc compatibility and to switch to uv for Python installation. The changes in the base stage of the Dockerfile correctly implement this, simplifying the Python setup.

However, there is a critical omission: the vllm-base stage (starting at line 271) has not been updated. It still uses the old deadsnakes PPA method for installing Python, which is inconsistent with the changes made in the base stage and the stated goal of the PR. To fully adopt uv for Python management and ensure consistency across the Docker build, the vllm-base stage must be refactored to use the same uv-based Python installation method.

Additionally, I've left one comment regarding an improvement for build script robustness.

Signed-off-by: Tyler Michael Smith <[email protected]>
@simon-mo simon-mo added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 2, 2025
Copy link
Collaborator

@ProExpertProg ProExpertProg left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

@simon-mo
Copy link
Collaborator

simon-mo commented Oct 2, 2025

UV not happy




[2025-10-02T18:21:16Z] #19 [base 10/10] RUN --mount=type=cache,target=/root/.cache/uv     uv pip install --system -r requirements/cuda.txt     --extra-index-url https://download.pytorch.org/whl/cu$(echo 12.8.1 \| cut -d. -f1,2 \| tr -d '.')
  | [2025-10-02T18:21:17Z] #19 1.133 Using Python 3.12.11 environment at: /root/.local/share/uv/python/cpython-3.12.11-linux-x86_64-gnu
  | [2025-10-02T18:21:17Z] #19 1.134 error: The interpreter at /root/.local/share/uv/python/cpython-3.12.11-linux-x86_64-gnu is externally managed, and indicates the following:
  | [2025-10-02T18:21:17Z] #19 1.134
  | [2025-10-02T18:21:17Z] #19 1.134   This Python installation is managed by uv and should not be modified.
  | [2025-10-02T18:21:17Z] #19 1.134
  | [2025-10-02T18:21:17Z] #19 1.134 Consider creating a virtual environment with `uv venv`.
  | [2025-10-02T18:21:17Z] #19 ERROR: process "/bin/sh -c uv pip install --system -r requirements/cuda.txt     --extra-index-url ${PYTORCH_CUDA_INDEX_BASE_URL}/cu$(echo $CUDA_VERSION \| cut -d. -f1,2 \| tr -d '.')" did not complete successfully: exit code: 2
  |  

<br class="Apple-interchange-newline">

@simon-mo
Copy link
Collaborator

simon-mo commented Oct 2, 2025

@codex review

@chatgpt-codex-connector
Copy link

Codex Review: Didn't find any major issues. Another round soon, please!

ℹ️ 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

@simon-mo
Copy link
Collaborator

simon-mo commented Oct 3, 2025

I'll just merge this in given pip install is working




[2025-10-03T04:13:59Z] #19 [base 10/10] RUN --mount=type=cache,target=/root/.cache/uv     uv pip install --python /opt/venv/bin/python3 -r requirements/cuda.txt     --extra-index-url https://download.pytorch.org/whl/cu$(echo 12.8.1 \| cut -d. -f1,2 \| tr -d '.')
  |  

<br class="Apple-interchange-newline">

@simon-mo simon-mo merged commit 27edd2a into main Oct 3, 2025
19 of 24 checks passed
@simon-mo simon-mo deleted the back_to_20.04 branch October 3, 2025 05:21
simon-mo added a commit that referenced this pull request Oct 3, 2025
…26103)

Signed-off-by: Tyler Michael Smith <[email protected]>
Co-authored-by: Simon Mo <[email protected]>
Signed-off-by: simon-mo <[email protected]>
yewentao256 pushed a commit that referenced this pull request Oct 3, 2025
…26103)

Signed-off-by: Tyler Michael Smith <[email protected]>
Co-authored-by: Simon Mo <[email protected]>
Signed-off-by: yewentao256 <[email protected]>
tomeras91 pushed a commit to tomeras91/vllm that referenced this pull request Oct 6, 2025
karan pushed a commit to karan/vllm that referenced this pull request Oct 6, 2025
southfreebird pushed a commit to southfreebird/vllm that referenced this pull request Oct 7, 2025
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 10, 2025
choprahetarth pushed a commit to Tandemn-Labs/vllm that referenced this pull request Oct 11, 2025
shyeh25 pushed a commit to shyeh25/vllm that referenced this pull request Oct 14, 2025
…t#26103)

Signed-off-by: Tyler Michael Smith <[email protected]>
Co-authored-by: Simon Mo <[email protected]>
Signed-off-by: simon-mo <[email protected]>
lywa1998 pushed a commit to lywa1998/vllm that referenced this pull request Oct 20, 2025
alhridoy pushed a commit to alhridoy/vllm that referenced this pull request Oct 24, 2025
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
rtourgeman pushed a commit to rtourgeman/vllm that referenced this pull request Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/build documentation Improvements or additions to documentation 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.

6 participants