Skip to content

[DO NOT MERGE][test] Rewire Multi-Arch CI onto TheRock's consolidated CI engine - #13645

Closed
geomin12 wants to merge 5 commits into
developfrom
users/geomin12/external-ci-file-goodbye
Closed

geomin12 wants to merge 5 commits into
developfrom
users/geomin12/external-ci-file-goodbye

Conversation

@geomin12

Copy link
Copy Markdown
Collaborator

Summary

Test scaffolding for ROCm/TheRock#external-ci-file-goodbye, which deletes
configure_external_repo_ci.py and folds its logic into
configure_ci_path_filters.py + configure_multi_arch_ci.py.

What this PR specifically tests

This diff touches .github/workflows/therock*, which is exactly the #8514 case.

Expected: changed_projects resolves to all rocm-libraries configured projects, NOT full coverage. Specifically, rocm-systems components (rccl, hip-tests, rocshmem) must NOT appear in the test selection.

Base branch for the stacked PRs

The rocPRIM and skip-CI test PRs target this branch rather than develop, so
their diffs contain no workflow change. A bundled workflow edit would widen
them to every configured project and make the narrowing paths untestable.

⚠️ Do not merge. TheRock refs point at a branch, not a pinned SHA.

TheRock removed configure_external_repo_ci.py and now derives changed_projects
itself, so drop the `configure` job and read the value off `setup` instead.
The external_repo JSON gains "projects_config" so TheRock can map this repo's
diff onto project paths; test_type now comes from setup as well.

Also migrates the setup call from the retired linux_test_labels/
windows_test_labels inputs to the general pr_labels input.

cv-libs is dropped from prebuilt_stages. It used to be removed conditionally
when projects/rpp changed, which is no longer expressible: prebuilt_stages is
an input to the same job that now computes changed_projects, and TheRock does
not filter explicit prebuilt stages by changed-project impact. Always
rebuilding cv-libs is the safe choice until TheRock does that filtering.

TEMPORARY: all TheRock refs point at the branch
users/geomin12/external-ci-file-goodbye for testing. Do not merge as-is.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot added github actions project: none Does not target any component labels Oct 14, 2026
@therock-pr-bot

therock-pr-bot Bot commented Oct 14, 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

Copy link
Copy Markdown

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

geomin12 and others added 3 commits October 14, 2026 11:34
Removing the `configure` job left a dangling dependency in
multi_arch_ci_summary's needs list, which fails workflow validation before any
job starts:

  job "multi_arch_ci_summary" needs job "configure" which does not exist

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ernal-ci-file-goodbye

# Conflicts:
#	.github/workflows/therock-multi-arch-ci.yml
The bot cleared baseline-run-id in #13623 alongside the TheRock ref bump to
2a55cec, and this branch inherited the empty value when develop was merged.
An empty baseline makes therock-multi-arch-ci.yml pass prebuilt_stages="" and
stage_reuse_mode=off, so every stage rebuilds and the run looks like
changed_projects failed to narrow anything. It did not: setup resolved all 34
rocm-libraries projects and put rccl/rocshmem/core-hiptests in
reusable_artifacts, exactly as #8514 expects.

Restore 38220501023, the baseline develop carried before the bump and the one
the stacked rocPRIM PR still uses, so the two test PRs differ only in their
diffs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
geomin12 added a commit to ROCm/TheRock that referenced this pull request Oct 15, 2026
Two gaps let a rocm-libraries change confined to CI plumbing schedule the full
test suite, including rocm-systems components it cannot affect.

_CALLER_WORKFLOW_PATTERNS covered .github/workflows/therock* but not the
ci-env action those workflows read their pinned TheRock ref, baseline run ID,
container image and runner labels from. ci-env is edited on exactly the same
occasions as the workflows -- every TheRock ref bump touches both -- so such a
PR carried a path matching no pattern and no configured project, hit the
conservative _unclassifiable_paths fallback, and widened to full coverage. The
narrowing in _WORKFLOW_CHANGE_SCOPED_REPOS was never reached.

test_type was also pinned to "standard" for every external repo run.
GitContext.from_external_repo() synthesizes changed_files=[repo_name] and
submodule_paths=[repo_name] so stage reuse can treat the repo as one unit,
which makes has_submodule_changes unconditionally True and always selects the
submodule branch of _determine_test_type(). No external repo could reach
"quick" without an explicit test_filter:quick label.

is_external_ci_only_change() now answers that question from the calling repo's
real diff, and runs ahead of the submodule check. Skippable paths are stripped
first so a docs edit riding along does not disqualify the run; an empty or
fully skippable diff returns False, falling through to the caller's default
rather than quietly downgrading. The test_filter: label still outranks it.

Replaying ROCm/rocm-libraries#13645's diff against the real repos-config.json
narrows 56 scheduled suites to 34, dropping rccl, rocshmem, hip-tests,
rocgdb-*, profiler-hub, rocprofiler-*, amdsmi, aqlprofile and the rest of
rocm-systems.

Note that "quick" drops tests entirely for families flagged
run-full-tests-only, so gfx1250 becomes build-only on ref bumps.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ernal-ci-file-goodbye

Both conflicts trace to #13698, which reverted the TheRock ref bump #13623.

.github/workflows/therock-multi-arch-ci.yml: kept this branch's version. Every
develop hunk re-pins an @<ref> from 2a55cec to c01e3a3, and this branch
deliberately points those at the users/geomin12/external-ci-file-goodbye
branch instead. The remaining hunk restores the `configure` job and its
configure_external_repo_ci.py call, which is exactly what this PR deletes.

.github/actions/ci-env/action.yml: took develop's version. The revert restored
baseline-run-id 38220501023, the same value 67a16e4 pinned by hand, so the
stage reuse this branch needs survives while the now-stale DO-NOT-MERGE
rationale goes away: develop owns that value again. therock-ref follows develop
back to c01e3a3; its only consumer here is the rocjitsu race check job.

Side effect: ci-env now matches develop exactly, so it drops out of the PR diff
and the run narrows on the workflow path alone.
@geomin12

Copy link
Copy Markdown
Collaborator Author

testing complete

@geomin12 geomin12 closed this Oct 15, 2026
@geomin12
geomin12 deleted the users/geomin12/external-ci-file-goodbye branch October 15, 2026 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant