Skip to content

fix(stinkytofu): guard clang-tidy setup behind ENABLE_CLANG_TIDY - #10468

Merged
KKyang merged 1 commit into
developfrom
users/kkyang/stinkytofu-clang-tidy-guard
Aug 10, 2026
Merged

KKyang merged 1 commit into
developfrom
users/kkyang/stinkytofu-clang-tidy-guard

Conversation

@KKyang

@KKyang KKyang commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

JIRA ID : N/A

Close #10467

Motivation

ENABLE_CLANG_TIDY defaults to OFF, but the CMAKE_MODULE_PATH append, include(ClangTidy) and add_clang_tidy_custom_target() all ran unconditionally. ClangTidy.cmake lives in the superbuild's top-level cmake/modules, two levels above the subproject, so configuring stinkytofu depended on a path outside its own tree even with the feature disabled.

Any checkout that does not materialise the top-level cmake/ directory fails to configure:

CMake Error at CMakeLists.txt:366 (include):
  include could not find requested file:

    ClangTidy

CMake Error at CMakeLists.txt:367 (add_clang_tidy_custom_target):
  Unknown CMake command "add_clang_tidy_custom_target".

The second error is a cascade of the first, so the reported failure is an unknown command rather than a missing file — disproportionately confusing to diagnose.

Technical Details

Moved all three statements inside if(ENABLE_CLANG_TIDY).

include(ClangTidy OPTIONAL) was deliberately not used: it would silently skip the include and defer the failure to add_clang_tidy_custom_target() with a worse message. When clang-tidy is explicitly requested, a missing module should stay a hard error.

Test Plan

Standalone configure of shared/stinkytofu with the guard applied, both with the option off (default) and on, checking that clang-tidy targets appear only when requested.

Test Result

cmake -S shared/stinkytofu -B /tmp/st-off -G Ninja -DROCM_PATH=$ROCM_PATH
  exit=0  -- Build files have been written to: /tmp/st-off

cmake -S shared/stinkytofu -B /tmp/st-on  -G Ninja -DROCM_PATH=$ROCM_PATH -DENABLE_CLANG_TIDY=ON
  exit=0  -- Build files have been written to: /tmp/st-on

ninja -t targets | grep -c tidy → 0 with the option off, 3 with it on. The original failure was reproduced before the fix on a checkout whose cmake/ directory was not materialised.

Not run: full stinkytofu build/test suite (configure-only change), Windows.

Submission Checklist

Risk level

None. Configure-time only, and strictly narrows when the clang-tidy block executes. With ENABLE_CLANG_TIDY=ON behaviour is unchanged; with it off the block simply no longer runs.

ENABLE_CLANG_TIDY defaults to OFF, but the CMAKE_MODULE_PATH append,
include(ClangTidy) and add_clang_tidy_custom_target() ran
unconditionally. ClangTidy.cmake lives in the superbuild's
cmake/modules, two levels above this subproject, so configuring
stinkytofu depended on a path outside its own tree even with the
feature disabled -- a standalone or partial checkout failed with
"include could not find requested file: ClangTidy" followed by an
"Unknown CMake command" error that gives no hint at the cause.

Move all three inside the option guard. When clang-tidy is explicitly
requested a missing module is still a hard error, which is the intended
behaviour; include(... OPTIONAL) would instead defer the failure to
add_clang_tidy_custom_target() with a worse message.
@therock-pr-bot

therock-pr-bot Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@codecov

codecov Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #10468      +/-   ##
===========================================
- Coverage    69.18%   69.18%   -0.00%     
===========================================
  Files         2762     2762              
  Lines       454772   454772              
  Branches     67092    67092              
===========================================
- Hits        314625   314624       -1     
  Misses      117273   117273              
- Partials     22874    22875       +1     
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 38.11% <ø> (-<0.01%) ⬇️
TensileLite-Unit 75.91% <ø> (ø)
hipBLAS 90.62% <ø> (ø) Carriedforward from a0fb871
hipBLASLt 34.89% <ø> (ø)
hipCUB 83.47% <ø> (ø) Carriedforward from a0fb871
hipDNN 86.52% <ø> (ø) Carriedforward from a0fb871
hipFFT 47.74% <ø> (ø) Carriedforward from a0fb871
hipRAND 76.12% <ø> (ø) Carriedforward from a0fb871
hipSOLVER 69.03% <ø> (ø) Carriedforward from a0fb871
hipSPARSE 86.27% <ø> (ø) Carriedforward from a0fb871
rocBLAS 48.23% <ø> (ø) Carriedforward from a0fb871
rocFFT 52.13% <ø> (ø) Carriedforward from a0fb871
rocRAND 57.02% <ø> (ø) Carriedforward from a0fb871
rocSOLVER 76.92% <ø> (ø) Carriedforward from a0fb871
rocSPARSE 72.56% <ø> (ø) Carriedforward from a0fb871
rocThrust 91.60% <ø> (ø) Carriedforward from a0fb871

*This pull request uses carry forward flags. Click here to find out more.
see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@KKyang
KKyang merged commit 2531c64 into develop Aug 10, 2026
123 of 136 checks passed
@KKyang
KKyang deleted the users/kkyang/stinkytofu-clang-tidy-guard branch August 10, 2026 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stinkytofu: include(ClangTidy) runs unconditionally, breaking configure when ENABLE_CLANG_TIDY=OFF

2 participants