Repository navigation
test(rccl): guard Makefile dep tracking stays on -MM - #12933
Merged
Merged
Conversation
NCCL PR #1806 dropped system headers from include tracking so the device DEPENDS realpath loop does not walk libc/HIP trees. Scan the src Makefiles in the host-test guards phase so a revert to -M fails CI.
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
![amd-argus[bot]](https://avatars.githubusercontent.com/in/4740020?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
ARGUS
Bottom line: Worth settling before approval: depgen_sites reports a logical index rather than a Makefile line, so a real -MM revert at src/Makefile:284 is cited as line 243.
Comment @argus review to re-run, or @argus re-review after pushing.
pvallem
reviewed
Oct 6, 2026
pvallem
reviewed
Oct 6, 2026
Move the scan under test/, report physical Makefile line numbers in failures.
Contributor
There was a problem hiding this comment.
ARGUS
Bottom line: Worth settling before approval: test_parser_ignores_sed_of_d_tmp never reaches the gate it names, so the reject branch at test_makefile_depflags.py:70 has no coverage.
Comment @argus review to re-run, or @argus re-review after pushing.
mustafabar
approved these changes
Oct 13, 2026
sdarisala
approved these changes
Oct 14, 2026
Contributor
TheRock Submodule Bump ActivityNewest first
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NCCL PR #1806 dropped system headers from include tracking so the device DEPENDS realpath loop does not walk libc/HIP trees. Scan the src Makefiles in the host-test guards phase so a revert to -M fails CI.
Motivation
NCCL 2.30.7 switched Makefile include tracking from -M to -MM so system headers are not realpath’d during depgen. That change is already on develop; this PR only guards it against a silent revert.
Technical Details
src/test_makefile_depflags.py requires -MM/-MMD on the dep recipes in src/Makefile, src/device/Makefile, and src/diagnostics/device/Makefile, and rejects -M/-MD. Host-test CI runs it via run_host_tests.sh guards. The ctest registration is local-only.
Issue Tracking
JIRA ID: AICOMRCCL-1848
Test Plan
Run python3 projects/rccl/src/test_makefile_depflags.py -v and confirm a -MM → -M revert fails. After push, check that the host-unit-test job runs the scan.
Test Result
All 6 local cases passed, and a -M mutation failed as expected. GPU / TheRock jobs were not run; they do not execute this script.
Submission Checklist