-
Notifications
You must be signed in to change notification settings - Fork 271
[ci] Adding multi-arch CI ASAN run for submodule bumps #5140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
c268007
3412276
d59fc2a
49e986b
e2725ae
0bce1c9
cd156a1
5067be1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -100,6 +100,7 @@ def select_build_runner(platform: str, build_variant: str) -> str: | |
| "build_variant_label": "asan", | ||
| "build_variant_suffix": "asan", | ||
| "build_variant_cmake_preset": "linux-release-asan", | ||
| "expect_failure": True, | ||
| }, | ||
|
Comment on lines
99
to
103
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| "tsan": { | ||
| "build_variant_label": "tsan", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -501,6 +501,20 @@ def should_skip_ci( | |
| print(" Skipping: 'ci:skip' PR label") | ||
| return True | ||
|
|
||
| # 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 | ||
|
Comment on lines
+504
to
+518
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
|
|
||
| # If we have a list of changed files (push/pull_request events), check if | ||
| # CI should run for that set of changed files. For example: if only .md | ||
| # files are changed, skip CI. | ||
|
|
||
There was a problem hiding this comment.
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 Summaryshows up in the UI:The job name in code
ci_summarymatters for required checks:TheRock/build_tools/github_actions/workflow_summary.py
Lines 35 to 39 in 66232fa
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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 😅
There was a problem hiding this comment.
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