Skip to content

[ci] Adding multi-arch CI ASAN run for submodule bumps#5140

Merged
geomin12 merged 8 commits into
mainfrom
users/geomin12/asan-submodule
May 11, 2026
Merged

[ci] Adding multi-arch CI ASAN run for submodule bumps#5140
geomin12 merged 8 commits into
mainfrom
users/geomin12/asan-submodule

Conversation

@geomin12
Copy link
Copy Markdown
Contributor

@geomin12 geomin12 commented May 8, 2026

To get better signal, we are enabling Multi-arch CI ASAN runs for submodule bumps

geomin12 and others added 4 commits May 8, 2026 12:56
- Add push trigger to multi_arch_ci_asan.yml for rocm-systems and
  rocm-libraries path changes
- Add build_only flag to TestRocmDecision to skip tests entirely
- When ASAN variant detects submodule changes (non-schedule), set
  build_only=True which disables all test runners
- Nightly ASAN runs continue to run full tests as before

This validates ASAN compilation on submodule bumps without consuming
test runner capacity.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add PR triggers (labeled, opened, synchronize) to ASAN workflow
- Skip ASAN CI on PRs unless submodules changed, reducing CI load
- Add expect_failure flag for ASAN variant
- Add tests for ASAN skip logic in should_skip_ci

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines +80 to +81
ci_summary:
name: CI Summary
name: CI ASAN Summary
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have a branch that updates these all to be unambiguous, fyi:

The "name" CI Summary shows up in the UI:

Image

The job name in code ci_summary matters for required checks:

Notes:
* Choose a name for the summary step that is unique across workflow files.
ci.yml should use ci_summary, unit_tests.yml should use unit_tests_summary, etc.
This ensures that required checks can be added in the github UI without
the ambiguity of names overlapping.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oops let me revert that then!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't mind you including the same change here... I'm juggling too many branches and idk when I'll finish + send that 😅

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it's no problem :) i would rather not complicate things for u

Comment on lines 99 to 104
"asan": {
"build_variant_label": "asan",
"build_variant_suffix": "asan",
"build_variant_cmake_preset": "linux-release-asan",
"expect_failure": True,
},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

expect_failure is not working with multi-arch CI when last I analyzed it. I want to remove it. See #4500

@geomin12 geomin12 requested a review from ScottTodd May 8, 2026 21:09
Comment on lines +504 to +516
# Skip ASAN on PRs unless submodule changes are present.
# This avoids running expensive ASAN builds on every PR while still
# catching ASAN issues when library code (submodules) changes.
if (
ci_inputs.is_pull_request
and ci_inputs.build_variant == "asan"
and git_context.changed_files is not None
and git_context.submodule_paths is not None
):
matching = set(git_context.submodule_paths) & set(git_context.changed_files)
if not matching:
print(" Skipping: ASAN PR without submodule changes")
return True
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

People are going to send their own [draft] PRs that include submodule updates outside of our regularly scheduled bumps, and this is going to run expensive ASan builds on those PRs.

I bet we'll want a system like https://github.com/iree-org/iree/blob/fbe60d8c0fc391d5f64fe0a71bca330f7bff445c/build_tools/github_actions/configure_ci.py#L203-L212

Let's keep an eye on this. Probably worth a TODO or more details in the comment.

@geomin12 geomin12 merged commit b7abaa4 into main May 11, 2026
16 checks passed
@geomin12 geomin12 deleted the users/geomin12/asan-submodule branch May 11, 2026 16:02
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage May 11, 2026
litvaOo pushed a commit that referenced this pull request May 12, 2026
To get better signal, we are enabling Multi-arch CI ASAN runs for
submodule bumps

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants