Skip to content

[CI/Build][Bugfix] fix qutlass cmake error when set QUTLASS_SRC_DIR#26773

Merged
mgoin merged 3 commits intovllm-project:mainfrom
izhuhaoran:fix-qutlass-build
Oct 15, 2025
Merged

[CI/Build][Bugfix] fix qutlass cmake error when set QUTLASS_SRC_DIR#26773
mgoin merged 3 commits intovllm-project:mainfrom
izhuhaoran:fix-qutlass-build

Conversation

@izhuhaoran
Copy link
Copy Markdown
Contributor

@izhuhaoran izhuhaoran commented Oct 14, 2025

Purpose

When building from source with QUTLASS_SRC_DIR specified, the current code encounters the following error:

  CMake Error at cmake/external_projects/qutlass.cmake:30 (message):
    [QUTLASS] source directory could not be resolved.

The cause is that when specifying QUTLASS_SRC_DIR, only FetchContent_Declare is executed, but FetchContent_Populate is omitted. This results in qutlass_SOURCE_DIR remaining undefined.

This PR fixes the error the same way as in cmake/external_projects/flashmla.cmake


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

Signed-off-by: izhuhaoran <izhuhaoran@qq.com>
@mergify mergify bot added the ci/build label Oct 14, 2025
Copy link
Copy Markdown
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 correctly fixes a CMake build error for the qutlass external project that occurs when QUTLASS_SRC_DIR is specified. The change to use FetchContent_MakeAvailable is the proper, idiomatic solution, making the build script more robust. The fix is sound. I have one suggestion to improve logging consistency for better maintainability.

endif()
message(STATUS "[QUTLASS] QuTLASS is available at ${qutlass_SOURCE_DIR}")
FetchContent_MakeAvailable(qutlass)
message(STATUS "QuTLASS is available at ${qutlass_SOURCE_DIR}")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

For consistency with other log messages in this file, it is recommended to retain the [QUTLASS] prefix. This helps in quickly identifying the origin of build messages, which is valuable for debugging build issues.

message(STATUS "[QUTLASS] QuTLASS is available at ${qutlass_SOURCE_DIR}")

@izhuhaoran izhuhaoran marked this pull request as draft October 14, 2025 07:23
…avoid custlass already exists error

Signed-off-by: izhuhaoran <izhuhaoran@qq.com>
@izhuhaoran izhuhaoran marked this pull request as ready for review October 14, 2025 07:55
@youkaichao
Copy link
Copy Markdown
Member

cc @mgoin

Copy link
Copy Markdown
Member

@mgoin mgoin left a comment

Choose a reason for hiding this comment

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

LGTM although I'm not sure why FetchContent_Populate is needed if flashmla doesn't need it

@mgoin mgoin added bug Something isn't working ready ONLY add when PR is ready to merge/full CI is needed labels Oct 14, 2025
@izhuhaoran
Copy link
Copy Markdown
Contributor Author

LGTM although I'm not sure why FetchContent_Populate is needed if flashmla doesn't need it

flashmla.cmake use FetchContent_MakeAvailable, which contains FetchContent_Populate.

@mgoin mgoin enabled auto-merge (squash) October 15, 2025 01:12
@mgoin mgoin merged commit e471d7c into vllm-project:main Oct 15, 2025
85 checks passed
bbartels pushed a commit to bbartels/vllm that referenced this pull request Oct 16, 2025
…llm-project#26773)

Signed-off-by: izhuhaoran <izhuhaoran@qq.com>
Co-authored-by: Michael Goin <mgoin64@gmail.com>
Signed-off-by: bbartels <benjamin@bartels.dev>
lywa1998 pushed a commit to lywa1998/vllm that referenced this pull request Oct 20, 2025
…llm-project#26773)

Signed-off-by: izhuhaoran <izhuhaoran@qq.com>
Co-authored-by: Michael Goin <mgoin64@gmail.com>
alhridoy pushed a commit to alhridoy/vllm that referenced this pull request Oct 24, 2025
…llm-project#26773)

Signed-off-by: izhuhaoran <izhuhaoran@qq.com>
Co-authored-by: Michael Goin <mgoin64@gmail.com>
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
…llm-project#26773)

Signed-off-by: izhuhaoran <izhuhaoran@qq.com>
Co-authored-by: Michael Goin <mgoin64@gmail.com>
Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
…llm-project#26773)

Signed-off-by: izhuhaoran <izhuhaoran@qq.com>
Co-authored-by: Michael Goin <mgoin64@gmail.com>
Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
rtourgeman pushed a commit to rtourgeman/vllm that referenced this pull request Nov 10, 2025
…llm-project#26773)

Signed-off-by: izhuhaoran <izhuhaoran@qq.com>
Co-authored-by: Michael Goin <mgoin64@gmail.com>
devpatelio pushed a commit to SumanthRH/vllm that referenced this pull request Nov 29, 2025
…llm-project#26773)

Signed-off-by: izhuhaoran <izhuhaoran@qq.com>
Co-authored-by: Michael Goin <mgoin64@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ci/build 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.

3 participants