Repository navigation
feat(ci): adding prefix ci: to gfx targets along with configure_ci_labels.py script to keep it uniform - #8435
Conversation
Add configure_ci_labels.py to create and sync CI-related labels (ci:, gfx, test:, test_filter:, etc.) across repositories. Cleanup only targets gfx, test:, and test_filter: prefixes to avoid touching other labels. Intended for local use only. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove cleanup feature from configure_ci_labels.py (create only) - Add ci: prefix to gfx labels (e.g., ci:gfx120X-all, ci:gfx950-dcgpu) - Update ci_behavior_manipulation.md docs to reflect new label format Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update configure_multi_arch_ci.py to recognize the new ci:gfx prefix for GPU architecture labels (e.g., ci:gfx120X-all instead of gfx120X-all). Also update tests to use the new label format. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When using --create --force, existing labels will be updated to match the color and description defined in the script. Without --force, existing labels are skipped (checked by name only). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
All test: labels now use consistent color #3FA7D6 (light blue). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When using --force --dry-run, now shows what will change: [DRY RUN] Would update label: test:amdsmi (color: #277804 -> #3FA7D6) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
Pre-commit check failed⛔ pre-commit failed Please run locally:
This repo uses |
- Add fmt: off/on around CI_LABELS to preserve readability - Shorten test: label descriptions to fit line limits - Fix other long lines to pass black formatter Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
🎉 All checks passed! This PR is ready for review. |
| # Labels from ROCm/TheRock for CI behavior manipulation. | ||
| # Format: (name, color, description) | ||
| # fmt: off | ||
| CI_LABELS: list[tuple[str, str, str]] = [ |
There was a problem hiding this comment.
Any specific reason we have CI_LABELS as list of tuples instead of list[Label]?
I see that the Label dataclass (line 115) is only used for labels read back from GitHub and CI_LABELS as list of tuples. Now create_labels compares tuples against a Label-keyed map on every iteration. Would it make more sense to make this as list[Label] too?
| # Family keys are lowercase, so normalize the target. | ||
| target = label.split("-")[0].lower() | ||
| # Strip ci: prefix, then split on dash to get the base family. | ||
| target = label[3:].split("-")[0].lower() |
There was a problem hiding this comment.
nit: might want to remove hardcoded length 3 here and use len. Using manual length will cause silent failures in case the prefix changes in the future.
There was a problem hiding this comment.
We can use label.removeprefix("ci:") here instead of label[3:]
| target = label[3:].split("-")[0].lower() | |
| target = label.removeprefix("ci:")].split("-")[0].lower() |
| | `ci:asan` | Enable ASAN CI builds and tests. ASAN CI is skipped by default on PRs unless this label is present. | | ||
| | `ci:host-asan` | Alias for `ci:asan`. Enable ASAN CI builds and tests. | | ||
| | `gfx...` | Opt-in to building and testing the specified gfx family (e.g. `gfx120X`, `gfx950`) | | ||
| | `ci:gfx...` | Opt-in to building and testing the specified gfx family (e.g. `ci:gfx120X-all`, `ci:gfx950-dcgpu`) | |
There was a problem hiding this comment.
(follow-up ok) let's consider ways to make this table easier to understand and reference, such as:
- an exhaustive list of all labels. We could link to https://github.com/ROCm/TheRock/labels?q=ci%3A and also list them directly in the document
- screenshots showing how to apply labels and what the CI does once those labels are applied
- a warning/note that applying labels will restart the CI jobs
| # Family keys are lowercase, so normalize the target. | ||
| target = label.split("-")[0].lower() | ||
| # Strip ci: prefix, then split on dash to get the base family. | ||
| target = label[3:].split("-")[0].lower() |
There was a problem hiding this comment.
We can use label.removeprefix("ci:") here instead of label[3:]
| target = label[3:].split("-")[0].lower() | |
| target = label.removeprefix("ci:")].split("-")[0].lower() |
| # test: labels (project-specific test opt-in) | ||
| ("test:hipblaslt", "3FA7D6", "Run full tests for hipblaslt"), | ||
| ("test:hipcub", "3FA7D6", "Run full tests for hipcub"), |
There was a problem hiding this comment.
How about a named constant for each color? Then we can document the colors in one place and change them with a one line code change as needed.
| if dry_run: | ||
| print(f" [DRY RUN] Would update label: {name} ({change_str})") | ||
| return True |
There was a problem hiding this comment.
Does this script also log when a label does not need updates? The Summary: 20 created, 9 updated, 40 already matched output seems like no?
How about logging that too?
[DRY RUN] Would create label: ci:gfx90a
[DRY RUN] Would create label: ci:gfx90c
+ [DRY RUN] Label is already up to date: test:rocprim (color: #3FA7D6)
[DRY RUN] Would update label: test:ocltst (color: #03B8ED -> #3FA7D6)
[DRY RUN] Would update label: test:hip-tests (color: #20566E -> #3FA7D6)The lines don't fit neatly along the same column that way though :P
- Convert CI_LABELS from list of tuples to list[Label] dataclass
- Add named color constants for label categories (COLOR_CI_GENERAL, etc.)
- Use label.removeprefix("ci:") instead of hardcoded label[3:]
- Add logging for labels already up to date during dry run
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| # Trim suffixes from labels since amdgpu_family_matrix.py | ||
| # specifies families with no suffix (e.g. `gfx94x`) but | ||
| # we have some labels like `gfx94X-dcgpu` or `gfx103X-linux`. | ||
| # we have some labels like `ci:gfx94X-dcgpu` or `ci:gfx103X-linux`. |
There was a problem hiding this comment.
Oh, should we also update https://github.com/ROCm/TheRock/blob/main/build_tools/github_actions/bump_automation.py ?
Bump PRs are still using the unprefixed labels:
TheRock/build_tools/github_actions/bump_automation.py
Lines 62 to 67 in 5b25aa6
Example: #8576
Progress on #7375 with much more updates incoming as we expand our labelling system
We are beginning the label enhancement work incrementally, with some work starting when #8322 lands.
In this PR, we are handling:
We are adding a script to create labels and ensure uniformity across all repos that use multi-arch CI. we are also adding
ci:on the gfx targets and using the script to ensure uniformity. The script has no destructive properties, only creating or updatingLocal dry run:
Local actual run: