Skip to content

build: Exclude ucx-exchange sources from clang-tidy#17715

Closed
dan13bauer wants to merge 1 commit into
facebookincubator:mainfrom
dan13bauer:remove-ucx-exchange-from-clang-tidy
Closed

build: Exclude ucx-exchange sources from clang-tidy#17715
dan13bauer wants to merge 1 commit into
facebookincubator:mainfrom
dan13bauer:remove-ucx-exchange-from-clang-tidy

Conversation

@dan13bauer
Copy link
Copy Markdown
Collaborator

Summary

Exclude ucx-exchange/ sources from scripts/checks/run-clang-tidy.py, mirroring
the existing cudf/ and wave/ exclusions.

Motivation

The ucx-exchange GPU transport plugin (under velox/experimental/) is built on
top of cuDF and UCX. Its sources pull in CUDA compiler flags and CUDA/UCX headers,
which clang-tidy does not support — the same reason cudf/ and wave/ are already
excluded. Without this exclusion, clang-tidy fails when it encounters changed
ucx-exchange/ files.

Changes

  • Add "ucx-exchange/" not in matched_file to the changed-line detection in
    git_changed_lines, so changed lines in those files are not collected for
    tidying.
  • Add "ucx-exchange/" not in file to the file-list filter in tidy, so those
    files are excluded from the clang-tidy run.

clang-tidy does not support CUDA compiler flags or CUDA/UCX headers, so
the ucx-exchange GPU transport sources cannot be compiled as standalone
translation units. Exclude them from both the changed-line detection and
the file list, mirroring the existing cudf/ and wave/ exclusions.
@netlify
Copy link
Copy Markdown

netlify Bot commented Jun 3, 2026

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 652cf1e
🔍 Latest deploy log https://app.netlify.com/projects/meta-velox/deploys/6a20463ca5a1ee00080daa56

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 3, 2026
@dan13bauer
Copy link
Copy Markdown
Collaborator Author

@majetideepak Is there a way to make clang support the CUDA/UCX headers?

FYI: @zoltan

Copy link
Copy Markdown
Collaborator

@czentgr czentgr left a comment

Choose a reason for hiding this comment

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

Thanks!

@meta-codesync
Copy link
Copy Markdown

meta-codesync Bot commented Jun 4, 2026

@bikramSingh91 has imported this pull request. If you are a Meta employee, you can view this in D107459896.

@bikramSingh91 bikramSingh91 added the ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall label Jun 4, 2026
@meta-codesync meta-codesync Bot closed this in 058b1a3 Jun 4, 2026
@meta-codesync
Copy link
Copy Markdown

meta-codesync Bot commented Jun 4, 2026

@bikramSingh91 merged this pull request in 058b1a3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants