-
-
Notifications
You must be signed in to change notification settings - Fork 15.9k
[build] fix priority of cuda-compat libraries in ld loading #34226
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -132,8 +132,8 @@ ENV UV_LINK_MODE=copy | |
| # Verify GCC version | ||
| RUN gcc --version | ||
|
|
||
| # Ensure CUDA compatibility library is loaded | ||
| RUN echo "/usr/local/cuda-$(echo "$CUDA_VERSION" | cut -d. -f1,2)/compat/" > /etc/ld.so.conf.d/cuda-compat.conf && ldconfig | ||
| # Ensure CUDA compatibility library is loaded at last to avoid overriding the system libraries | ||
| RUN echo "/usr/local/cuda-$(echo "$CUDA_VERSION" | cut -d. -f1,2)/compat/" > /etc/ld.so.conf.d/zzz-cuda-compat.conf && ldconfig | ||
|
|
||
| # ============================================================ | ||
| # SLOW-CHANGING DEPENDENCIES BELOW | ||
|
|
@@ -560,8 +560,8 @@ ENV UV_HTTP_TIMEOUT=500 | |
| ENV UV_INDEX_STRATEGY="unsafe-best-match" | ||
| ENV UV_LINK_MODE=copy | ||
|
|
||
| # Ensure CUDA compatibility library is loaded | ||
| RUN echo "/usr/local/cuda-$(echo "$CUDA_VERSION" | cut -d. -f1,2)/compat/" > /etc/ld.so.conf.d/cuda-compat.conf && ldconfig | ||
| # Ensure CUDA compatibility library is loaded at last to avoid overriding the system libraries | ||
| RUN echo "/usr/local/cuda-$(echo "$CUDA_VERSION" | cut -d. -f1,2)/compat/" > /etc/ld.so.conf.d/zzz-cuda-compat.conf && ldconfig | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the comment on line 136, this is another instance of repeated logic for CUDA version string manipulation. Applying the suggested refactoring in this |
||
|
|
||
| # ============================================================ | ||
| # SLOW-CHANGING DEPENDENCIES BELOW | ||
|
|
||
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.
While this change is correct, the command substitution
$(echo "$CUDA_VERSION" | cut -d. -f1,2)to extract the major and minor CUDA version is repeated over 20 times in this Dockerfile. This makes the file hard to maintain and prone to errors if the logic needs to be updated.To improve maintainability, consider defining variables for the different CUDA version formats at the beginning of the build stage and reusing them. A pattern for this already exists in this file for
PYTHON_VERSION_STR(lines 499-500).You could add a
RUNcommand in thebasestage (e.g., after line 121) to define and export these variables:Then, you could source this file in subsequent
RUNcommands and use the variables. For example, this line would become:Applying this pattern throughout the Dockerfile would significantly reduce redundancy and improve readability. A similar change would be needed in the
vllm-basestage.